Skip to content

fix(redis): send the TLS SNI and add redis_server_name - #13938

Merged
nic-6443 merged 4 commits into
apache:masterfrom
janiussyafiq:fix/redis-tls-sni
Sep 15, 2026
Merged

nic-6443 merged 4 commits into
apache:masterfrom
janiussyafiq:fix/redis-tls-sni

Conversation

@janiussyafiq

@janiussyafiq janiussyafiq commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Description

redis_ssl: true never set the TLS SNI, so Redis behind a name-routed TLS front (a cloud endpoint on 443) got the default vhost and the plugin failed with unknown prefix: "72".

This builds on #13935 (redis_server_name option, SNI defaults to redis_host, SNI isolated in the keepalive pool) and adds:

  • skip the SNI when the host is an IP literal, otherwise redis_ssl_verify: true with an IP host starts failing on the hostname check (regression against 3.18.0)
  • t/utils/redis-sni.t: end-to-end tests against real TLS fronts, replacing the mocked t/utils/redis.t

Which issue(s) this PR fixes:

Fixes #13926

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

nic-6443
nic-6443 previously approved these changes Sep 14, 2026
AlinsRan
AlinsRan previously approved these changes Sep 15, 2026

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Document the hostname verification change and upgrade path

Defaulting server_name to a DNS redis_host also enables certificate hostname matching when redis_ssl_verify is true. Previously, a connection could pass certificate-chain verification even when the configured DNS alias was not covered by the certificate. That existing configuration can now fail with certificate host mismatch after upgrading.

Please retain the hostname verification, explicitly document the affected configuration and migration path, and explain how to set redis_server_name to a name covered by the certificate. Please also add a regression case showing that a mismatched default hostname fails while an explicit matching override succeeds.

Evidence: apisix/utils/redis.lua and apisix/plugins/limit-count/util.lua pass conf.redis_server_name or conf.redis_host to the TLS handshake for non-IP names. The current description explains the SNI default and IP compatibility, but does not explain this DNS-alias upgrade scenario.

membphis
membphis previously approved these changes Sep 15, 2026

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@janiussyafiq
janiussyafiq dismissed stale reviews from membphis, AlinsRan, and nic-6443 via 540e78d September 15, 2026 04:14

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nic-6443
nic-6443 merged commit 4278bc9 into apache:master Sep 15, 2026
19 checks passed
@janiussyafiq
janiussyafiq deleted the fix/redis-tls-sni branch September 15, 2026 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Redis TLS connections should set SNI (server_name) to redis_host when redis_ssl is enabled

5 participants