Avoid SSLError: Cannot create a client socket with a PROTOCOL_TLS_SERVER context

[hroncok]

When we build mailman3 in Fedora with Python 3.10.0rc2, we see the following problem:

Traceback (most recent call last):
  File "/builddir/build/BUILD/mailman-3.3.4/src/mailman/testing/layers.py", line 297, in setUp
    cls.smtpd.start()
  File "/builddir/build/BUILD/mailman-3.3.4/src/mailman/testing/mta.py", line 177, in start
    super().start()
  File "/usr/lib/python3.10/site-packages/aiosmtpd/controller.py", line 288, in start
    self._trigger_server()
  File "/usr/lib/python3.10/site-packages/aiosmtpd/controller.py", line 481, in _trigger_server
    InetMixin._trigger_server(self)
  File "/usr/lib/python3.10/site-packages/aiosmtpd/controller.py", line 428, in _trigger_server
    s = stk.enter_context(self.ssl_context.wrap_socket(s))
  File "/usr/lib64/python3.10/ssl.py", line 512, in wrap_socket
    return self.sslsocket_class._create(
  File "/usr/lib64/python3.10/ssl.py", line 1061, in _create
    self._sslobj = self._context._wrap_socket(
ssl.SSLError: Cannot create a client socket with a PROTOCOL_TLS_SERVER context (_ssl.c:801)

This makes the problem go away.

Disclaimer: I have no idea what I'm doing here.

What do these changes do?

They check if the context we are about to wrap to is not server context.

Are there changes in behavior for the user?

I hope not.

Related issue number

Related to https://github.com/aio-libs/aiosmtpd/issues/277

Checklist

• [x] I think the code is well written
• [x] Unit tests for the changes exist (the original tests suite)
• [ ] tox testenvs have been executed in the following environments:
  • [ ] Linux (Ubuntu 18.04, Ubuntu 20.04, Arch): {py36,py37,py38,py39}-{nocov,cov,diffcov}, qa, docs
  • [ ] Windows (7, 10): {py36,py37,py38,py39}-{nocov,cov,diffcov}
  • [ ] WSL 1.0 (Ubuntu 18.04): {py36,py37,py38,py39}-{nocov,cov,diffcov}, pypy3-{nocov,cov}, qa, docs
  • [ ] FreeBSD (12.2, 12.1, 11.4): {py36,pypy3}-{nocov,cov,diffcov}, qa
  • [ ] Cygwin: py36-{nocov,cov,diffcov}, qa, docs
• [ ] Documentation reflects the changes
• [ ] Add a news fragment into the NEWS.rst file
  • Add under the "aiosmtpd-next" section, creating one if necessary
    • You may create subsections to group the changes, if you like
  • Use full sentences with correct case and punctuation
  • Refer to relevant Issue if applicable

I am sorry but I won't be able to finish this checklist. Maybe my change is good, maybe not, but it helped me in mailman3 in Fedora.

[waynew]

I'm wondering if this is an issue in mailman3 - if they incorrectly created an SSL context - or used the wrong SSL context.

[hroncok]

That is certainly possible. However the tests of aiosmtpd before this patch:

____________________ ERROR at setup of TestSMTPS.test_smtps ____________________
get_controller = <function get_controller.<locals>.getter at 0x7fff9b3363b0>
ssl_context_server = <ssl.SSLContext object at 0x7fff9ac43240>
    @pytest.fixture
    def ssl_controller(
        get_controller, ssl_context_server
    ) -> Generator[Controller, None, None]:
        handler = ReceivingHandler()
        controller = get_controller(handler, ssl_context=ssl_context_server)
>       controller.start()
aiosmtpd/tests/test_smtps.py:25: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
aiosmtpd/controller.py:288: in start
    self._trigger_server()
aiosmtpd/controller.py:481: in _trigger_server
    InetMixin._trigger_server(self)
aiosmtpd/controller.py:428: in _trigger_server
    s = stk.enter_context(self.ssl_context.wrap_socket(s))
/usr/lib64/python3.10/ssl.py:512: in wrap_socket
    return self.sslsocket_class._create(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
cls = <class 'ssl.SSLSocket'>
sock = <socket.socket [closed] fd=-1, family=AddressFamily.AF_INET6, type=SocketKind.SOCK_STREAM, proto=6>
server_side = False, do_handshake_on_connect = True, suppress_ragged_eofs = True
server_hostname = None, context = <ssl.SSLContext object at 0x7fff9ac43240>
session = None
    @classmethod
    def _create(cls, sock, server_side=False, do_handshake_on_connect=True,
                suppress_ragged_eofs=True, server_hostname=None,
                context=None, session=None):
        if sock.getsockopt(SOL_SOCKET, SO_TYPE) != SOCK_STREAM:
            raise NotImplementedError("only stream sockets are supported")
        if server_side:
            if server_hostname:
                raise ValueError("server_hostname can only be specified "
                                 "in client mode")
            if session is not None:
                raise ValueError("session can only be specified in "
                                 "client mode")
        if context.check_hostname and not server_hostname:
            raise ValueError("check_hostname requires server_hostname")
    
        kwargs = dict(
            family=sock.family, type=sock.type, proto=sock.proto,
            fileno=sock.fileno()
        )
        self = cls.__new__(cls, **kwargs)
        super(SSLSocket, self).__init__(**kwargs)
        self.settimeout(sock.gettimeout())
        sock.detach()
    
        self._context = context
        self._session = session
        self._closed = False
        self._sslobj = None
        self.server_side = server_side
        self.server_hostname = context._encode_hostname(server_hostname)
        self.do_handshake_on_connect = do_handshake_on_connect
        self.suppress_ragged_eofs = suppress_ragged_eofs
    
        # See if we are connected
        try:
            self.getpeername()
        except OSError as e:
            if e.errno != errno.ENOTCONN:
                raise
            connected = False
        else:
            connected = True
    
        self._connected = connected
        if connected:
            # create the SSL object
            try:
>               self._sslobj = self._context._wrap_socket(
                    self, server_side, self.server_hostname,
                    owner=self, session=self._session,
                )
E               ssl.SSLError: Cannot create a client socket with a PROTOCOL_TLS_SERVER context (_ssl.c:801)
/usr/lib64/python3.10/ssl.py:1061: SSLError
------------------------------ Captured log setup ------------------------------
INFO     mail.log:smtp.py:407 Available AUTH mechanisms: LOGIN(builtin) PLAIN(builtin)

______________ TestUnixSocketController.test_server_creation_ssl _______________
self = <aiosmtpd.tests.test_server.TestUnixSocketController object at 0x7fff9ba75120>
safe_socket_dir = PosixPath('/tmp/tmpwj9xuk1w')
ssl_context_server = <ssl.SSLContext object at 0x7fff9aed7740>
    def test_server_creation_ssl(self, safe_socket_dir, ssl_context_server):
        sockfile = safe_socket_dir / "smtp"
        cont = UnixSocketController(
            Sink(), unix_socket=sockfile, ssl_context=ssl_context_server
        )
        try:
>           cont.start()
aiosmtpd/tests/test_server.py:383: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
aiosmtpd/controller.py:288: in start
    self._trigger_server()
aiosmtpd/controller.py:493: in _trigger_server
    UnixSocketMixin._trigger_server(self)
aiosmtpd/controller.py:470: in _trigger_server
    s = stk.enter_context(self.ssl_context.wrap_socket(s))
/usr/lib64/python3.10/ssl.py:512: in wrap_socket
    return self.sslsocket_class._create(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
cls = <class 'ssl.SSLSocket'>
sock = <socket.socket [closed] fd=-1, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0>
server_side = False, do_handshake_on_connect = True, suppress_ragged_eofs = True
server_hostname = None, context = <ssl.SSLContext object at 0x7fff9aed7740>
session = None
    @classmethod
    def _create(cls, sock, server_side=False, do_handshake_on_connect=True,
                suppress_ragged_eofs=True, server_hostname=None,
                context=None, session=None):
        if sock.getsockopt(SOL_SOCKET, SO_TYPE) != SOCK_STREAM:
            raise NotImplementedError("only stream sockets are supported")
        if server_side:
            if server_hostname:
                raise ValueError("server_hostname can only be specified "
                                 "in client mode")
            if session is not None:
                raise ValueError("session can only be specified in "
                                 "client mode")
        if context.check_hostname and not server_hostname:
            raise ValueError("check_hostname requires server_hostname")
    
        kwargs = dict(
            family=sock.family, type=sock.type, proto=sock.proto,
            fileno=sock.fileno()
        )
        self = cls.__new__(cls, **kwargs)
        super(SSLSocket, self).__init__(**kwargs)
        self.settimeout(sock.gettimeout())
        sock.detach()
    
        self._context = context
        self._session = session
        self._closed = False
        self._sslobj = None
        self.server_side = server_side
        self.server_hostname = context._encode_hostname(server_hostname)
        self.do_handshake_on_connect = do_handshake_on_connect
        self.suppress_ragged_eofs = suppress_ragged_eofs
    
        # See if we are connected
        try:
            self.getpeername()
        except OSError as e:
            if e.errno != errno.ENOTCONN:
                raise
            connected = False
        else:
            connected = True
    
        self._connected = connected
        if connected:
            # create the SSL object
            try:
>               self._sslobj = self._context._wrap_socket(
                    self, server_side, self.server_hostname,
                    owner=self, session=self._session,
                )
E               ssl.SSLError: Cannot create a client socket with a PROTOCOL_TLS_SERVER context (_ssl.c:801)
/usr/lib64/python3.10/ssl.py:1061: SSLError
------------------------------ Captured log call -------------------------------
INFO     mail.log:smtp.py:407 Available AUTH mechanisms: LOGIN(builtin) PLAIN(builtin)

And with this patch, we only see the TestUnixSocketController.test_server_creation_ssl failure, not the error.

In fact, before this patch:

 = 3 failed, 531 passed, 2 skipped, 126 warnings, 27 errors in 186.11s (0:03:06) =

After this patch:

=========== 3 failed, 558 passed, 2 skipped, 126 warnings in 55.14s ============

[waynew]

@hroncok I'm not positive, but based on this comment I don't think this is necessary.

Would you be able to take a look this week? We're aiming to get v1.5 released ~ mid/late December with some of these assorted fixes, especially the ones that enable support for the newer versions of Python.

[hroncok]

Would you be able to take a look this week?

Sorry, I will not be able to look into this.

[waynew]

No worries, thanks for letting me know!

[pepoluan]

Do the PRs #309 and #314 (which had been merged) also fix this?

I tested GitHub CI with all possible Python combos and they are all green (except PyPy3.8 coverage -- but tests are successful).

Can we close this?

[pepoluan]

@hroncok can you verify that 1.4.3 solves this problem?

If so we can close this PR without merging.

[hroncok]

Looks like we still patch this in Fedora by using this PR. I can try removing the patch and updating the package early January.

[pepoluan]

Looks like we still patch this in Fedora by using this PR. I can try removing the patch and updating the package early January.

Okay.

I'm close to releasing 1.4.4 though, that one will fix some DeprecationWarnings.

So in January, if possible tell us if 1.4.4 is good enough, and if it is, we can close this without merging.

[hroncok]

I can no longer reproduce the problem with 1.4.3 and recent Python 3.10.

[pepoluan]

I can no longer reproduce the problem with 1.4.3 and recent Python 3.10.

Awesome!

Thanks for verifying 👍🏼