Merge pull request #499 from sigmavirus24/deprecation-warning-when-no-subjectAltName

Issue warning when certificate has no subjectAltName.

7 files changed, 108 insertions, 3 deletions

diff --git a/dummyserver/certs/README.rst b/dummyserver/certs/README.rst
new file mode 100644
index 00000000..6a16823f
--- /dev/null
+++ b/dummyserver/certs/README.rst
@@ -0,0 +1,24 @@
+Creating a new CRT
+------------------
+
+(Instructions lifted from Heroku_)
+
+1. Generate a new CSR::
+   
+   openssl req -new -key server.key -out server.new.csr
+
+1. Generate a new CRT::
+
+   openssl x509 -req -in server.new.csr -signkey server.key -out server.new.crt
+
+Creating a new PEM file with your new CRT
+-----------------------------------------
+
+1. Concatenate the ``crt`` and ``key`` files into one::
+
+   cat server.new.crt server.key > cacert.new.pem
+
+
+:Last Modified: 1 Nov 2014
+
+.. _Heroku: https://devcenter.heroku.com/articles/ssl-certificate-self
diff --git a/dummyserver/certs/cacert.no_san.pem b/dummyserver/certs/cacert.no_san.pem
new file mode 100644
index 00000000..4d23a1bd
--- /dev/null
+++ b/dummyserver/certs/cacert.no_san.pem
@@ -0,0 +1,28 @@
+-----BEGIN CERTIFICATE-----
+MIIB8zCCAVwCCQCFaGO2yYcGEzANBgkqhkiG9w0BAQUFADA+MQswCQYDVQQGEwJV
+UzEbMBkGA1UEChMSdXJsbGliMyBkZXZlbG9wZXJzMRIwEAYDVQQDEwlsb2NhbGhv
+c3QwHhcNMTQxMTAxMjAxODIyWhcNMTQxMjAxMjAxODIyWjA+MQswCQYDVQQGEwJV
+UzEbMBkGA1UEChMSdXJsbGliMyBkZXZlbG9wZXJzMRIwEAYDVQQDEwlsb2NhbGhv
+c3QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBANd7cWqYJa8/xc/Gq1P7Rt8v
+VO/O95uLjqTRYhxRXxs/e/JiJJFcb0AubLNkfgYdj1RWzg4QOQIwM4FJ7MHTL1qL
+/12hM1xqJy10QG7yd91cqhO3Ouf5VfhQ+uQfmGbs1DV9NC6PUxHvIEKZio3dDZD4
+R+66PLHQXAHMC7LVopi7AgMBAAEwDQYJKoZIhvcNAQEFBQADgYEARvNwipUQGT/1
+JCuykx5NckpGXFcVbrhXqOJ36jHD5z8sjgnHWWOokt8QwlG962GyLHDkNTRKmMyI
+JFF5JBjTKaFHyrSEhvum8USXLyWA0CvRH04xm1vOVDrl5f/w66+HoDnizjLvFExS
+QUI9AXdl8GZBRu1m1bSxaKd0o/rjNEA=
+-----END CERTIFICATE-----
+-----BEGIN RSA PRIVATE KEY-----
+MIICXgIBAAKBgQDXe3FqmCWvP8XPxqtT+0bfL1Tvzvebi46k0WIcUV8bP3vyYiSR
+XG9ALmyzZH4GHY9UVs4OEDkCMDOBSezB0y9ai/9doTNcaictdEBu8nfdXKoTtzrn
++VX4UPrkH5hm7NQ1fTQuj1MR7yBCmYqN3Q2Q+Efuujyx0FwBzAuy1aKYuwIDAQAB
+AoGBANOGBM6bbhq7ImYU4qf8+RQrdVg2tc9Fzo+yTnn30sF/rx8/AiCDOV4qdGAh
+HKjKKaGj2H/rotqoEFcxBy05LrgJXxydBP72e9PYhNgKOcSmCQu4yALIPEXfKuIM
+zgAErHVJ2l79fif3D4hzNyz+u5E1A9n3FG9cgaJSiYP8IG2RAkEA82GZ8rBkSGQQ
+ZQ3oFuzPAAL21lbj8D0p76fsCpvS7427DtZDOjhOIKZmaeykpv+qSzRraqEqjDRi
+S4kjQvwh6QJBAOKniZ+NDo2lSpbOFk+XlmABK1DormVpj8KebHEZYok1lRI+WiX9
+Nnoe9YLgix7++6H5SBBCcTB4HvM+5A4BuwMCQQChcX/eZbXP81iQwB3Rfzp8xnqY
+icDf7qKvz9Ma4myU7Y5E9EpaB1mD/P14jDpYcMW050vNyqTfpiwB8TFL0NZpAkEA
+02jkFH9UyMgZV6qo4tqI98l/ZrtyF8OrxSNSEPhVkZf6EQc5vN9/lc8Uv1vESEgb
+3AwRrKDcxRH2BHtv6qSwkwJAGjqnkIcEkA75r1e55/EF2chcZW1+tpwKupE8CtAH
+VXGd5DVwt4cYWkLUj2gF2fJbV97uu2MAg5CFDb+vQ6p5eA==
+-----END RSA PRIVATE KEY-----
diff --git a/dummyserver/certs/server.no_san.crt b/dummyserver/certs/server.no_san.crt
new file mode 100644
index 00000000..135a020f
--- /dev/null
+++ b/dummyserver/certs/server.no_san.crt
@@ -0,0 +1,13 @@
+-----BEGIN CERTIFICATE-----
+MIIB8zCCAVwCCQCFaGO2yYcGEzANBgkqhkiG9w0BAQUFADA+MQswCQYDVQQGEwJV
+UzEbMBkGA1UEChMSdXJsbGliMyBkZXZlbG9wZXJzMRIwEAYDVQQDEwlsb2NhbGhv
+c3QwHhcNMTQxMTAxMjAxODIyWhcNMTQxMjAxMjAxODIyWjA+MQswCQYDVQQGEwJV
+UzEbMBkGA1UEChMSdXJsbGliMyBkZXZlbG9wZXJzMRIwEAYDVQQDEwlsb2NhbGhv
+c3QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBANd7cWqYJa8/xc/Gq1P7Rt8v
+VO/O95uLjqTRYhxRXxs/e/JiJJFcb0AubLNkfgYdj1RWzg4QOQIwM4FJ7MHTL1qL
+/12hM1xqJy10QG7yd91cqhO3Ouf5VfhQ+uQfmGbs1DV9NC6PUxHvIEKZio3dDZD4
+R+66PLHQXAHMC7LVopi7AgMBAAEwDQYJKoZIhvcNAQEFBQADgYEARvNwipUQGT/1
+JCuykx5NckpGXFcVbrhXqOJ36jHD5z8sjgnHWWOokt8QwlG962GyLHDkNTRKmMyI
+JFF5JBjTKaFHyrSEhvum8USXLyWA0CvRH04xm1vOVDrl5f/w66+HoDnizjLvFExS
+QUI9AXdl8GZBRu1m1bSxaKd0o/rjNEA=
+-----END CERTIFICATE-----
diff --git a/dummyserver/certs/server.no_san.csr b/dummyserver/certs/server.no_san.csr
new file mode 100644
index 00000000..98a17e91
--- /dev/null
+++ b/dummyserver/certs/server.no_san.csr
@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE REQUEST-----
+MIIBlzCCAQACAQAwPjELMAkGA1UEBhMCVVMxGzAZBgNVBAoTEnVybGxpYjMgZGV2
+ZWxvcGVyczESMBAGA1UEAxMJbG9jYWxob3N0MIGfMA0GCSqGSIb3DQEBAQUAA4GN
+ADCBiQKBgQDXe3FqmCWvP8XPxqtT+0bfL1Tvzvebi46k0WIcUV8bP3vyYiSRXG9A
+LmyzZH4GHY9UVs4OEDkCMDOBSezB0y9ai/9doTNcaictdEBu8nfdXKoTtzrn+VX4
+UPrkH5hm7NQ1fTQuj1MR7yBCmYqN3Q2Q+Efuujyx0FwBzAuy1aKYuwIDAQABoBkw
+FwYJKoZIhvcNAQkHMQoTCHBhc3N3b3JkMA0GCSqGSIb3DQEBBQUAA4GBAKqeoXDe
+gJZG8/SQSoZnqLer/5Xtq1KkapgZUcedIlaDRsvbNa2nvoSbWuZmqZgi5BAnZxMO
+IzAGt3hER5L8ueBpGvG5UMWdwzrDSnwCDO0EaiWhZ20JwAazlH8vrQ7oaW/7LB+j
+Oz6SgsW5FcV8ucQVb5bchtK0CixntusrT7H/
+-----END CERTIFICATE REQUEST-----
diff --git a/dummyserver/server.py b/dummyserver/server.py
index 99f08358..fe3f7caf 100755
--- a/dummyserver/server.py
+++ b/dummyserver/server.py
@@ -28,8 +28,13 @@ DEFAULT_CERTS = {
     'certfile': os.path.join(CERTS_PATH, 'server.crt'),
     'keyfile': os.path.join(CERTS_PATH, 'server.key'),
 }
