← Back to team overview

txaws-dev team mailing list archive

[Merge] lp:~therve/txaws/ssl-verify into lp:txaws

 

Thomas Herve has proposed merging lp:~therve/txaws/ssl-verify into lp:txaws.

Requested reviews:
  txAWS Developers (txaws-dev)
Related bugs:
  Bug #781949 in txAWS: "Must check certificates for validity"
  https://bugs.launchpad.net/txaws/+bug/781949

For more details, see:
https://code.launchpad.net/~therve/txaws/ssl-verify/+merge/83740

The branch implements optional SSL hostname verification, set by changing the endpoint passed to the client (for backward compatibility reasons).
-- 
https://code.launchpad.net/~therve/txaws/ssl-verify/+merge/83740
Your team txAWS Developers is requested to review the proposed merge of lp:~therve/txaws/ssl-verify into lp:txaws.
=== modified file 'txaws/client/base.py'
--- txaws/client/base.py	2011-04-21 21:16:37 +0000
+++ txaws/client/base.py	2011-11-29 08:38:01 +0000
@@ -3,7 +3,7 @@
 except ImportError:
     from xml.parsers.expat import ExpatError as ParseError
 
-from twisted.internet import reactor, ssl
+from twisted.internet.ssl import ClientContextFactory
 from twisted.web import http
 from twisted.web.client import HTTPClientFactory
 from twisted.web.error import Error as TwistedWebError
@@ -12,6 +12,7 @@
 from txaws.credentials import AWSCredentials
 from txaws.exception import AWSResponseParseError
 from txaws.service import AWSServiceEndpoint
+from txaws.client.ssl import VerifyingContextFactory
 
 
 def error_wrapper(error, errorClass):
@@ -73,13 +74,16 @@
 
 class BaseQuery(object):
 
-    def __init__(self, action=None, creds=None, endpoint=None):
+    def __init__(self, action=None, creds=None, endpoint=None, reactor=None):
         if not action:
             raise TypeError("The query requires an action parameter.")
         self.factory = HTTPClientFactory
         self.action = action
         self.creds = creds
         self.endpoint = endpoint
+        if reactor is None:
+            from twisted.internet import reactor
+        self.reactor = reactor
         self.client = None
 
     def get_page(self, url, *args, **kwds):
@@ -92,11 +96,14 @@
         contextFactory = None
         scheme, host, port, path = parse(url)
         self.client = self.factory(url, *args, **kwds)
-        if scheme == 'https':
-            contextFactory = ssl.ClientContextFactory()
-            reactor.connectSSL(host, port, self.client, contextFactory)
+        if scheme == "https":
+            if self.endpoint.ssl_hostname_verification:
+                contextFactory = VerifyingContextFactory(host)
+            else:
+                contextFactory = ClientContextFactory()
+            self.reactor.connectSSL(host, port, self.client, contextFactory)
         else:
-            reactor.connectTCP(host, port, self.client)
+            self.reactor.connectTCP(host, port, self.client)
         return self.client.deferred
 
     def get_request_headers(self, *args, **kwds):

