Repository navigation
Support HTTP/2 over HTTPS and apply the configured TLS settings - #1483
Merged
Merged
Conversation
AlpnProtocolTestServer only reported the negotiated HTTP protocol, and closed any connection that negotiated h2. Record the last handshake instead: the protocols the client offered in ALPN, the TLS version, the cipher suite and the client certificate. Let it act as an HTTP/1.1-only server or require a client certificate, and answer h2 requests as well, so that changes to the TLS configuration and to ALPN can be tested.
Play WS created its TLS engines with AHC's JsseSslEngineFactory, which does not apply the enabled protocols and cipher suites, so play.ws.ssl.enabledProtocols and play.ws.ssl.enabledCipherSuites had no effect. Create the engines through ConfiguredSslEngineFactory instead. It wraps the configured SSLContext in Netty's JdkSslContext and takes the protocols and cipher suites from the final AHC configuration, so modifyUnderlying can still change them. Fail when the configured lists leave nothing enabled, rather than at the first handshake. The loose path is unchanged for now. A Netty SslContext set through modifyUnderlying is still ignored on this path, as before, but now with a warning.
With play.ws.ssl.loose.acceptAnyCertificate = true, Play WS used a separate Netty context, which dropped the configured client certificates and ignored the configured protocols and cipher suites in favor of Netty's defaults. Create the loose path's engines through ConfiguredSslEngineFactory as well, from an SSLContext with the configured key managers and an insecure trust manager, still without a hostname check. A Netty SslContext set through modifyUnderlying is still used as is on this path, as before.
The h2c sibling test only showed that a demanded stream keeps moving while another one on the same connection is suspended. A client that buffered the whole suspended response would have passed as well. Keep the server's write of the suspended response's last frame, and assert that it is still pending after the sibling's 1 MiB has arrived; a write that failed would not pass either. Use a 128 KiB stream window, above the 65,535-byte connection window, so that the suspended stream alone could use up the connection's credit. AHC returns connection credit while a response is suspended; AHC's own tests verify that mechanism directly.
With play.ws.ahc.http2Enabled = true, HTTPS still stayed on HTTP/1.1, because Play WS's engine factory offered no ALPN. Offer h2 and then http/1.1 when HTTP/2 is enabled, as AHC's own factory does, and only http/1.1 for connections that AHC must keep on HTTP/1.1, such as WebSocket and per-connection authentication. With HTTP/2 disabled, the default, no ALPN extension is sent, as before. An SslContext used as is on the loose path still decides ALPN itself. Test h2 over HTTPS on the normal and loose paths, the fallback to HTTP/1.1, an HTTP CONNECT tunnel, and every SSL debug option. Test that NTLM-authenticated requests offer only http/1.1, also after the same client used h2, that client certificates and TLS protocols apply over h2, and that a suspended h2 stream over HTTPS is held back without stalling a sibling. Run the SSL debug hostname tests with HTTP/2 enabled as well.
With HTTP/2 enabled, AHC 3.0.14 also offers h2 on the TLS connection to an HTTPS proxy. If the proxy selects it, the request fails, because AHC keeps writing HTTP/1.1. Play WS's own proxy settings never create such a proxy: setting WSProxyServer.protocol to "https" only selects the authentication scheme. So only a proxy configured on AHC directly is affected. Revert this note once an AHC release offers only http/1.1 on that connection.
Draft
18 of 22 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Play WS can now negotiate HTTP/2 over HTTPS through ALPN when
play.ws.ahc.http2Enabled = true. The default staysfalse, so the TLS engines Play WS creates keep HTTPS on HTTP/1.1 unless you opt in. A customSslContexton the loose path, or a customSslEngineFactory, still decides ALPN itself, as before.To get there, Play WS now creates all its TLS engines through one engine factory built from its configured
SSLContext. That applies with or without HTTP/2, but with the default configuration the TLS engines stay exactly the same; see "TLS behavior changes" below.TLS behavior changes
These apply whether or not HTTP/2 is enabled.
With the default configuration, nothing changes on the normal and
play.ws.ssl.default = truepaths. The TLS engines thatmainand this branch create from the default settings are identical: the same protocols (TLS 1.3 and 1.2), the same 31 cipher suites in the same order, the same hostname verification and SNI, and no ALPN while HTTP/2 is off.Who is affected:
play.ws.ssl.enabledProtocolsorplay.ws.ssl.enabledCipherSuites. These lists now take effect. Earlier versions ignored them, so a handshake could use a TLS version or cipher suite outside them.["TLSv1.2", "TLSv1.1"]now caps connections at TLS 1.2, where TLS 1.3 was negotiated before.enabledProtocols = [TLSv1.1]on a JVM that disables TLS 1.1, now fails when the client is created instead of being ignored. Falling back to the defaults would defeat the allowlist. The error names the requested values and, for protocols, the enabled ones.AhcConfigBuilder.modifyUnderlyingcan change the enabled protocols and cipher suites, because the factory reads them from the final AHC configuration.play.ws.ssl.loose.acceptAnyCertificate = true. Harmless in practice:play.ws.ssl.keyManager.stores) are now presented. Before, they were silently dropped, and a server that required one failed the handshake withcertificate_required. A server that only optionally requests a client certificate now receives one, and may act on it.enabledProtocols = ["TLSv1.3", "TLSv1.2"]used to be ignored and now applies. Servers that only speak TLS 1.0 or 1.1 then stop working.SslContextset throughmodifyUnderlyingis handled as before:http2Enableddoes not restrict what it offers.main, but Play WS now logs a warning once per client instead of ignoring it silently. Using it there would bypass Play WS's hostname verification and HTTP/2 gating.SslEngineFactorythroughmodifyUnderlying; it then becomes responsible for hostname verification and ALPN.Unchanged: hostname verification on the normal and
ssl.defaultpaths,disableHttpsEndpointIdentificationAlgorithm, and the SSL debug options. The hostname tests from #1480 still pass, with and without HTTP/2.HTTP/2 over HTTPS
play.ws.ahc.http2Enabled = true, their HTTPS connections offerh2and thenhttp/1.1through ALPN. They use HTTP/2 when the server selects it, and HTTP/1.1 otherwise. With the defaultfalse, they send no ALPN extension at all, exactly as before. A customSslContextused on the loose path, or a customSslEngineFactory, controls its own ALPN.h2. So Play WS's engines offerh2only when HTTP/2 is enabled and AHC allows it for that connection. Connections that AHC must keep on HTTP/1.1, such as WebSocket and per-connection authentication (NTLM, SPNEGO, Kerberos), offer onlyhttp/1.1.CONNECTtunnel.HTTPSalso offersh2. If the proxy selects it, the request fails, because AHC keeps speaking HTTP/1.1 to the proxy.WSProxyServer.protocolto"https"(Some("https")in Scala) only selects the authentication scheme and does not make the connection to the proxy use TLS.HTTPSproxy configured on AHC directly.fix/proxy-hop-alpn; its status is asked about in Plans for thefix/proxy-hop-alpnbranch AsyncHttpClient/async-http-client#2358. The README note for it is commit 6.Turning HTTP/2 on by default is a separate decision and not part of this PR.
Commits
ConfiguredSslEngineFactoryon the normal andssl.defaultpaths.modifyUnderlyingsupport, and the warning for an ignoredSslContext.SslContextis still used as is. This can be reviewed on its own, but not reverted on its own once commit 5 is in, because commit 5 builds on it.Testing
New and changed tests:
TlsConfigurationSpec(normal and loose paths):modifyUnderlyingchanges apply;SslContextis used only on the loose path;HttpsHttp2Spec:CONNECTtunnel with HTTP/2 enabled and disabled;http/1.1, also after the same client used h2;AhcConfigBuilderSpec:http2Enabledandhttp2Allowed;SslContext.SslDebugHostnameSpec: now also runs with HTTP/2 enabled.Ahc3CompatibilitySpec:Local runs, all on JDK 17:
validateCode,Test/compileand the tests covering its change.doc, and all tests (18 standalone and 17 integration suites).The tests were also checked against deliberately broken code:
modifyUnderlyingand loose-path client-certificate tests fail againstmain'sconfigureSSL, while the tests for unchanged behavior, such as the hostname checks and theSslContextoverride, pass there;h2is offered while HTTP/2 is disabled, whenhttp2Allowedis ignored, and when a userSslContextis ignored on the loose path;A one-off comparison, not part of the test suite, checked that
mainand this branch build identical TLS engines from the default settings, on the normal and thessl.defaultpath.Not covered: SOCKS proxies, and Kerberos/SPNEGO beyond the NTLM case.
The CI matrix (JDK 17/21/25 × Scala 2.13/3.3/3.9/3.next) runs on this PR.