Skip to content

Commit 0067af6

Browse files
miss-islingtongpsheadpicnixz
authored
[3.15] gh-160171: Fix ssl get_unverified_chain() on resumed server sessions (GH-160172) (#160418)
gh-160171: Fix ssl get_unverified_chain() on resumed server sessions (GH-160172) Include the client's certificate when the session was resumed from a ticket, and return [] instead of raising AttributeError when the client sent no certificate. ANT-2026-2RMA0IPV (cherry picked from commit a4ac244) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent fdf88ed commit 0067af6

4 files changed

Lines changed: 108 additions & 22 deletions

File tree

‎Doc/library/ssl.rst‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1284,13 +1284,21 @@ SSL sockets also have the following additional methods and attributes:
12841284
If certificate verification was disabled method acts the same as
12851285
:meth:`~SSLSocket.get_unverified_chain`.
12861286

1287+
The verified chain is not stored with a session, so the list is empty
1288+
when the session is resumed (see :attr:`~SSLSocket.session_reused`).
1289+
12871290
.. versionadded:: 3.13
12881291

12891292
.. method:: SSLSocket.get_unverified_chain()
12901293

12911294
Returns raw certificate chain provided by the other
12921295
end of the SSL channel as a list of DER-encoded bytes.
12931296

1297+
No certificates are exchanged when a session is resumed (see
1298+
:attr:`~SSLSocket.session_reused`); the list then holds what was stored
1299+
with the session, which for a session resumed from a ticket on the
1300+
server side is the peer's own certificate only.
1301+
12941302
.. versionadded:: 3.13
12951303

12961304
.. method:: SSLSocket.cipher()

‎Lib/test/test_ssl.py‎

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3009,11 +3009,19 @@ def run(self):
30093009
cert = self.sslconn.getpeercert()
30103010
self.write(repr(cert).encode("us-ascii") + b"\n")
30113011
elif stripped == b'VERIFIEDCHAIN':
3012-
certs = self.sslconn._sslobj.get_verified_chain()
3013-
self.write(len(certs).to_bytes(1, "big") + b"\n")
3012+
try:
3013+
certs = self.sslconn.get_verified_chain()
3014+
except Exception as e:
3015+
self.write(repr(e).encode("us-ascii") + b"\n")
3016+
else:
3017+
self.write(len(certs).to_bytes(1, "big") + b"\n")
30143018
elif stripped == b'UNVERIFIEDCHAIN':
3015-
certs = self.sslconn._sslobj.get_unverified_chain()
3016-
self.write(len(certs).to_bytes(1, "big") + b"\n")
3019+
try:
3020+
certs = self.sslconn.get_unverified_chain()
3021+
except Exception as e:
3022+
self.write(repr(e).encode("us-ascii") + b"\n")
3023+
else:
3024+
self.write(len(certs).to_bytes(1, "big") + b"\n")
30173025
else:
30183026
if (support.verbose and
30193027
self.server.connectionchatty):
@@ -5690,6 +5698,66 @@ def test_internal_chain_server(self):
56905698
res = s.recv(1024)
56915699
self.assertEqual(res, b'\x02\n')
56925700

5701+
def test_internal_chain_server_resumed_session(self):
5702+
# A session resumed from a ticket stores the client's certificate
5703+
# but not the chain it was sent with.
5704+
for version in ssl.TLSVersion.TLSv1_2, ssl.TLSVersion.TLSv1_3:
5705+
if not has_tls_version(version):
5706+
continue
5707+
with self.subTest(version=version.name):
5708+
self.check_chain_server_resumed_session(version)
5709+
5710+
def check_chain_server_resumed_session(self, version):
5711+
client_context, server_context, hostname = testing_context()
5712+
client_context.load_cert_chain(SIGNED_CERTFILE)
5713+
server_context.verify_mode = ssl.CERT_REQUIRED
5714+
client_context.minimum_version = version
5715+
client_context.maximum_version = version
5716+
5717+
server = ThreadedEchoServer(context=server_context, chatty=False)
5718+
with server:
5719+
with client_context.wrap_socket(
5720+
socket.socket(),
5721+
server_hostname=hostname
5722+
) as s:
5723+
s.connect((HOST, server.port))
5724+
s.write(b'UNVERIFIEDCHAIN\n')
5725+
res = s.recv(1024)
5726+
self.assertEqual(res, b'\x02\n')
5727+
# TLS 1.3 sends the ticket after the handshake, so only
5728+
# get the session once data was received.
5729+
session = s.session
5730+
self.assertTrue(session.has_ticket)
5731+
with client_context.wrap_socket(
5732+
socket.socket(),
5733+
server_hostname=hostname,
5734+
session=session
5735+
) as s:
5736+
s.connect((HOST, server.port))
5737+
self.assertTrue(s.session_reused)
5738+
s.write(b'UNVERIFIEDCHAIN\n')
5739+
res = s.recv(1024)
5740+
self.assertEqual(res, b'\x01\n')
5741+
# The verified chain is not stored with the session at all.
5742+
s.write(b'VERIFIEDCHAIN\n')
5743+
res = s.recv(1024)
5744+
self.assertEqual(res, b'\x00\n')
5745+
5746+
def test_internal_chain_server_no_client_cert(self):
5747+
client_context, server_context, hostname = testing_context()
5748+
server_context.verify_mode = ssl.CERT_OPTIONAL
5749+
5750+
server = ThreadedEchoServer(context=server_context, chatty=False)
5751+
with server:
5752+
with client_context.wrap_socket(
5753+
socket.socket(),
5754+
server_hostname=hostname
5755+
) as s:
5756+
s.connect((HOST, server.port))
5757+
s.write(b'UNVERIFIEDCHAIN\n')
5758+
res = s.recv(1024)
5759+
self.assertEqual(res, b'\x00\n')
5760+
56935761

56945762
class TestSSLDebug(unittest.TestCase):
56955763

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
On the server side, :meth:`ssl.SSLSocket.get_unverified_chain` now returns
2+
the client's certificate when the session was resumed from a session ticket,
3+
instead of an empty list, and returns an empty list instead of raising
4+
:exc:`AttributeError` when the client sent no certificate.

‎Modules/_ssl.c‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2018,31 +2018,37 @@ _ssl__SSLSocket_get_unverified_chain_impl(PySSLSocket *self)
20182018
/*[clinic end generated code: output=5acdae414e13f913 input=079f8ff5c205cb3b]*/
20192019
{
20202020
PyObject *retval;
2021+
X509 *peer = NULL;
20212022
/* borrowed reference */
2022-
/* TODO: include SSL_get_peer_certificate() for server-side sockets */
20232023
STACK_OF(X509) *chain = SSL_get_peer_cert_chain(self->ssl);
2024-
if (chain == NULL) {
2024+
/* OpenSSL does not include the peer cert in the chain for server side
2025+
* connections, so the chain alone does not tell whether the client sent
2026+
* a certificate: the chain is empty when it did not, and NULL when the
2027+
* session was resumed from a ticket (which stores the peer cert only). */
2028+
if (self->socket_type == PY_SSL_SERVER) {
2029+
peer = SSL_get_peer_certificate(self->ssl);
2030+
if (peer == NULL) {
2031+
Py_RETURN_NONE;
2032+
}
2033+
}
2034+
else if (chain == NULL) {
20252035
Py_RETURN_NONE;
20262036
}
2027-
retval = _PySSL_CertificateFromX509Stack(self->ctx->state, chain, 1);
2037+
retval = chain == NULL
2038+
? PyList_New(0)
2039+
: _PySSL_CertificateFromX509Stack(self->ctx->state, chain, 1);
20282040
if (retval == NULL) {
2041+
X509_free(peer);
20292042
return NULL;
20302043
}
2031-
/* OpenSSL does not include peer cert for server side connections */
2032-
if (self->socket_type == PY_SSL_SERVER) {
2033-
PyObject *peerobj = NULL;
2034-
X509 *peer = SSL_get_peer_certificate(self->ssl);
2035-
2036-
if (peer == NULL) {
2037-
peerobj = Py_NewRef(Py_None);
2038-
} else {
2039-
/* consume X509 reference on success */
2040-
peerobj = _PySSL_CertificateFromX509(self->ctx->state, peer, 0);
2041-
if (peerobj == NULL) {
2042-
X509_free(peer);
2043-
Py_DECREF(retval);
2044-
return NULL;
2045-
}
2044+
if (peer != NULL) {
2045+
/* consume X509 reference on success */
2046+
PyObject *peerobj = _PySSL_CertificateFromX509(self->ctx->state,
2047+
peer, 0);
2048+
if (peerobj == NULL) {
2049+
X509_free(peer);
2050+
Py_DECREF(retval);
2051+
return NULL;
20462052
}
20472053
int res = PyList_Insert(retval, 0, peerobj);
20482054
Py_DECREF(peerobj);

0 commit comments

Comments
 (0)