=== added file 'txaws/client/ssl.py'
--- txaws/client/ssl.py	1970-01-01 00:00:00 +0000
+++ txaws/client/ssl.py	2011-11-29 08:38:01 +0000
@@ -0,0 +1,74 @@
+from glob import glob
+import os
+import re
+
+from OpenSSL import SSL
+from OpenSSL.crypto import load_certificate, FILETYPE_PEM
+
+from twisted.internet.ssl import CertificateOptions
+
+
+__all__ = ["VerifyingContextFactory", "get_ca_certs"]
+
+
+class VerifyingContextFactory(CertificateOptions):
+    """
+    A SSL context factory to pass to C{connectSSL} to check for hostname
+    validity.
+    """
+
+    def __init__(self, host, caCerts=None):
+        if caCerts is None:
+            caCerts = get_global_ca_certs()
+        CertificateOptions.__init__(self, verify=True, caCerts=caCerts)
+        self.host = host
+
+    def verify_callback(self, connection, x509, errno, depth, preverifyOK):
+        # Only check depth == 0 on chained certificates.
+        if depth == 0:
+            commonName = x509.get_subject().commonName
+            if commonName is None:
+                return False
+            # The commonName might contain a wildcard, so turn it into a
+            # regex for checking.
+            commonName_re = "^%s$" % commonName.replace(
+                ".", "\.").replace("*", "[^.]*")
+            if not re.search(commonName_re, self.host, re.I):
+                return False
+        return preverifyOK
+
+    def _makeContext(self):
+        context = CertificateOptions._makeContext(self)
+        context.set_verify(
+            SSL.VERIFY_PEER | SSL.VERIFY_FAIL_IF_NO_PEER_CERT,
+            self.verify_callback)
+        return context
+
+
+def get_ca_certs(files="/etc/ssl/certs/*.pem"):
+    """Retrieve a list of CAs pointed by C{files}."""
+    certificateAuthorityMap = {}
+    for certFileName in glob(files):
+        # There might be some dead symlinks in there, so let's make sure it's
+        # real.
+        if not os.path.exists(certFileName):
+            continue
+        certFile = open(certFileName)
+        data = certFile.read()
+        certFile.close()
+        x509 = load_certificate(FILETYPE_PEM, data)
+        digest = x509.digest("sha1")
+        # Now, de-duplicate in case the same cert has multiple names.
+        certificateAuthorityMap[digest] = x509
+    return certificateAuthorityMap.values()
+
+
+_ca_certs = None
+
+
+def get_global_ca_certs():
+    """Retrieve a singleton of CA certificates."""
+    global _ca_certs
+    if _ca_certs is None:
+        _ca_certs = get_ca_certs()
+    return _ca_certs

=== added file 'txaws/client/tests/badprivate.ssl'
--- txaws/client/tests/badprivate.ssl	1970-01-01 00:00:00 +0000
+++ txaws/client/tests/badprivate.ssl	2011-11-29 08:38:01 +0000
@@ -0,0 +1,15 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIICXgIBAAKBgQDGYFWP2Ine2OFIPjX+Tu+S403KW63EWq/I1DYXiezLoUpYPed3
+0tAkAXH1gOwQZbARFlUn0LgvXDSpuQLvgKQZwP/e1D8SvZUZ6nexW+aYlPE9kjd1
+dhK1xpe1h5y09AjCz02xxzcFzrJrJ47uU7vV+FGArE8FFh3hO+dz0/PmZQIDAQAB
+AoGBAKfv+983yJfgcO9QwzLULlrilQNfk36r6y6QAG7y84T7uU10spSs4kno80mL
+58yF2YTNrC91scdePrMEDikldUVcCqtPYcZKHyw5+4aGaDDO244tznexOQnQcNIe
+2BbLFuh+jmJpoFIY/H7EsLQQzn6+6dGPnYGBQfiyitWfAXRNAkEA/ShQkYCRAHgq
+g6WBIYsw/ISQydhiMiKrL2ZUXERT+pWU9MoSdMskgyMi3S7wzwJQXkrHA36q8QkL
++H8n5K+f5wJBAMiajfEtv0wRW0awX40qJtuqW3cSKeGHBH9mMObcRJd5OcK6giC/
+Cc5st/ZcuE/8i4r44DfeC+cwY6QdIqI8rdMCQQCKuq78LWJIyZEyt12+ThK4LsVR
+d1zIcKsyvHb6YQ9MQPBx/NKEYlZN7tFKOFEKgBAevAe3aJCwqe5/bN8luQB9AkEA
+uQVD8bR+AgzoIPS/zJWaLXSc09/e3PIJBfAdHnD+mq7mxWH8b3OD+e5wZjvyi2Ok
+2NLfCug0FlGdNVrh/Lz2nQJATdcNvHNzJcWOHe05lo+xAqkjz73FWGpPNrdXRigG
+YnjIsZVy4k48xIxPhT2rC44yo1iPEP5EnHCE2bLyUlTAYA==
+-----END RSA PRIVATE KEY-----

