Skip to content

Future-proof HTTPS endpoint identification - #2104

Merged
hyperxpro merged 2 commits into
AsyncHttpClient:mainfrom
chrisvest:endpoint-validation
Aug 23, 2025
Merged

Future-proof HTTPS endpoint identification#2104
hyperxpro merged 2 commits into
AsyncHttpClient:mainfrom
chrisvest:endpoint-validation

Conversation

@chrisvest

Copy link
Copy Markdown
Contributor

Netty 4.2 changes the default for hostname verification for TLS clients, so that it is now enabled by default.

As a result, clients that rely on the default being off will find themselves unable to disable it.

Instead, clients should explicitly configure their desired endpoint identification algorithm in all cases.

Since Netty 4.1.112 we also have a convenient method on the SslContextBuilder for doing this, so we don't need multiple round-trips through SSLParameters.

This PR changes the DefaultSslEngineFactory to make use of this method, so it always configures the endpoint identification algorithm to match the desired setting of AsyncHttpClientConfig..isDisableHttpsEndpointIdentificationAlgorithm().

Netty 4.2 changes the default for hostname verification for TLS clients, so that it is now enabled by default.

As a result, clients that rely on the default being _off_ will find themselves unable to disable it.

Instead, clients should explicitly configure their desired endpoint identification algorithm in all cases.

Since Netty 4.1.112 we also have a convenient method on the `SslContextBuilder` for doing this, so we don't need multiple round-trips through `SSLParameters`.

This PR changes the `DefaultSslEngineFactory` to make use of this method, so it always configures the endpoint identification algorithm to match the desired setting of `AsyncHttpClientConfig..isDisableHttpsEndpointIdentificationAlgorithm()`.
@chrisvest

Copy link
Copy Markdown
Contributor Author

FYI @slandelle @sullis @hyperxpro

@sullis sullis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@chrisvest

Copy link
Copy Markdown
Contributor Author

@hyperxpro I don't have the merge button, so feel free to merge any time.

@hyperxpro
hyperxpro merged commit d2c780d into AsyncHttpClient:main Aug 23, 2025
3 checks passed
@hyperxpro

Copy link
Copy Markdown
Member

Thanks @chrisvest

@chrisvest
chrisvest deleted the endpoint-validation branch August 23, 2025 14:32
hyperxpro added a commit that referenced this pull request Jul 18, 2026
Motivation

`JsseSslEngineFactory` creates the `SSLEngine` using
`sslContext.createSSLEngine(host, port)` but only enables client mode
via `setUseClientMode(true)`. It never configures endpoint
identification, so a JDK `SSLEngine` does not verify the server hostname
during the TLS handshake. As a result, any certificate signed by a
trusted CA is accepted regardless of the requested hostname, leaving the
connection vulnerable to man-in-the-middle attacks.
`DefaultSslEngineFactory` correctly enables hostname verification.

This regression was introduced in #2104, which removed the shared
`setEndpointIdentificationAlgorithm("HTTPS")` call from
`configureSslEngine()` and restored it only in
`DefaultSslEngineFactory`, unintentionally leaving the pure JSSE path
(used with a user-supplied JDK `SSLContext`) without hostname
verification.

Modification

Update `JsseSslEngineFactory.newSslEngine()` to configure the engine's
`SSLParameters` with `setEndpointIdentificationAlgorithm("HTTPS")`,
unless `config.isDisableHttpsEndpointIdentificationAlgorithm()` is
enabled. This matches the behavior of `DefaultSslEngineFactory` while
preserving the documented opt-out.

Result

Connections created through `JsseSslEngineFactory` with a custom JDK
`SSLContext` now perform hostname verification during the TLS handshake,
matching the default implementation. Existing opt-out behavior remains
unchanged, and the `DefaultSslEngineFactory` path is unaffected.
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.

3 participants