Fix IDN SAN handling

Signed-off-by: Christian Heimes <christian@python.org>

Author: tiran

Doc/library/ssl.rst

@@ -1269,9 +1269,9 @@ SSL sockets also have the following additional methods and attributes:
    .. versionadded:: 3.2
 
    .. versionchanged:: 3.7
-      When ``server_hostname`` is an internationalized domain name
-      (IDN), this attribute now stores the A-label form
-      (``"xn--pythn-mua.org"``), rather than the U-label form
+      The attribute is now always ASCII text. When ``server_hostname`` is
+      an internationalized domain name (IDN), this attribute now stores the
+      A-label form (``"xn--pythn-mua.org"``), rather than the U-label form
       (``"pythön.org"``).
 
 .. attribute:: SSLSocket.session
@@ -1538,23 +1538,34 @@ to speed up repeated connections from the same clients.
 
    .. versionadded:: 3.3
 
-.. method:: SSLContext.set_servername_callback(server_name_callback)
+.. attribute:: SSLContext.set_servername_callback(server_name_callback)
+
+   See :attr:`SSLContext.sni_callback`
+
+   If there is an decoding error on the server name, the TLS connection will
+   terminate with an :const:`ALERT_DESCRIPTION_INTERNAL_ERROR` fatal TLS
+   alert message to the client.
+
+   .. versionadded:: 3.4
+
+.. attribute:: SSLContext.sni_callback
 
    Register a callback function that will be called after the TLS Client Hello
    handshake message has been received by the SSL/TLS server when the TLS client
    specifies a server name indication. The server name indication mechanism
    is specified in :rfc:`6066` section 3 - Server Name Indication.
 
-   Only one callback can be set per ``SSLContext``.  If *server_name_callback*
-   is ``None`` then the callback is disabled. Calling this function a
+   Only one callback can be set per ``SSLContext``.  If *sni_callback*
+   is set to ``None`` then the callback is disabled. Calling this function a
    subsequent time will disable the previously registered callback.
 
-   The callback function, *server_name_callback*, will be called with three
+   The callback function, will be called with three
    arguments; the first being the :class:`ssl.SSLSocket`, the second is a string
    that represents the server name that the client is intending to communicate
    (or :const:`None` if the TLS Client Hello does not contain a server name)
    and the third argument is the original :class:`SSLContext`. The server name
-   argument is the IDNA decoded server name.
+   argument is text. For internationalized domain name, the server
+   name is an IDN A-label (``"xn--pythn-mua.org"``).
 
    A typical use of this callback is to change the :class:`ssl.SSLSocket`'s
    :attr:`SSLSocket.context` attribute to a new object of type
@@ -1575,18 +1586,14 @@ to speed up repeated connections from the same clients.
    returned.  Other return values will result in a TLS fatal error with
    :const:`ALERT_DESCRIPTION_INTERNAL_ERROR`.
 
-   If there is an IDNA decoding error on the server name, the TLS connection
-   will terminate with an :const:`ALERT_DESCRIPTION_INTERNAL_ERROR` fatal TLS
-   alert message to the client.
-
    If an exception is raised from the *server_name_callback* function the TLS
    connection will terminate with a fatal TLS alert message
    :const:`ALERT_DESCRIPTION_HANDSHAKE_FAILURE`.
 
    This method will raise :exc:`NotImplementedError` if the OpenSSL library
    had OPENSSL_NO_TLSEXT defined when it was built.
 
-   .. versionadded:: 3.4
+   .. versionadded:: 3.7
 
 .. method:: SSLContext.load_dh_params(dhfile)
 

Lib/ssl.py

@@ -355,13 +355,20 @@ def __new__(cls, protocol=PROTOCOL_TLS, *args, **kwargs):
         self = _SSLContext.__new__(cls, protocol)
         return self
 
-    def __init__(self, protocol=PROTOCOL_TLS):
-        self.protocol = protocol
+    def _encode_hostname(self, hostname):
+        if hostname is None:
+            return None
+        elif isinstance(hostname, str):
+            return hostname.encode('idna').decode('ascii')
+        else:
+            return hostname.decode('ascii')
 
     def wrap_socket(self, sock, server_side=False,
                     do_handshake_on_connect=True,
                     suppress_ragged_eofs=True,
                     server_hostname=None, session=None):
