feat: support redis TLS#19666
Conversation
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 2 |
| P3 | 0 |
| Total | 2 |
Reviewed 4 of 4 changed files.
Found 2 Redis TLS/cluster configuration issues.
This is an automated review by Codex GPT-5.5
| // database applies to standalone only; Redis Cluster supports database 0 only, so this | ||
| // is a no-op in cluster mode (the default is 0). | ||
| .database(config.getDatabase()) | ||
| .ssl(config.getEnableTls()) |
There was a problem hiding this comment.
[P2] Verify Redis hostnames when TLS is enabled
Setting only .ssl(true) encrypts the socket but does not enable hostname verification. In Jedis 7.0.0 the default socket factory only verifies the host when a HostnameVerifier is supplied, or when endpoint-identification parameters are supplied through SSLParameters; this config sets neither. As added, druid.cache.enableTls=true can accept any certificate trusted by the JVM regardless of the Redis hostname, so a trusted-but-wrong certificate can impersonate the cache endpoint. Configure endpoint identification or expose an explicit verification/skip option.
There was a problem hiding this comment.
Thank you, i missed that point.
Like dev env case, skipping is lots more easier i think. how about this?
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
I rechecked the two Redis TLS follow-up threads. Current head addresses both: cluster mode no longer propagates the standalone database index, and TLS now defaults to hostname verification with an explicit CA-only hostname-skip option.
Reviewed 4 of 4 changed files.
This is an automated review by Codex GPT-5.5
2 participants
Fixes #17962.
Description
Supporting TLS for Redis and some refactoring simplify calling ctor.
This PR has: