Repository navigation
Conversation
Follow the nats.go implementation: parseServerURL saves the current DNS hostname in Server.tlsName for newly discovered IP servers, and makeTLSConn uses that name as TLSConfig.ServerName. As in Go, preserve names per discovered entry without overriding explicit IP servers or hostname URLs. Also retain the original TLS hostname when Java resolves a configured hostname to IP addresses, and pass it to SSLSocket creation so initial connections and reconnects send the intended SNI. Add a loopback reproduction with a valid SNI-selected certificate and an expired default certificate, covering normal TLS and TLS-first connections. Verified the initial-connect and reconnect failures against the original implementations; 116 related tests pass with the fix.
|
Claude finished @wallyqs's task in 3m 42s —— View job Review: Preserve TLS hostname through DNS resolution and server discovery
Summary: The approach is sound and matches the nats.go model: Notes (not blocking):
I did not find correctness or security issues in the production code changes. I was unable to run |
|
Hey @wallyqs , from my research with weak understanding of Java: The PR is correct for what it does: it restores SNI, and I verified that locally. It does not give the parity with nats.go that the description suggests. In Go, ServerName sets SNI and drives the hostname check. The Java client checks no hostname at all, before or after this PR. The PR is still a prerequisite for fixing that, and we could either follow up with second PR, or add it here. |
… server that supplied it A server learned from connect_urls as a bare ip address has no hostname to present as the TLS server name, and nothing to verify its certificate against but the ip. As nats.go has done since 2018 with tlsName, it now carries the hostname of the server whose INFO supplied it, one hop only: an origin that is itself an ip address supplies nothing. - ServerPool: a default overload acceptDiscoveredUrls(discoveredServers, origin) that forwards to the existing method, so custom pools are unaffected. - NatsConnection: handleInfo passes the origin, currentServer or, while the initial INFO is being read, the server being connected to. - NatsServerPool: a newly discovered entry whose host is an ip address gets the origin's hostname when the origin was configured with one. Explicit servers and already known entries are untouched. - NatsUri: tlsHost with getTlsHost() and withTlsHost(), carried through reHost. Equality, hash code and the string form are unchanged. Taken from PR #1637. - SocketDataPort: the unresolved uri's tls host is the server name to present. Tests: reconnect to a gossiped bare ip presents the first server's hostname, TLS after INFO and TLS first; the same reconnect with tlsVerifyHostname on, against a certificate with a DNS name and no ip subject alternative name, passes because the check is against that hostname; NatsUri identity is unchanged by the name; the pool saves the origin's hostname per entry, honors ignoreDiscoveredServers, does not override explicit servers, saves nothing from an ip origin or through the overload without an origin.
|
Superseded by #1638 |
…stname resolution mode Since 2.16.9 the client resolves a hostname to ip addresses before connecting and hands the ip to the SSL socket factory, so no SNI is sent. An endpoint that selects its certificate by SNI, like a TLS terminating proxy or ingress, then serves its default certificate. Before 2.16.9 the hostname reached the TLS layer because the JDK resolved inside InetSocketAddress and the host string was kept. - DataPort: a default connect overload that also receives the uri before resolution. Existing implementations are unchanged. - NatsConnection: tryToConnect passes the unresolved uri to the data port. - SocketDataPort: keeps the hostname as tlsHost, uses it as the peer host and sets it explicitly with SSLParameters.setServerNames before the handshake, in every mode. The explicit call also covers a single label hostname, such as a kubernetes service name, which the JSSE does not derive from the peer host on its own. - Tests: SniTestServer, a loopback server that selects its certificate by SNI, adapted from PR nats-io#1637, and TlsHostnameTests covering all six HostnameResolveMode values with TLS after INFO and TLS first, a single label hostname, the explicit ip control case and the data port hand-off. Unresolved runs through a loopback HTTP CONNECT tunnel, the only way that mode connects. Not covered: a bare ip learned from connect_urls still has no hostname to send.
4 participants
Follow the nats.go implementation: parseServerURL saves the current DNS hostname in Server.tlsName for newly discovered IP servers, and makeTLSConn uses that name as TLSConfig.ServerName. As in Go, preserve names per discovered entry without overriding explicit IP servers or hostname URLs.
Also retain the original TLS hostname when Java resolves a configured hostname to IP addresses, and pass it to SSLSocket creation so initial connections and reconnects send the intended SNI.
Add a loopback reproduction with a valid SNI-selected certificate and an expired default certificate, covering normal TLS and TLS-first connections.