=== added file 'txaws/client/tests/badpublic.ssl'
--- txaws/client/tests/badpublic.ssl	1970-01-01 00:00:00 +0000
+++ txaws/client/tests/badpublic.ssl	2011-11-29 08:38:01 +0000
@@ -0,0 +1,23 @@
+-----BEGIN CERTIFICATE-----
+MIIDzjCCAzegAwIBAgIJANqT3vXxSVFjMA0GCSqGSIb3DQEBBQUAMIGhMQswCQYD
+VQQGEwJCUjEPMA0GA1UECBMGUGFyYW5hMREwDwYDVQQHEwhDdXJpdGliYTEhMB8G
+A1UEChMYRmFrZSBMYW5kc2NhcGUgKFRlc3RpbmcpMREwDwYDVQQLEwhTZWN1cml0
+eTESMBAGA1UEAxMJbG9jYWxob3N0MSQwIgYJKoZIhvcNAQkBFhVhbmRyZWFzQGNh
+bm9uaWNhbC5jb20wHhcNMDkwMTA5MTUyNTAwWhcNMTkwMTA3MTUyNTAwWjCBoTEL
+MAkGA1UEBhMCQlIxDzANBgNVBAgTBlBhcmFuYTERMA8GA1UEBxMIQ3VyaXRpYmEx
+ITAfBgNVBAoTGEZha2UgTGFuZHNjYXBlIChUZXN0aW5nKTERMA8GA1UECxMIU2Vj
+dXJpdHkxEjAQBgNVBAMTCWxvY2FsaG9zdDEkMCIGCSqGSIb3DQEJARYVYW5kcmVh
+c0BjYW5vbmljYWwuY29tMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDGYFWP
+2Ine2OFIPjX+Tu+S403KW63EWq/I1DYXiezLoUpYPed30tAkAXH1gOwQZbARFlUn
+0LgvXDSpuQLvgKQZwP/e1D8SvZUZ6nexW+aYlPE9kjd1dhK1xpe1h5y09AjCz02x
+xzcFzrJrJ47uU7vV+FGArE8FFh3hO+dz0/PmZQIDAQABo4IBCjCCAQYwHQYDVR0O
+BBYEFF4A8+YHCLAt19OtWTjIjBKzLUokMIHWBgNVHSMEgc4wgcuAFF4A8+YHCLAt
+19OtWTjIjBKzLUokoYGnpIGkMIGhMQswCQYDVQQGEwJCUjEPMA0GA1UECBMGUGFy
+YW5hMREwDwYDVQQHEwhDdXJpdGliYTEhMB8GA1UEChMYRmFrZSBMYW5kc2NhcGUg
+KFRlc3RpbmcpMREwDwYDVQQLEwhTZWN1cml0eTESMBAGA1UEAxMJbG9jYWxob3N0
+MSQwIgYJKoZIhvcNAQkBFhVhbmRyZWFzQGNhbm9uaWNhbC5jb22CCQDak9718UlR
+YzAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBBQUAA4GBABszkA3CCzt+nTOX+A7/
+I98DvI0W1Ss0J+Tq+diLr+kw6Z5ZTj5hrIS/x6XhVHjpim4724UBXA0Sels4JXbw
+hhJovuncExce316gAol/9eEzTffZ9mt1jZQy9LL7IAENiobnsj2F65zNaJzXp5UC
+rE/h/xIxz9rAmXtVOWHqZLcw
+-----END CERTIFICATE-----

