Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Redis TLS #13223

Open
bbobrov opened this issue Oct 8, 2020 · 23 comments · May be fixed by #19565
Open

Support for Redis TLS #13223

bbobrov opened this issue Oct 8, 2020 · 23 comments · May be fixed by #19565

Comments

@bbobrov
Copy link

bbobrov commented Oct 8, 2020

As a Harbor instance operator, i want to secure the traffic to Redis instance.

This issue might seem not so pressing when internal Redis is used. However, external managed Redis instances can be used, such as Azure Cache for Redis. Although it is possible to enable non-encrypted port, it is disabled by default; Microsoft recommends enabling TLS.

Now Harbor does not support connecting to SSL/TLS redis endpoints. go-redis library supports it. Docker-distribution has a patch for it - distribution/distribution#3161. Chartmuseum has a bugreport about it - helm/chartmuseum#326

I would like to be able to provide rediss:// urls or to set use_ssl=true option when configuring harbor. The options to provide certificates are also required.

@yanji09 yanji09 added the kind/requirement New feature or idea on top of harbor label Oct 19, 2020
@dsalcedolab
Copy link

is there anything new with the upgrade?

@shinji62
Copy link

@yanji09 @ninjadq Any update on that ? Thanks

@ninjadq
Copy link
Member

ninjadq commented May 17, 2021

Hi, we have a plan to support TLS for Redis and the database. It might be included in a future release.

@flyingbricks
Copy link

flyingbricks commented Jun 3, 2021

Similar asks here due to setting up Harbor with AWS ElastiCache and Azure Redis.

It seems without enabling TLS, ElastiCache doesn't even allow to enable authentication. This potentially means even though there are some level of security to be in a VPC, ElastiCache can be accidentally messed up by others in the VPC.

@yilmi
Copy link

yilmi commented Nov 2, 2021

Similar ask from our chart users - bitnami/charts#7691

@lukasmrtvy
Copy link

Maybe would be possible to use Redis Proxy as a sidecar as a temporary solution..

Something like:

@danielzhanghl
Copy link

use stunnel sidecar is another temp option.

https://www.stunnel.org/

@colinwilson
Copy link

@slushysnowman
Copy link
Contributor

Is there any vision on when this is going to be implemented?

@danielzhanghl
Copy link

FYI. trivy support TLS redis from 0.23.0, while trivy adapter does not support TLS redis yet.

@github-actions
Copy link

github-actions bot commented Jul 6, 2022

This issue is being marked stale due to a period of inactivity. If this issue is still relevant, please comment or remove the stale label. Otherwise, this issue will close in 30 days.

@github-actions github-actions bot added the Stale label Jul 6, 2022
@stonezdj stonezdj removed the Stale label Jul 7, 2022
@keliansb
Copy link

Still an issue, please do not close.

@MinerYang
Copy link
Contributor

MinerYang commented Dec 8, 2022

This distribution PR hasn't been included in the distribution v2.8.1 https://github.com/distribution/distribution/pull/3161/files
We will hold this feature request until distribution new release support it.

@hgranillo
Copy link

Another security feature blocked by distribution... (the other one being AWS IAM AssumeRoleWithWebIdentity )

@marevers
Copy link

Distribution release v2.8.2 from May this year (which came more than a year after v2.8.1) also did not include the mentioned PR. It does not look like this feature will be added anytime soon there.

@pfarikrispy
Copy link

Please consider adding this feature as it makes the "supply chain" more robust and secure

@thavlik
Copy link

thavlik commented Sep 15, 2023

I find it incredible how trivial this oft-requested feature would be to implement, and yet here we are three years later. I wanted to volunteer to implement it, but this issue's thread has suggested that would be a fool's errand.

@altynbaev altynbaev linked a pull request Nov 10, 2023 that will close this issue
5 tasks
@krab-skunk
Copy link

krab-skunk commented Nov 14, 2023

i'm using harbor latest helm chart, but i do not know how to configure the TLS option, i'm using AWS Elastic cache for redis.

in the helm chart values.yaml file, i only see those options:

  external:
    # support redis, redis+sentinel
    # addr for redis: <host_redis>:<port_redis>
    # addr for redis+sentinel: <host_sentinel1>:<port_sentinel1>,<host_sentinel2>:<port_sentinel2>,<host_sentinel3>:<port_sentinel3>
    addr: "192.168.0.2:6379"
    # The name of the set of Redis instances to monitor, it must be set to support redis+sentinel
    sentinelMasterSet: ""
    # The "coreDatabaseIndex" must be "0" as the library Harbor
    # used doesn't support configuring it
    # harborDatabaseIndex defaults to "0", but it can be configured to "6", this config is optional
    # cacheLayerDatabaseIndex defaults to "0", but it can be configured to "7", this config is optional
    coreDatabaseIndex: "0"
    jobserviceDatabaseIndex: "1"
    registryDatabaseIndex: "2"
    trivyAdapterIndex: "5"
    # harborDatabaseIndex: "6"
    # cacheLayerDatabaseIndex: "7"
    # username field can be an empty string, and it will be authenticated against the default user
    username: ""
    password: ""
    # If using existingSecret, the key must be REDIS_PASSWORD
    existingSecret: ""```

@marevers
Copy link

i'm using harbor latest helm chart, but i do not know how to configure the TLS option, i'm using AWS Elastic cache for redis.

in the helm chart values.yaml file, i only see those options:

  external:
    # support redis, redis+sentinel
    # addr for redis: <host_redis>:<port_redis>
    # addr for redis+sentinel: <host_sentinel1>:<port_sentinel1>,<host_sentinel2>:<port_sentinel2>,<host_sentinel3>:<port_sentinel3>
    addr: "192.168.0.2:6379"
    # The name of the set of Redis instances to monitor, it must be set to support redis+sentinel
    sentinelMasterSet: ""
    # The "coreDatabaseIndex" must be "0" as the library Harbor
    # used doesn't support configuring it
    # harborDatabaseIndex defaults to "0", but it can be configured to "6", this config is optional
    # cacheLayerDatabaseIndex defaults to "0", but it can be configured to "7", this config is optional
    coreDatabaseIndex: "0"
    jobserviceDatabaseIndex: "1"
    registryDatabaseIndex: "2"
    trivyAdapterIndex: "5"
    # harborDatabaseIndex: "6"
    # cacheLayerDatabaseIndex: "7"
    # username field can be an empty string, and it will be authenticated against the default user
    username: ""
    password: ""
    # If using existingSecret, the key must be REDIS_PASSWORD
    existingSecret: ""```

It's not yet supported yet unfortunately. I assume support is going to be added in the 2.11.0 release.

@krab-skunk
Copy link

oh ok :( thanks for the so quick reply @marevers

@MinerYang
Copy link
Contributor

This distribution PR hasn't been included in the distribution v2.8.2, v2.8.3 https://github.com/distribution/distribution/pull/3161/files
We will hold this feature request until distribution new release support it.

@MinerYang
Copy link
Contributor

MinerYang commented Jan 19, 2024

While taking a look into distribution src, found this redisTLS config seems to be ignored in main when migrating from redigo to go-redis by this commit.
https://github.com/distribution/distribution/blob/fcbc25e7896b6ea115d1f62107483c9325b4a305/registry/handlers/app.go#L522
cc @wy65701436

@MinerYang
Copy link
Contributor

progress:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.