+        # SSLSocket class handles server_hostname encoding before it calls
+        # ctx._wrap_socket()
         return self.sslsocket_class(
             sock=sock,
             server_side=server_side,
@@ -374,8 +381,12 @@ def wrap_socket(self, sock, server_side=False,
 
     def wrap_bio(self, incoming, outgoing, server_side=False,
                  server_hostname=None, session=None):
-        sslobj = self._wrap_bio(incoming, outgoing, server_side=server_side,
-                                server_hostname=server_hostname)
+        # Need to encode server_hostname here because _wrap_bio() can only
+        # handle ASCII str.
+        sslobj = self._wrap_bio(
+            incoming, outgoing, server_side=server_side,
+            server_hostname=self._encode_hostname(server_hostname)
+        )
         return self.sslobject_class(sslobj, session=session)
 
     def set_npn_protocols(self, npn_protocols):
@@ -389,6 +400,20 @@ def set_npn_protocols(self, npn_protocols):
 
         self._set_npn_protocols(protos)
 
+    def set_servername_callback(self, server_name_callback):
+        if server_name_callback is None:
+            self.sni_callback = None
+        else:
+            if not hasattr(server_name_callback, '__call__'):
+                raise TypeError("not a callable object")
+
+            def shim_cb(sslobj, servername, sslctx):
+                if servername is not None:
+                    servername = servername.encode("ascii").decode("idna")
+                return server_name_callback(sslobj, servername, sslctx)
+
+            self.sni_callback = shim_cb
+
     def set_alpn_protocols(self, alpn_protocols):
         protos = bytearray()
         for protocol in alpn_protocols:
@@ -447,6 +472,10 @@ def hostname_checks_common_name(self, value):
         def hostname_checks_common_name(self):
             return True
 
+    @property
+    def protocol(self):
+        return _SSLMethod(super().protocol)
+
     @property
     def verify_flags(self):
         return VerifyFlags(super().verify_flags)
@@ -749,7 +778,7 @@ def __init__(self, sock=None, keyfile=None, certfile=None,
             raise ValueError("check_hostname requires server_hostname")
         self._session = _session
         self.server_side = server_side
-        self.server_hostname = server_hostname
+        self.server_hostname = self._context._encode_hostname(server_hostname)
         self.do_handshake_on_connect = do_handshake_on_connect
         self.suppress_ragged_eofs = suppress_ragged_eofs
         if sock is not None:
@@ -781,7 +810,7 @@ def __init__(self, sock=None, keyfile=None, certfile=None,
             # create the SSL object
             try:
                 sslobj = self._context._wrap_socket(self, server_side,
-                                                    server_hostname)
+                                                    self.server_hostname)
                 self._sslobj = SSLObject(sslobj, owner=self,
                                          session=self._session)
                 if do_handshake_on_connect:

Lib/test/test_ssl.py

@@ -119,15 +119,6 @@ def data_file(*name):
 DHFILE = data_file("dh1024.pem")
 BYTES_DHFILE = os.fsencode(DHFILE)
 
-# These were generated by doing 'pip install trustme', and then:
-# import trustme
-# ca = trustme.CA()
-# cert = ca.issue_server_cert("pythön.example.org")
-# ca.cert_pem.write_to_path("ssl-idn-ca.pem")
-# cert.private_key_and_cert_chain_pem.write_to_path("ssl-idn-cert.pem")
-IDN_CA = data_file("ssl-idn-ca.pem")
-IDN_CERT = data_file("ssl-idn-cert.pem")
-
 # Not defined in all versions of OpenSSL
 OP_NO_COMPRESSION = getattr(ssl, "OP_NO_COMPRESSION", 0)
 OP_SINGLE_DH_USE = getattr(ssl, "OP_SINGLE_DH_USE", 0)
@@ -2634,7 +2625,7 @@ def test_check_hostname_idn(self):
             sys.stdout.write("\n")
 
         server_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
-        server_context.load_cert_chain(IDN_CERT)
+        server_context.load_cert_chain(IDNSANSFILE)
 
         context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
         context.verify_mode = ssl.CERT_REQUIRED
@@ -2645,18 +2636,26 @@ def test_check_hostname_idn(self):
         # different ways
         idn_hostnames = [
             ('könig.idn.pythontest.net',
-             'könig.idn.pythontest.net',),
+             'xn--knig-5qa.idn.pythontest.net'),
             ('xn--knig-5qa.idn.pythontest.net',
              'xn--knig-5qa.idn.pythontest.net'),
             (b'xn--knig-5qa.idn.pythontest.net',
-             b'xn--knig-5qa.idn.pythontest.net'),
+             'xn--knig-5qa.idn.pythontest.net'),
 
             ('königsgäßchen.idna2003.pythontest.net',
-             'königsgäßchen.idna2003.pythontest.net'),
+             'xn--knigsgsschen-lcb0w.idna2003.pythontest.net'),
             ('xn--knigsgsschen-lcb0w.idna2003.pythontest.net',
              'xn--knigsgsschen-lcb0w.idna2003.pythontest.net'),
             (b'xn--knigsgsschen-lcb0w.idna2003.pythontest.net',
-             b'xn--knigsgsschen-lcb0w.idna2003.pythontest.net'),
+             'xn--knigsgsschen-lcb0w.idna2003.pythontest.net'),
+
+            # ('königsgäßchen.idna2008.pythontest.net',
+            #  'xn--knigsgchen-b4a3dun.idna2008.pythontest.net'),
+            ('xn--knigsgchen-b4a3dun.idna2008.pythontest.net',
+             'xn--knigsgchen-b4a3dun.idna2008.pythontest.net'),
+            (b'xn--knigsgchen-b4a3dun.idna2008.pythontest.net',
+             'xn--knigsgchen-b4a3dun.idna2008.pythontest.net'),
+
         ]
         for server_hostname, expected_hostname in idn_hostnames:
             server = ThreadedEchoServer(context=server_context, chatty=True)
@@ -2675,16 +2674,6 @@ def test_check_hostname_idn(self):
                     s.getpeercert()
                     self.assertEqual(s.server_hostname, expected_hostname)
 
-        # bug https://bugs.python.org/issue28414
-        # IDNA 2008 deviations are broken
-        idna2008 = 'xn--knigsgchen-b4a3dun.idna2008.pythontest.net'
-        server = ThreadedEchoServer(context=server_context, chatty=True)
-        with server:
-            with self.assertRaises(UnicodeError):
-                with context.wrap_socket(socket.socket(),
-                                         server_hostname=idna2008) as s:
-                    s.connect((HOST, server.port))
-
         # incorrect hostname should raise an exception
         server = ThreadedEchoServer(context=server_context, chatty=True)
         with server: