axTLS-based modussl: ussl.wrap_socket silently accepts invalid certificates
Investigating https://github.com/micropython/micropython-lib/issues/69, I found the current SSL/TLS socket creation code at https://github.com/micropython/micropython/blob/d19e4f0ba4df487b4ebd36b5fe6a16e68c0afe77/extmod/modussl.c#L49
If I'm reading that correctly:
- Wrapping a socket without providing any certificate verification details results in no verification being performed;
- Even if verification details are provided, they're still ignored
This makes the documentation at http://docs.micropython.org/en/latest/library/ussl.html#ussl.ssl.wrap_socket thoroughly misleading, as even if the additional arguments are passed in, they won't be processed.
I realise actually implementing this will require a significant amount of work, so my request at this point would be for passing in unsupported arguments to result in a hard failure, rather than silently appearing to succeed without actually providing the claimed security guarantees.
Request for package: micropython-ssl
Would you be open to adding a micropython-ssl package that wraps ussl, similarly to how micropython-socket wraps usocket? I can think of two use cases for this, as discussed below. I'd be happy to contribute the code for this, but I want to make sure it's something you'd be receptive to first, especially considering that it would be a new package.
Use case 1: Drop default arguments
According to the docs for most (or maybe all) ports, the signature for wrap_socket is ssl.wrap_socket(sock, server_side=False, keyfile=None, certfile=None, cert_reqs=CERT_NONE, ca_certs=None), with the caveat that
Depending on the underlying module implementation for a particular board, some or all keyword arguments above may be not supported.
Indeed, most ports (it seems) do not support nearly so many of these. On Unix, for example, even the keyfile and certfile arguments are not accepted.
Note though, that even ussl.wrap_socket(s, keyfile=None) does not work, even though it should be equivalent to ussl.wrap_socket(s). The proposed micropython-ssl package could provide an ssl.wrap_socket function that drops keyword arguments whose values are the default (i.e. ssl.wrap_socket(s, keyfile=None) results in a call to ussl.wrap_socket(s)).
This would be useful for user-defined libraries that accept a keyfile parameter from their callers and pass this to wrap_socket. (It would provide better compatibility across MicroPython ports and with CPython, without growing MicroPython proper.)
Use case 2: Allow SSL-wrapping of micropython-socket sockets
Due to micropython/micropython#2749, and the fact that socket.socket is a subclass of usocket.socket, micropython-socket sockets cannot be passed to ussl.wrap_socket (attempts to do so cause a crash, for now). This makes the micropython-socket package almost entirely unusable with SSL.
If micropython-socket were modified, though, to wrap instances of usocket.socket, rather than subclassing it, then the proposed micropython-ssl package could provide a wrap_socket function that retrieves the underlying usocket.socket instance from any passed-in socket.sockets before forwarding to ussl.
Perhaps there's a better solution, but this is the only one I can think of besides fixing it in MicroPython proper, as was said here. That might take a while though; even in CPython subclassing native objects has limitations. This solution would affect the result of issubclass(socket.socket, usocket.socket), but I doubt anyone's relying on that (just a guess, though).
Maintainers: What say you? Would this be useful?
Somehow I missed this ticket, sorry. Lack of "micropython-ssl" is indeed an oversight, I added dummy implementation of it in ea60d9d71f11b66c7a2d67c9ca54061e459bcc1c. I'll review your other points as time permits, sorry for the further delays.
No worries about delays; I have other things on my list right now too. Since you've added the ssl package, I might submit a pull request implementing "Use case 1", as I described above, since that seems pretty straightforward and uncontroversial.
In the mean time, I'll keep an eye out for your comments. Thanks.
So, the aim of micropython-lib is to implement as complete as possible Python stdlib, so in general, this work is welcome. One thing to keep in mind, also in general, is that at last time, there's growing interest to use more of micropython-lib on baremetal targets, the outcome being that the idea that it'll run only on unix, in "unlimited" resources, no longer should be counted on. Nothing of that directly applies to the proposal above, just general things to keep in mind.
So, "Use case 1" indeed can be implemented seamlessly, and I don't see big problems with proposed plan for "Use case 2", just everything needs to be done step by step.
Thanks for looking into that btw!
+1 for supporting bare metal ports. It also helps to highlight inconsistencies between ports of MicroPython proper.
As for doing everything step by step, I'm with you there too.
Pull request incoming for "Use case 1", described above. Have not worked yet on Use case 2.
As it has been a few years with no activity I would like to validate if Use case 2 is still in need for a solution ?
If not then I propose to close this issue.
Since commit 35d41dbb0e4acf1518f520220d405ebe2db257d6 the
sslmodule is now implemented in pure Python in this repo. It uses the underlying MicroPython-specifictlsmodule for its implementation.