Fixes AttributeError: module 'ssl' has no attribute 'wrap_socket' #70
Conversation
|
I've updated PR disabling certificate checking to maintain backward compability |
|
@gdt I`m going to create a new issue to ask for an explicit and configurable way to enable/disable certificate checking, but I think this fix should be enough to get the library work with latest python versions maintaining previous behavior. |
|
Fair enough to match previous behavior. My comment was mainly aimed at "As a reviewer, it should be clear whether this is a change or not". Secondarily, I think defaulting to not validating is a bug. |
|
@gdt I think that defaulting to not validating is no ideal (security matters) but necessary if we want to maintain previous behavior because when creating a default context and using context's wrap_socket always did certificate validation (since Python 2.7.x for some x < 20 IIRC). But the xmpppy library uses the top-level wrap_socket function, which got removed in Python 3.12; that one never did certificate validation. So the way I understand it, with Python < 3.12 xmpppy doesn't seem to validate certificates, and with Python >= 3.12 it doesn't work if you use TLS (because the function it uses was removed). |
There was a problem hiding this comment.
The change looks correct to me. I agree that enabling certificate validation by default would be better, but this didn't happen before.
This PR might cause problems with very early 2.7.x releases, since the ssl module changed a lot for the first couple of x. This has been a looong time ago, though, so I think it's OK to ignore that :)
|
Please, anyone could merge the proposed patch and release a new version with this fixed? More than one year with the solution ready... |
|
@juanvalino , I feel you and thanks for your patch. An alternative, when security is not a concern (like in a lab) and until this PR is merged, is to use If I were to use this library in a "real" context (not in a lab), I would probably prefer to be able to control specifically which security flag is ignored. |
| @@ -386,6 +386,7 @@ def _startSSL(self): | |||
| tcpsock=self._owner.Connection | |||
| context=ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) | |||
| context.check_hostname = False | |||
| context.verify_mode = ssl.CERT_NONE | |||
There was a problem hiding this comment.
I have applied the diff from this pull request, more or less, to pkgsrc. I am not meaning to ask you to change the PR, but rather just adding comments in case they are helpful to you or to others.
I left checking enabled, because I think it's a bug for TLS not to validate. But, it always failed. I then added
context.set_default_verify_paths()
and that let the python ssl implementation find the system trust anchors on both NetBSD and macOS.
While trying various things, I think I found that check_hostname = False without setting verify_mode = ssl.CERT_NONE threw an error.
But, with my patch (largely taken from yours):
--- xmpp/transports.py.orig 2022-04-10 18:25:44.000000000 +0000
+++ xmpp/transports.py
@@ -383,8 +383,14 @@ class TLS(PlugIn):
def _startSSL(self):
""" Immidiatedly switch socket to TLS mode. Used internally."""
""" Here we should switch pending_data to hint mode."""
+ context=ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
+ context.set_default_verify_paths()
+ # Uncomment if you need to work around trust anchor configuration.
+ #context.check_hostname = False
+ #context.verify_mode = ssl.CERT_NONE
+
tcpsock=self._owner.Connection
- tcpsock._sslObj = ssl.wrap_socket(tcpsock._sock, None, None)
+ tcpsock._sslObj = context.wrap_socket(tcpsock._sock, server_hostname=self._owner.Server)
tcpsock._sslIssuer = tcpsock._sslObj.getpeercert().get('issuer')
tcpsock._sslServer = tcpsock._sslObj.getpeercert().get('server')
tcpsock._recv = tcpsock._sslObj.read
I am able to use xmpppy to send jabber messages to a server that requires TLS.
There was a problem hiding this comment.
If you want to enable certificate checking, better create context with ssl.create_default_context() (https://docs.python.org/3/library/ssl.html#ssl.create_default_context), and don't manually call set_default_verify_paths and set check_hostname and verify_mode.
There was a problem hiding this comment.
Thanks - I have changed pkgsrc as you suggest.
Fixes #69