Merge pull request scrapy#3166 from lucywang000/catch-tls-certificate…

…-error catch CertificateError in tls verification
yashrsharma44 · Mar 14, 2018 · 6cc6bbb · 6cc6bbb
2 parents 6a7cdf9 + 2c58da1
commit 6cc6bbb
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 3 deletions.
diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py
@@ -40,6 +40,14 @@
     from twisted.internet._sslverify import (ClientTLSOptions,
                                              verifyHostname,
                                              VerificationError)
+    try:
+        # XXX: this import would fail on Debian jessie with system installed
+        # service_identity library, due to lack of cryptography.x509 dependency
+        # See https://github.com/pyca/service_identity/issues/21
+        from service_identity.exceptions import CertificateError
+        verification_errors = (CertificateError, VerificationError)
+    except ImportError:
+        verification_errors = VerificationError
 
     if twisted_version < (17, 0, 0):
         from twisted.internet._sslverify import _maybeSetHostNameIndication
@@ -55,8 +63,9 @@ class ScrapyClientTLSOptions(ClientTLSOptions):
         (for genuinely invalid certificates or bugs in verification code).
 
         Same as Twisted's private _sslverify.ClientTLSOptions,
-        except that VerificationError and ValueError exceptions are caught,
-        so that the connection is not closed, only logging warnings.
+        except that VerificationError, CertificateError and ValueError
+        exceptions are caught, so that the connection is not closed, only
+        logging warnings.
         """
 
         def _identityVerifyingInfoCallback(self, connection, where, ret):
@@ -65,7 +74,7 @@ def _identityVerifyingInfoCallback(self, connection, where, ret):
             elif where & SSL_CB_HANDSHAKE_DONE:
                 try:
                     verifyHostname(connection, self._hostnameASCII)
-                except VerificationError as e:
+                except verification_errors as e:
                     logger.warning(
                         'Remote certificate is not valid for hostname "{}"; {}'.format(
                             self._hostnameASCII, e))

diff --git a/tests/keys/localhost-ip.gen.README b/tests/keys/localhost-ip.gen.README
@@ -0,0 +1,21 @@
+$ openssl req -x509 -sha256 -nodes -newkey rsa:2048 -days 365 -keyout localhost.key -out localhost.crt
+Generating a 2048 bit RSA private key
+...................................................................................................+++
+.....+++
+writing new private key to 'localhost.key'
+-----
+You are about to be asked to enter information that will be incorporated
+into your certificate request.
+What you are about to enter is what is called a Distinguished Name or a DN.
+There are quite a few fields but you can leave some blank
+For some fields there will be a default value,
+If you enter '.', the field will be left blank.
+-----
+Country Name (2 letter code) [AU]:IE
+State or Province Name (full name) [Some-State]:.
+Locality Name (eg, city) []:.
+Organization Name (eg, company) [Internet Widgits Pty Ltd]:Scrapy
+Organizational Unit Name (eg, section) []:.
+Common Name (e.g. server FQDN or YOUR name) []:127.0.0.1
+Email Address []:.
+
diff --git a/tests/keys/localhost.ip.crt b/tests/keys/localhost.ip.crt
@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDNzCCAh+gAwIBAgIJAKAIhM4nA8W7MA0GCSqGSIb3DQEBCwUAMDIxCzAJBgNV
+BAYTAklFMQ8wDQYDVQQKDAZTY3JhcHkxEjAQBgNVBAMMCTEyNy4wLjAuMTAeFw0x
+ODAzMTIxNDMyMjlaFw0xOTAzMTIxNDMyMjlaMDIxCzAJBgNVBAYTAklFMQ8wDQYD
+VQQKDAZTY3JhcHkxEjAQBgNVBAMMCTEyNy4wLjAuMTCCASIwDQYJKoZIhvcNAQEB
+BQADggEPADCCAQoCggEBAK7Vzr+zdsbAEej6D8XFBS5frHnfmqSivQS/zrRZcSVL
+JgPwHJSRMyVCNvlpRV4ulu7I6zTY0ItzeAJPiH/euSokM8AkM87y9GAugljVtuev
+y0uKLUfznPvPZxfYzaB7lyQtU9E6AF8Amtuta8eb7rdqsuqjRopKp3pIheBAfvjV
+ewkMlxz3xcKZHs8T3UWdceWftLEZJSi13FHe/uoohRBiXVn/6DvycBjk1TC+zNpR
+v8mSm+uqcYoG8/CFZ/r1T2EveBH4jZjNReIlM9zFwVHjtjAdunSdMLVY59kBGNE4
+JqxjJ021W2XqoW4VFf6XrIdg8ai4NxHDpWO4blOoMbcCAwEAAaNQME4wHQYDVR0O
+BBYEFBZWEo9+kkTjdGxJdvRNGyhpWfjMMB8GA1UdIwQYMBaAFBZWEo9+kkTjdGxJ
+dvRNGyhpWfjMMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAGjMcuVr
+idLmbuu/Krxmqnebt0zPLgJXg1ACUEto7110mmEK3jsZg/brdLf74PP+FUa6B/ZP
+8+FJCgF1KZLc3tS9w2OVRSdz+uZ2WYgN6R7uJiVs77BiD6TR6wRrEicRsS6Cq90X
+kNVhqExG4cDr8wGLiCGNfVfFwea7wGhF2zCohF82u1mAgqR/1obas0ils5fh+soJ
+FmTd5A9vCbRpZRXost9J7Z4LCj86MYATgyH9bZp7aN6NJ2nI4uKgeafDFT83c5Vb
+smQ/R0HeP5oylIhpmWWliNjT+XPONPIPDWgQgeFBBofX/vuv82KXz1ZBYfqpArgO
+zh6AcsnjkLumOkM=
+-----END CERTIFICATE-----
diff --git a/tests/keys/localhost.ip.key b/tests/keys/localhost.ip.key
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQCu1c6/s3bGwBHo
++g/FxQUuX6x535qkor0Ev860WXElSyYD8ByUkTMlQjb5aUVeLpbuyOs02NCLc3gC
+T4h/3rkqJDPAJDPO8vRgLoJY1bbnr8tLii1H85z7z2cX2M2ge5ckLVPROgBfAJrb
+rWvHm+63arLqo0aKSqd6SIXgQH741XsJDJcc98XCmR7PE91FnXHln7SxGSUotdxR
+3v7qKIUQYl1Z/+g78nAY5NUwvszaUb/JkpvrqnGKBvPwhWf69U9hL3gR+I2YzUXi
+JTPcxcFR47YwHbp0nTC1WOfZARjROCasYydNtVtl6qFuFRX+l6yHYPGouDcRw6Vj
+uG5TqDG3AgMBAAECggEBAKaLO0g3j3SicC0rT60IEfhr4OOzkh80erQ0dpYsAXES
+FeN4bfFEI6FhYvbRRegCn3pVYGDWDEpasz4YPyH3qxEurTFiCwwfOZUJmNdAtdwc
+BJ8vwBSjRq5EkqMPvkkakg4/M3HCO6pD7EBJAbuCmbKU7FxBLqf7l3AP9594MLud
+JE1zkioK8tz6auBq4qLwDUNJhqv7eug1CKEpfArA9ZqW3orWg21+Octac8R82ZyD
+bt+Veh0vWd16MkcSX574vydqYzNiseY70yNjBRxHLD+/HA8BvWn7M6d0ULuEN1UT
+ojm+NAMc65ms3MkXksdUeDQ3eFIF9M4+/rTRU8gHeAECgYEA487ERT3/qEDMezYx
+KcUkLE2VwqqnW0+Sfd6fzOG+VGqeYgHG/d9sjo1RsJR/D/ZgzO3oeJ4lgov3HN5N
+yfPIGyJfYd7p9WWml4AiWvj3YVg5V4vmwnDs7LBxHU60bLClgvMQx4iSZ4q4QrXA
+hRLBDrJuNGvuLUqFb6jar8BtVbcCgYEAxHjORgNxsBfzuAs0ZfvVyTYai4f+92U+
+32tPxghpI4gHnQnz7MbUccJGy+SR23N8DLNJv8K+LbVm7UNIdsy6d5b9vazkYIie
+PyS3ynRO3vgIL3NbMC2cc+uc2dL2n/FnMA8nrdZMTgXukmnCn8tzSLphoZBu7SaY
+r9938XE8BAECgYEAmuXzCun3Nl6pK3ZTw4Uq7Xzrwevr0+itQSzpF5S/qAK/IwD2
+X5VV6TAqRZkTNLVgaLe0BJ/z/WpSYqy90/4RKHIczR2Xk6bEuesEcTssamJkyyRz
+ie7jCqWGpFjp0aXjRMElvacddY4bcDDJcTKpVub4jGh/EQjE5oG4AR0kus0CgYBZ
+Eed56C/PRFySUEoV/gCisquAHExjvfut8Al/XurDV/UTpaJ28oD3fbr4zoutcIKJ
+g3JoxBHRyQ57e+hLK29RrhsktU/nz6fmOnA0EVx8SvfzAxoREmx+RQ+b1L9ILXm5
+WPWFIsT/DkNlDxtTtDl0fEKsqz0OuFO6T9YhmFM8AQKBgCFn6FV8AdzLBtdKrPT+
+inQASBr264pb5lp7g9JdBmaQZ3McrQ35VOA3ZfhyTAMhYtY1wk0xp8+fW1bV325u
+BiLdJ/gAocPBRlw7rS0rq1+U1+zAQCgxutrm2aRQd1qEUrCRvCtCyIeuUntshHAz
+m1Q+9xJdtRxlYc1YGTK1YGCq
+-----END PRIVATE KEY-----
diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py
@@ -505,6 +505,19 @@ def setUp(self):
         super(Https11InvalidDNSId, self).setUp()
         self.host = '127.0.0.1'
 
+class Https11InvalidDNSPattern(Https11TestCase):
+    """Connect to HTTPS hosts where the certificate are issued to an ip instead of a domain."""
+
+    keyfile = 'keys/localhost.ip.key'
+    certfile = 'keys/localhost.ip.crt'
+
+    def setUp(self):
+        try:
+            from service_identity.exceptions import CertificateError
+        except ImportError:
+            raise unittest.SkipTest("cryptography lib is too old")
+        super(Https11InvalidDNSPattern, self).setUp()
+
 
 class Http11MockServerTestCase(unittest.TestCase):
     """HTTP 1.1 test case with MockServer"""