[ZeroIntensity]

Based on #135435.

• Issue: Support TLS Encrypted ClientHello (ECH) #89730

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #35363261 | 📁 Comparing 7e3fc05 against main (7ee9ddd)

  🔍 Preview build  

21 files changed · ± 21 modified

± Modified

• c-api/code.html
• deprecations/index.html
• deprecations/pending-removal-in-3.19.html
• deprecations/pending-removal-in-future.html
• library/cmd.html
• library/concurrent.futures.html
• library/dis.html
• library/email.utils.html
• library/fileinput.html
• library/json.html
• and 11 more...

[sftcd]

Good to see this being progressed. Feel free to reach out if we can help or test something.

[gpshead]

Disclosure: I used Claude Fable to help analyze the PR as I don't spend much time deep in the _ssl module guts these days, or speak sparkly new ECH terms yet. It read OpenSSL's source and the RFC, and it worked to reproduce many of the behaviors described. I've read through the findings myself and edited some for the wording and suggestions - but yes this will still read a lot like a model review. -gps

This was reviewed against RFC 9849 and a local build with OpenSSL 4.0. The comments below are mostly about the docs (the GREASE example contradicts the RFC's 6.2.1, and the rejection/retry semantics need spelling out). One seemingly upstream OpenSSL 4.0 bug (noted below) that affects set_outer_alpn_protocols(), and some smaller API consistency questions. The docs example and the outer-ALPN constraint are the ones to block on.

[gpshead]

This example (and the OP_ECH_GREASE text above, "This can elicit an ECH retry configuration from a server") teaches saving the retry config from a GREASE handshake and reconnecting with it. RFC 9849 6.2.1 forbids that for a GREASEing client.

I don't think we need to hide what OpenSSL exposes via get_ech_retry_config(), but our docs shouldn't present it as the way to bootstrap ECH.

Could the example instead show the real flow: config from DNS, catch SSLECHRejectedError, retry once with exc.retry_config? And reword the OP_ECH_GREASE entry to describe its actual purpose (making non-ECH connections look like ECH ones, the RFC's 6.2).

[gpshead]

This suppress is hiding something users will hit. When the outer ALPN list isn't a subset of the inner one and the server rejects ECH, the client doesn't get SSLECHRejectedError. It gets a plain SSLError (BAD_EXTENSION), with no retry_config, and get_ech_status() reports ECH_STATUS_FAILED. I tried three combinations against OpenSSL 4.0 with a stale config:

inner	outer	client result
h2	unset	SSLECHRejectedError, retry config present
h2, http/1.1	http/1.1	SSLECHRejectedError, retry config present
h2	http/1.1	SSLError: BAD_EXTENSION, no retry config

This breaks the recovery flow in RFC 9849 6.1.6 for anyone using set_outer_alpn_protocols() the way 10.5 contemplates (a different, non-sensitive list in the outer hello).

I think this is an OpenSSL bug - cc: @sftcd . In ssl/statem/extensions_clnt.c (openssl-4.0), tls_parse_stoc_alpn() validates the server's selection against s->ext.alpn (the inner list) unconditionally and raises SSL_R_BAD_EXTENSION. tls_construct_ctos_alpn() sent s->ext.ech.alpn_outer in the outer hello, and after rejection that is the list the server chose from. The SNI parser in the same file already handles this case ("if we tried ECH and failed, the outer is what's expected"); ALPN doesn't. Because the handshake dies there, it never reaches the ech_required abort in statem_clnt.c that sets retry_configs_ok, which is why no retry config is available. OpenSSL's own tests only use disjoint inner/outer ALPN on the success path. Worth raising upstream. [This was Claude Fable's finding by analyzing the behavior, RFC, and code -gps]

What to do? For now, assuming that is an upstream issue, lets document the constraint on set_outer_alpn_protocols(). Should this test assert the actual exception rather than suppress it, so we notice when the behavior changes or is that going to turn into a change-detector based on the version/behavior of OpenSSL linked against?

We could just assert the server-side ALPN as now and add a comment linking the upstream issue, so the suppress is explained perhaps.

[gpshead]

The per-entry loop skips an entry only when OpenSSL's first non-provider error is ERR_R_PASSED_INVALID_ARGUMENT. That covers unsupported suites and mandatory extensions, but OpenSSL's public_name checks in ech_final_config_checks() fail without raising that code, so those entries are treated as malformed input and the whole list is rejected:

good = ECH_CONFIG[2:]
bad = bytearray(good)
i = bad.index(b'localhost'); bad[i + 8] = ord('.')   # "localhos." (trailing dot)
entries = good + bytes(bad)
ctx.set_ech_config(len(entries).to_bytes(2, 'big') + entries)
# ssl.SSLError: [SSL] internal error

RFC 9849 6.1.7 says clients SHOULD ignore an ECHConfig whose public_name isn't a valid host name, so I'd expect the good entry to be used here. It is only a SHOULD, so I'm not suggesting this blocks the PR.

My bigger worry is that skip-versus-fail depends on which internal OpenSSL path happens to raise which error code, and that can change between OpenSSL releases without anyone noticing. Two thoughts:

• Could we ask OpenSSL to provide a way to load a list while skipping unusable entries, and treat this loop as a stopgap with a comment saying so? I guess OpenSSL 4.0.x has shipped so that API surface is presumably already set in stone...
• Whatever we keep, a test with one unusable entry of each kind (unsupported suite, mandatory extension, bad public_name) next to a good one would pin the behavior. test_mixed_ech_config_versions only covers the unknown-version case.

[gpshead]

I don't think the second half of this is what happens. With OpenSSL 4.0 and a stale client config, I get:

cert valid for public_name	server has ECH keys	exception	retry_config
yes	yes	SSLECHRejectedError	the server's config
yes	no	SSLECHRejectedError	b""
no	yes	SSLCertVerificationError	(no attribute)
no	no	SSLCertVerificationError	(no attribute)

That behaviour is right per RFC 9849 6.1.6: if authenticating the public name fails, it is an ordinary failure, and the client "MUST NOT use the retry configurations" and "MUST NOT treat this as a secure signal to disable ECH".

So SSLECHRejectedError with an empty retry_config has exactly one meaning: the authenticated server has securely disabled ECH, and 6.1.6 says the client SHOULD retry with ECH disabled. I think the docs should spell out the three outcomes, because that is the decision the caller has to make:

• non-empty retry_config: retry once on a new connection with that config
• empty retry_config: retry on a new connection without ECH
• SSLCertVerificationError (or any other error): don't retry, and don't fall back to non-ECH

It would also be worth a test for the unauthenticated case. ech.pem (public name public.example) with the existing localhost cert perhaps.

[gpshead]

Following on from the retry_config comment in the docs: in the "server has ECH keys but the cert isn't valid for public_name" case, calling this after the failed handshake raises rather than returning b"":

ssl.SSLError: [SSL] OSSL_STORE lib (_ssl.c:2213)

That comes from SSL_ech_get1_retry_config() refusing to release a config it received but couldn't authenticate, which is the right outcome per RFC 9849 6.1.6. But the docs for SSLSocket.get_ech_retry_config() say it returns b"" if no retry configuration is available, and the error text gives the caller nothing to go on.

fill_and_set_sslerror() already maps this same failure to an empty value for the exception attribute. Could this method do the same, so the two agree? If you'd rather keep raising, the docs should say so.

Minor, same function: config_len is uninitialized here but initialized to 0 at the other call site. OpenSSL sets it on every success path today, so this is only for consistency.

[gpshead]

SSLSocket.set_ech_config() and the ech_config= argument reject server-side use, but the context methods don't check the role at all. Both of these succeed silently:

ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER).set_ech_config(config)
ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT).load_ech_keys("ech.pem")

