From 49fdf118aeda891401d638ac32296c7d55d54678 Mon Sep 17 00:00:00 2001 From: juhovh Date: Sun, 18 Apr 2021 21:11:48 +1000 Subject: [PATCH] bpo-36076: Add SNI support to ssl.get_server_certificate. (GH-16820) Many servers in the cloud environment require SNI to be used during the SSL/TLS handshake, therefore it is not possible to fetch their certificates using the ssl.get_server_certificate interface. This change adds an additional optional hostname argument that can be used to set the SNI. Note that it is intentionally a separate argument instead of using the host part of the addr tuple, because one might want to explicitly fetch the default certificate or fetch a certificate from a specific IP address with the specified SNI hostname. A separate argument also works better for backwards compatibility. Automerge-Triggered-By: GH:tiran --- Lib/ssl.py | 2 +- Lib/test/test_ssl.py | 26 ++++++++++++++++++- Misc/ACKS | 1 + .../2019-10-16-17-21-53.bpo-36076.FGeQQT.rst | 1 + 4 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-10-16-17-21-53.bpo-36076.FGeQQT.rst diff --git a/Lib/ssl.py b/Lib/ssl.py index 9c1ba581dd6..99d0852dad1 100644 --- a/Lib/ssl.py +++ b/Lib/ssl.py @@ -1475,7 +1475,7 @@ def get_server_certificate(addr, ssl_version=PROTOCOL_TLS, ca_certs=None): cert_reqs=cert_reqs, cafile=ca_certs) with create_connection(addr) as sock: - with context.wrap_socket(sock) as sslsock: + with context.wrap_socket(sock, server_hostname=host) as sslsock: dercert = sslsock.getpeercert(True) return DER_cert_to_PEM_cert(dercert) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 3ad14c63968..403261dda96 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1962,7 +1962,9 @@ class SimpleBackgroundTests(unittest.TestCase): """Tests that connect to a simple server running in the background""" def setUp(self): - server = ThreadedEchoServer(SIGNED_CERTFILE) + self.server_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + self.server_context.load_cert_chain(SIGNED_CERTFILE) + server = ThreadedEchoServer(context=self.server_context) self.server_addr = (HOST, server.port) server.__enter__() self.addCleanup(server.__exit__, None, None, None) @@ -2143,6 +2145,28 @@ class SimpleBackgroundTests(unittest.TestCase): def test_get_server_certificate(self): _test_get_server_certificate(self, *self.server_addr, cert=SIGNING_CA) + @needs_sni + def test_get_server_certificate_sni(self): + host, port = self.server_addr + server_names = [] + + # We store servername_cb arguments to make sure they match the host + def servername_cb(ssl_sock, server_name, initial_context): + server_names.append(server_name) + self.server_context.set_servername_callback(servername_cb) + + pem = ssl.get_server_certificate((host, port)) + if not pem: + self.fail("No server certificate on %s:%s!" % (host, port)) + + pem = ssl.get_server_certificate((host, port), ca_certs=SIGNING_CA) + if not pem: + self.fail("No server certificate on %s:%s!" % (host, port)) + if support.verbose: + sys.stdout.write("\nVerified certificate for %s:%s is\n%s\n" % (host, port, pem)) + + self.assertEqual(server_names, [host, host]) + def test_get_server_certificate_fail(self): # Connection failure crashes ThreadedEchoServer, so run this in an # independent test method diff --git a/Misc/ACKS b/Misc/ACKS index 20359762a52..1eeae0caef5 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1803,6 +1803,7 @@ Michael Urman Hector Urtubia Elizabeth Uselton Lukas Vacek +Juho Vähä-Herttua Ville Vainio Yann Vaginay Andi Vajda diff --git a/Misc/NEWS.d/next/Library/2019-10-16-17-21-53.bpo-36076.FGeQQT.rst b/Misc/NEWS.d/next/Library/2019-10-16-17-21-53.bpo-36076.FGeQQT.rst new file mode 100644 index 00000000000..7e9bc4e461e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-10-16-17-21-53.bpo-36076.FGeQQT.rst @@ -0,0 +1 @@ +Added SNI support to :func:`ssl.get_server_certificate`.