+NO_SAN_CERTS = {
+    'certfile': os.path.join(CERTS_PATH, 'server.no_san.crt'),
+    'keyfile': DEFAULT_CERTS['keyfile']
+}
 DEFAULT_CA = os.path.join(CERTS_PATH, 'cacert.pem')
 DEFAULT_CA_BAD = os.path.join(CERTS_PATH, 'client_bad.pem')
+NO_SAN_CA = os.path.join(CERTS_PATH, 'cacert.no_san.pem')
 
 
 # Different types of servers we have:
diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py
index 082bd4dd..12fa9308 100644
--- a/test/with_dummyserver/test_https.py
+++ b/test/with_dummyserver/test_https.py
@@ -9,7 +9,8 @@ import mock
 from nose.plugins.skip import SkipTest
 
 from dummyserver.testcase import HTTPSDummyServerTestCase
-from dummyserver.server import DEFAULT_CA, DEFAULT_CA_BAD, DEFAULT_CERTS
+from dummyserver.server import (DEFAULT_CA, DEFAULT_CA_BAD, DEFAULT_CERTS,
+                                NO_SAN_CERTS, NO_SAN_CA)
 
 from test import (
     onlyPy26OrOlder,
@@ -370,5 +371,20 @@ class TestHTTPS_TLSv1(HTTPSDummyServerTestCase):
         self.assertRaises(SSLError, self._pool.request, 'GET', '/')
 
 
+class TestHTTPS_NoSAN(HTTPSDummyServerTestCase):
+    certs = NO_SAN_CERTS
+
+    def test_warning_for_certs_without_a_san(self):
+        """Ensure that a warning is raised when the cert from the server has
+        no Subject Alternative Name."""
+        with mock.patch('warnings.warn') as warn:
+            https_pool = HTTPSConnectionPool(self.host, self.port,
+                                             cert_reqs='CERT_REQUIRED',
+                                             ca_certs=NO_SAN_CA)
+            r = https_pool.request('GET', '/')
+            self.assertEqual(r.status, 200)
+            self.assertTrue(warn.called)
+
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/urllib3/connection.py b/urllib3/connection.py
index cebdd867..e5de769d 100644
--- a/urllib3/connection.py
+++ b/urllib3/connection.py
@@ -38,6 +38,7 @@ except NameError:  # Python 2:
 from .exceptions import (
     ConnectTimeoutError,
     SystemTimeWarning,
+    SecurityWarning,
 )
 from .packages.ssl_match_hostname import match_hostname
 
@@ -241,8 +242,15 @@ class VerifiedHTTPSConnection(HTTPSConnection):
                                self.assert_fingerprint)
         elif resolved_cert_reqs != ssl.CERT_NONE \
                 and self.assert_hostname is not False:
-            match_hostname(self.sock.getpeercert(),
-                           self.assert_hostname or hostname)
+            cert = self.sock.getpeercert()
+            if not cert.get('subjectAltName', ()):
+                warnings.warn((
+                    'Certificate has no `subjectAltName`, falling back to check for a `commonName` for now. '
+                    'This feature is being removed by major browsers and deprecated by RFC 2818. '
+                    '(See https://github.com/shazow/urllib3/issues/497 for details.)'),
+                    SecurityWarning
+                )
+            match_hostname(cert, self.assert_hostname or hostname)
 
         self.is_verified = (resolved_cert_reqs == ssl.CERT_REQUIRED
                             or self.assert_fingerprint is not None)