For PROTOCOL_TLS_CLIENT and PROTOCOL_TLS_SERVER contexts the role is known, so raising would be consistent with the socket level and with how newPySSLSocket() rejects mismatched roles. PROTOCOL_TLS contexts would have to stay permissive.

[gpshead]

A few places where the docs and the code (or the PR's own tests) disagree:

• outer_server_hostname is documented here as None for a server-side socket, but test_ech_handshake asserts the server side returns 'public.example' after a successful ECH handshake. Lets make the docs consistent, the public name the client used is useful to a server.
• SSLSocket.get_ech_status() and SSLSocket.get_ech_retry_config() return None when the socket has no SSL object yet (test_socket_without_ssl_object asserts this). The docs promise an ECHStatus and bytes. Either document the None, or raise as some other SSLSocket methods do when there is no SSL wrapper.
• ECHStatus has eleven members and only ECH_STATUS_GREASE_ECH is documented. Callers will need at least SUCCESS, FAILED, FAILED_ECH, NOT_TRIED and NOT_CONFIGURED explained, including the difference between FAILED and FAILED_ECH (whether the server sent a retry config).

[gpshead]

This is added and test_set_outer_server_name exercises it through _sslobj, but there is no wrapper in ssl.py and no documentation, so users can't reach it. Is it meant to be public in this PR?

If so, it needs the wrapper and docs, and I'd expect the same guards as set_ech_config(): client side only, and only before the handshake starts.

If we don't want to do that, leave it out?

[gpshead]

This reads ctx->outer_alpn_protocols and its length while holding only the socket's critical section. _SSLContext._set_outer_alpn_protocols() frees and replaces that buffer under the context's critical section, which is a different lock. On a free-threaded build, one thread calling SSLSocket.set_ech_config() while another calls SSLContext.set_outer_alpn_protocols() on the shared context can pass a freed pointer to SSL_ech_set1_outer_alpn_protos().

Wrapping the read and the OpenSSL call in Py_BEGIN_CRITICAL_SECTION(ctx) should be enough, since OpenSSL copies the buffer.

[bedevere-app]

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.