THRIFT-6233: Use the peer-address matcher in TSSLServerSocket on every Python - #3839
Draft
slachiewicz wants to merge 1 commit into
Draft
slachiewicz wants to merge 1 commit into
slachiewicz wants to merge 1 commit into
Conversation
slachiewicz
force-pushed
the
THRIFT-6233
branch
from
September 14, 2026 09:44
79be3ca to
42d6fb0
Compare
Member
Author
|
Rebased on master (42d6fb0) to pick up the TNonblockingServerTest fix from THRIFT-6244 behind the AppVeyor failure. The diff is unchanged (same patch-id). |
slachiewicz
marked this pull request as ready for review
September 15, 2026 21:08
slachiewicz
marked this pull request as draft
September 16, 2026 13:12
…y Python Client: py TSSLServerSocket matched a client certificate against the peer address with sslcompat.match_peer_ipaddress on Python 3.12 and later, but with ssl.match_hostname on 3.11 and earlier. The two disagree: only the former reduces an IPv4-mapped peer (THRIFT-6201), and only the latter falls back to the commonName when the certificate has no subjectAltName. A dual-stack listener reports an IPv4 client as ::ffff:127.0.0.1, so a certificate carrying 127.0.0.1 was accepted on 3.12 and refused on 3.10, which THRIFT-3660 papered over by listing the mapped address in client_v3.crt. The server only ever matches an IP address, so match_peer_ipaddress is now its default on every version. The check stays on whenever cert_reqs asks for a client certificate and covers IP subjectAltName records only; which subjects may connect is the application's policy through a validate_callback, which the docstring now shows. TSSLSocket on the client side is unchanged. The backports.ssl_match_hostname branch in sslcompat, the ValueError that named it and the setup.py dependency go (THRIFT-6265): the library requires Python 3 and nothing below 3.5 can reach them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
slachiewicz
force-pushed
the
THRIFT-6233
branch
from
September 16, 2026 13:49
42d6fb0 to
228341e
Compare
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.
JIRA: THRIFT-6233, THRIFT-6265
Client: py
TSSLServerSocketmatches a client certificate against the address the connection arrived from, whenevercert_reqsasks for a certificate. Which function did that depended on the interpreter:sslcompat.match_peer_ipaddresson Python 3.12 and later,ssl.match_hostnameon 3.11 and earlier. The two disagree. Only the former reduces an IPv4-mapped peer (THRIFT-6201); only the latter falls back to the commonName when the certificate has no subjectAltName. A dual-stack listener (host=None) reports an IPv4 client as::ffff:127.0.0.1, so a certificate carrying127.0.0.1was accepted on 3.12 and refused on 3.10. THRIFT-3660 worked around that in 2016 by listing the mapped address inclient_v3.crt; #3857 has to remove that entry because Go 1.27 rejects certificates that carry one.The server only ever matches an IP address, so
match_peer_ipaddressis now its default on every version. The check stays on by default and covers IP subjectAltName records only: an IPv4-mapped peer matches the plain address on every version, and a certificate without an IP subjectAltName is refused wheressl.match_hostnamecompared the commonName with the address string. DNS records are not matched, since a server has no name for its client. Which subjects may connect is the application's policy throughvalidate_callback, which receives thegetpeercert()dictionary and the peer address; the docstring shows a callback that refuses on the subject.TSSLSocketon the client side is unchanged.The
backports.ssl_match_hostnamebranch insslcompat._optional_dependencies, theValueErrorinTSSLServerSocket.__init__that named it, and thesetup.pydependency go, which is THRIFT-6265: the library requires Python 3, so nothing below 3.5 reaches them.Tests drive
TSSLServerSocket.accept()under the running interpreter:client_v3.crtis accepted,client.crt(no subjectAltName) is refused, a client arriving over IPv4 at a dual-stack listener is accepted with a certificate that carries127.0.0.1and no mapped entry (server.crtstands in as that certificate), and a caller-supplied callback refusing on the subject is honoured and receives the certificate and the peer address.lib/py/README.mdrecords the change for 0.25.0.Verified:
test/test_sslsocket.pypasses against this branch on Python 3.10, 3.11, 3.12, 3.13 and 3.14; against master's library on 3.10 the mapped-peer and default-callback tests fail, on 3.14 the default-callback test fails.flake8 --config setup.cfgreports the same 19 pre-existing E501 lines as master and none in the new code.This change was created with AI assistance.