=== added file 'txaws/client/tests/private.ssl'
--- txaws/client/tests/private.ssl	1970-01-01 00:00:00 +0000
+++ txaws/client/tests/private.ssl	2011-11-29 08:38:01 +0000
@@ -0,0 +1,15 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIICWwIBAAKBgQDX2VNEDZHtl5nimNocshar8pBmjqiGn9olCR2LcKifuJY4bFTg
+qib+Rr3v2DwDTbOMaquRSxFgwLJLCug3WclsGrYSPIsFCx+k3XhqM61JXEwrKuIp
+Js893XHkeg3SEFua/oVfDxNfJttoHW3FbsnDx5964kYwGExjJcH73GInUQIDAQAB
+AoGASiM9NEys6Lx/gJMbp2uL2fdwnak2PTc+iCX/XduOL34JKswawyfuSLwnlO/i
+fQf9OaeR0k/EYkUNeDUA2bIfOj6wWS8tamnX4fxL7A20y5VyqMMah8mcerZgtPdS
+7ZtYCbeijWSKpHgjALc2Hym7R68WZI+IHe0DQkcW6WxOMFkCQQD2jqHZn/Qtd62u
+mWVwIx6G7+Po5vzd86KyWWftdUtVCY9DmiX1rmWXbJhLnmaKCLkmHxyBvw7Biarr
+ZnCAafebAkEA4B2dSpLi7bAzjCa7JBtzV9kr1FVZOl2vA+9BqTAjCQu0b9VDEm8V
+x0061Z8rN7Og3ECGtKH/r3/4RnHUPpwJgwJAdyZQkvHYt4xJc8IPolRmcUFGu4u9
+Eammq1fHgJqZcBvxjvLUe1jvIXFKW+jNltFGYGTSiuUAxYi4/49+uJ/9FwJAGBB1
+/DTrcvQxhMH/5C+iYfNqtmD3tMGscjK1jTIjAOyl0kBG9GrDHuRXBesSW+fIxP2U
+uT6P0std4EqGrLZaewJAHT0n/3tXnsPjj+BMlC4ZkRKgPJ4I7zTU1XSlLY5zbMoV
+NvtHLlq7ttiarsH95xyge69uV1/zJVj/IiS71YY9PQ==
+-----END RSA PRIVATE KEY-----

=== added file 'txaws/client/tests/public.ssl'
--- txaws/client/tests/public.ssl	1970-01-01 00:00:00 +0000
+++ txaws/client/tests/public.ssl	2011-11-29 08:38:01 +0000
@@ -0,0 +1,22 @@
+-----BEGIN CERTIFICATE-----
+MIIDnDCCAwWgAwIBAgIJALPjWsknBC15MA0GCSqGSIb3DQEBBQUAMIGRMQswCQYD
+VQQGEwJCUjEPMA0GA1UECBMGUGFyYW5hMREwDwYDVQQHEwhDdXJpdGliYTESMBAG
+A1UEChMJTGFuZHNjYXBlMRAwDgYDVQQLEwdUZXN0aW5nMRIwEAYDVQQDEwlsb2Nh
+bGhvc3QxJDAiBgkqhkiG9w0BCQEWFWFuZHJlYXNAY2Fub25pY2FsLmNvbTAeFw0w
+OTAxMDgxNjQxMzlaFw0xOTAxMDYxNjQxMzlaMIGRMQswCQYDVQQGEwJCUjEPMA0G
+A1UECBMGUGFyYW5hMREwDwYDVQQHEwhDdXJpdGliYTESMBAGA1UEChMJTGFuZHNj
+YXBlMRAwDgYDVQQLEwdUZXN0aW5nMRIwEAYDVQQDEwlsb2NhbGhvc3QxJDAiBgkq
+hkiG9w0BCQEWFWFuZHJlYXNAY2Fub25pY2FsLmNvbTCBnzANBgkqhkiG9w0BAQEF
+AAOBjQAwgYkCgYEA19lTRA2R7ZeZ4pjaHLIWq/KQZo6ohp/aJQkdi3Con7iWOGxU
+4Kom/ka979g8A02zjGqrkUsRYMCySwroN1nJbBq2EjyLBQsfpN14ajOtSVxMKyri
+KSbPPd1x5HoN0hBbmv6FXw8TXybbaB1txW7Jw8efeuJGMBhMYyXB+9xiJ1ECAwEA
+AaOB+TCB9jAdBgNVHQ4EFgQU3eUz2XxK1J/oavkn/hAvYfGOZM0wgcYGA1UdIwSB
+vjCBu4AU3eUz2XxK1J/oavkn/hAvYfGOZM2hgZekgZQwgZExCzAJBgNVBAYTAkJS
+MQ8wDQYDVQQIEwZQYXJhbmExETAPBgNVBAcTCEN1cml0aWJhMRIwEAYDVQQKEwlM
+YW5kc2NhcGUxEDAOBgNVBAsTB1Rlc3RpbmcxEjAQBgNVBAMTCWxvY2FsaG9zdDEk
+MCIGCSqGSIb3DQEJARYVYW5kcmVhc0BjYW5vbmljYWwuY29tggkAs+NayScELXkw
+DAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQUFAAOBgQBZQcqhHAsasX3WtCXlIKqH
+hE4ZdsvtPHOnoWPxxN4CZEyu2YJ2PMXCkA7yISNokAZgkOpkYGPWwV3CwNCw032u
++ngwIo2sxx7ag8tVrYkIda717oBw7opDMVrjTNhZdak7s+hg+s9ZDPUMMcbJFtlN
+lmayn/uZSyog4Y+yriB1tQ==
+-----END CERTIFICATE-----

