Skip to content
This repository has been archived by the owner on Feb 19, 2020. It is now read-only.

Python >=3.4 TLS refactoring does not use self.certfile and self.keyfile, breaks mutual TLS #460

Open
aalba6675 opened this issue Aug 20, 2017 · 6 comments

Comments

@aalba6675
Copy link

With refactoring of TLS code for Python >=3.4 we don't load certfile and keyfile anymore

in xmlstream/xmlstream.py: XMLStream._create_secure_socket : in code path for python >=3.4 are we mssing ctx.load_cert_chain() somewhere? I don't see where we use self.certfile and self.keyfile in the >=3.4 code path.

I could not find the use of SSLContext.load_cert_chain() anywhere.

This breaks mutual TLS.

@woz5999
Copy link

woz5999 commented Sep 6, 2017

i seem to be encountering the same issue

@aalba6675
Copy link
Author

@woz5999 Does this work for you? WFM — If it does I will do a PR.

diff --git a/sleekxmpp/xmlstream/xmlstream.py b/sleekxmpp/xmlstream/xmlstream.py
index 061438c..2e0c2b6 100644
--- a/sleekxmpp/xmlstream/xmlstream.py
+++ b/sleekxmpp/xmlstream/xmlstream.py
@@ -470,7 +470,12 @@ class XMLStream(object):
                     ctx.check_hostname = False
                     ctx.verify_mode = ssl.CERT_NONE
                 elif cert_policy == ssl.CERT_REQUIRED:
+                    ctx.check_hostname = True
                     ctx.load_verify_locations(cafile=self.ca_certs)
+
+                if self.certfile:
+                    ctx.load_cert_chain(certfile = self.certfile,
+                                        keyfile = getattr(self, 'keyfile', None))
             else:
                 # Oops, create_default_context() is not supported.
                 if self.ssl_version == ssl.PROTOCOL_SSLv3:
@@ -517,7 +522,10 @@ class XMLStream(object):
         if ctx:
             if self.ciphers:
                 ctx.set_ciphers(self.ciphers)
-            return ctx.wrap_socket(self.socket, do_handshake_on_connect=False)
+            kwargs = {'do_handshake_on_connect': True}
+            if ctx.check_hostname:
+                kwargs['server_hostname'] = self.server_hostname
+            return ctx.wrap_socket(self.socket, **kwargs)
         else:
             if self.ciphers and sys.version_info >= (2, 7):
                 ssl_args['ciphers'] = self.ciphers

@woz5999
Copy link

woz5999 commented Sep 6, 2017

i'm out of the office for a few days. I'll try next week and let you know.

@woz5999
Copy link

woz5999 commented Sep 12, 2017

We're using this package as part of a docker image and were able to work around the issue by reverting to the previous version. Given that we have a functional workaround, it's not really worth the time and effort it would take to implement the instrumentation to test this change.

Sorry I couldn't be more helpful.

@Neustradamus
Copy link

Have you tested 1.3.2?

@Neustradamus
Copy link

@aalba6675 @woz5999: Any news?

Have you tested with "master"?

It works?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants