Testing this change, it no longer works because verify_mode defaults to CERT_REQUIRED... I guess that's secure, but we really need to make this work like it did before. So this needs to do ctx.verify_mode = CERT_NONE before wrapping the socket.

Similarly with the umqtt and urllib changes in this PR.

I'll change the behavior for requests and urllib, as we don't ship a CA list by default we can't enable ssl verification without the user choosing it. (Previously, using the ssl library, with the legacy ssl.wrap_socket did disable verification). For umqtt, I've adjusted the constructor to not take a boolean parameter for ssl anymore, but an sslcontext instead. So if someone wants to enable insecure ssl, they need to disable verification in the ssl context themselves (or load ca certificates or install a cert callback).

Ideally we should have a way to do that for urllib and requests as well. Looking at the documentation of their python counterpart, urllib has a capath parameter, and requests has a verify parameter, to point to a ca file. I'm not sure how strictly to cpython subset we adhere here, but I would like to implement parameters to allow passing a ca certificate (by bytes object and not necessarily as file) for both modules.

Oh, yes, umqtt now passes in the SSLContext, so no need to change the verify_mode for that.

I'm not sure how strictly to cpython subset we adhere here, but I would like to implement parameters to allow passing a ca certificate for both modules.

Yes, that would be a good addition, allowing to pass in certs. Ideally it would follow the CPython way. Of course that's for a separate PR!

This seems now like a "breaking" change, one needs to create and pass a SSLContext, see e.g. https://github.com/orgs/micropython/discussions/13624, I think this should be something like

if self.ssl:
 if hasattr(self.ssl, "wrap_socket"):
 self.sock = self.ssl.wrap_socket(self.sock, **self.ssl_params)
 else:
 import ssl as _ssl
 self.sock = _ssl.wrap_socket(self.sock, **self.ssl_params)

Yes, I was not aware that breaking changes here are not possible. I would argue that this breaking change is good here.

I think this breaking change is a fair change to make. We need to improve and make progress on things and sometimes it's not practical to retain backwards compatibility on everything.

In this case using the new library with old user code will raise an exception if the ssl argument is used. So it's easy for the user to know that things need to be changed.

And the new scheme matches better how SSL works in asyncio, passing in an SSLContext.

In the end a user can just keep using an old version of umqtt if needed.

Then document this somewhere in the umqtt's README or next release info should be fine 👍🏼