=== modified file 'txaws/client/tests/test_client.py'
--- txaws/client/tests/test_client.py	2011-04-21 21:16:37 +0000
+++ txaws/client/tests/test_client.py	2011-11-29 08:38:01 +0000
@@ -1,6 +1,10 @@
 import os
 
+from OpenSSL.crypto import load_certificate, FILETYPE_PEM
+from OpenSSL.SSL import Error as SSLError
+
 from twisted.internet import reactor
+from twisted.internet.ssl import DefaultOpenSSLContextFactory
 from twisted.internet.error import ConnectionRefusedError
 from twisted.protocols.policies import WrappingFactory
 from twisted.python import log
@@ -11,9 +15,21 @@
 from twisted.web.error import Error as TwistedWebError
 
 from txaws.client.base import BaseClient, BaseQuery, error_wrapper
+from txaws.client.ssl import VerifyingContextFactory
+from txaws.service import AWSServiceEndpoint
 from txaws.testing.base import TXAWSTestCase
 
 
+def sibpath(path):
+    return os.path.join(os.path.dirname(__file__), path)
+
+
+PRIVKEY = sibpath("private.ssl")
+PUBKEY = sibpath("public.ssl")
+BADPRIVKEY = sibpath("badprivate.ssl")
+BADPUBKEY = sibpath("badpublic.ssl")
+
+
 class ErrorWrapperTestCase(TXAWSTestCase):
 
     def test_204_no_content(self):
@@ -148,3 +164,112 @@
         d = query.get_page(self._get_url("file"))
         d.addCallback(query.get_response_headers)
         return d.addCallback(check_results)
+
+    def test_ssl_hostname_verification(self):
+        """
+        If the endpoint passed to L{BaseQuery} has C{ssl_hostname_verification}
+        sets to C{True}, a L{VerifyingContextFactory} is passed to
+        C{connectSSL}.
+        """
+
+        class FakeReactor(object):
+
+            def __init__(self):
+                self.connects = []
+
+            def connectSSL(self, host, port, client, factory):
+                self.connects.append((host, port, client, factory))
+
+        fake_reactor = FakeReactor()
+        endpoint = AWSServiceEndpoint(ssl_hostname_verification=True)
+        query = BaseQuery("an action", "creds", endpoint, fake_reactor)
+        query.get_page("https://example.com/file";)
+        [(host, port, client, factory)] = fake_reactor.connects
+        self.assertEqual("example.com", host)
+        self.assertEqual(443, port)
+        self.assertTrue(isinstance(factory, VerifyingContextFactory))
+        self.assertEqual("example.com", factory.host)
+        self.assertNotEqual([], factory.caCerts)
+
+
+class BaseQuerySSLTestCase(TXAWSTestCase):
+
+    def setUp(self):
+        self.cleanupServerConnections = 0
+        name = self.mktemp()
+        os.mkdir(name)
+        FilePath(name).child("file").setContent("0123456789")
+        r = static.File(name)
+        self.site = server.Site(r, timeout=None)
+        self.wrapper = WrappingFactory(self.site)
+        from txaws.client import ssl
+        pub_key = file(PUBKEY)
+        pub_key_data = pub_key.read()
+        pub_key.close()
+        ssl._ca_certs = [load_certificate(FILETYPE_PEM, pub_key_data)]
+
+    def tearDown(self):
+        from txaws.client import ssl
+        ssl._ca_certs = None
+        # If the test indicated it might leave some server-side connections
+        # around, clean them up.
+        connections = self.wrapper.protocols.keys()
+        # If there are fewer server-side connections than requested,
+        # that's okay.  Some might have noticed that the client closed
+        # the connection and cleaned up after themselves.
+        for n in range(min(len(connections), self.cleanupServerConnections)):
+            proto = connections.pop()
+            log.msg("Closing %r" % (proto,))
+            proto.transport.loseConnection()
+        if connections:
+            log.msg("Some left-over connections; this test is probably buggy.")
+        return self.port.stopListening()
+
+    def _get_url(self, path):
+        return "https://localhost:%d/%s"; % (self.portno, path)
+
+    def test_ssl_verification_positive(self):
+        """
+        The L{VerifyingContextFactory} properly allows to connect to the
+        endpoint if the certificates match.
+        """
+        context_factory = DefaultOpenSSLContextFactory(PRIVKEY, PUBKEY)
+        self.port = reactor.listenSSL(
+            0, self.site, context_factory, interface="127.0.0.1")
+        self.portno = self.port.getHost().port
+
+        endpoint = AWSServiceEndpoint(ssl_hostname_verification=True)
+        query = BaseQuery("an action", "creds", endpoint)
+        d = query.get_page(self._get_url("file"))
+        return d.addCallback(self.assertEquals, "0123456789")
+
+    def test_ssl_verification_negative(self):
+        """
+        The L{VerifyingContextFactory} fails with a SSL error the certificates
+        can't be checked.
+        """
+        context_factory = DefaultOpenSSLContextFactory(BADPRIVKEY, BADPUBKEY)
+        self.port = reactor.listenSSL(
+            0, self.site, context_factory, interface="127.0.0.1")
+        self.portno = self.port.getHost().port
+
+        endpoint = AWSServiceEndpoint(ssl_hostname_verification=True)
+        query = BaseQuery("an action", "creds", endpoint)
+        d = query.get_page(self._get_url("file"))
+        return self.assertFailure(d, SSLError)
+
+    def test_ssl_verification_bypassed(self):
+        """
+        L{BaseQuery} doesn't use L{VerifyingContextFactory}
+        if C{ssl_hostname_verification} is C{False}, thus allowing to connect
+        to non-secure endpoints.
+        """
+        context_factory = DefaultOpenSSLContextFactory(BADPRIVKEY, BADPUBKEY)
+        self.port = reactor.listenSSL(
+            0, self.site, context_factory, interface="127.0.0.1")
+        self.portno = self.port.getHost().port
+
+        endpoint = AWSServiceEndpoint(ssl_hostname_verification=False)
+        query = BaseQuery("an action", "creds", endpoint)
+        d = query.get_page(self._get_url("file"))
+        return d.addCallback(self.assertEquals, "0123456789")

=== modified file 'txaws/service.py'
--- txaws/service.py	2011-08-11 23:10:54 +0000
+++ txaws/service.py	2011-11-29 08:38:01 +0000
@@ -19,13 +19,16 @@
     """
     @param uri: The URL for the service.
     @param method: The HTTP method used when accessing a service.
+    @param ssl_hostname_verification: Whether or not SSL hotname verification
+        will be done when connecting to the endpoint.
     """
 
-    def __init__(self, uri="", method="GET"):
+    def __init__(self, uri="", method="GET", ssl_hostname_verification=False):
         self.host = ""
         self.port = None
         self.path = "/"
         self.method = method
+        self.ssl_hostname_verification = ssl_hostname_verification
         self._parse_uri(uri)
         if not self.scheme:
             self.scheme = "http"

=== modified file 'txaws/version.py'
--- txaws/version.py	2009-11-19 18:46:16 +0000
+++ txaws/version.py	2011-11-29 08:38:01 +0000
@@ -1,3 +1,3 @@
-txaws = "0.0.1"
+txaws = "0.2.2"
 ec2_api = "2008-12-01"
 s3_api = "2006-03-01"