Blob Blame History Raw
From a85c6968b2c9ac652a66460ea4ed200268555761 Mon Sep 17 00:00:00 2001
From: Thomas Leaman <thomas.leaman@hp.com>
Date: Tue, 18 Jun 2013 15:34:45 +0000
Subject: [PATCH] Fix SSL certificate CNAME checking

Currently, accessing a host via ip address will pass SSL verification;
the CNAME is not checked as intended as part of verify_callback.

'preverify_ok is True' will always return false (int/bool comparison).
preverify_ok will be 1 if preverification has passed.

Fixes bug 1192229

Change-Id: Ib651548ab4289295a9b92ee039b2aff2d08aba5f
(cherry picked from commit 822cd64c0718b46a065abbb8709f6b466d12e708)
---
 glanceclient/common/http.py |  4 ++-
 tests/test_ssl.py           | 61 +++++++++++++++++++++++++++++++++++++++------
 2 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/glanceclient/common/http.py b/glanceclient/common/http.py
index 7146ace..4401e82 100644
--- a/glanceclient/common/http.py
+++ b/glanceclient/common/http.py
@@ -317,11 +317,13 @@ class VerifiedHTTPSConnection(httplib.HTTPSConnection):
 
     def verify_callback(self, connection, x509, errnum,
                         depth, preverify_ok):
+        # NOTE(leaman): preverify_ok may be a non-boolean type
+        preverify_ok = bool(preverify_ok)
         if x509.has_expired():
             msg = "SSL Certificate expired on '%s'" % x509.get_notAfter()
             raise exc.SSLCertificateError(msg)
 
-        if depth == 0 and preverify_ok is True:
+        if depth == 0 and preverify_ok:
             # We verify that the host matches against the last
             # certificate in the chain
             return self.host_matches_cert(self.host, x509)
diff --git a/tests/test_ssl.py b/tests/test_ssl.py
index 8ee179f..feb165c 100644
--- a/tests/test_ssl.py
+++ b/tests/test_ssl.py
@@ -125,8 +125,8 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
         self.assertEqual(cert.get_subject().commonName, '0.0.0.0')
         try:
             conn = http.VerifiedHTTPSConnection('0.0.0.0', 0)
-            conn.verify_callback(None, cert, 0, 0, True)
-        except:
+            conn.verify_callback(None, cert, 0, 0, 1)
+        except Exception:
             self.fail('Unexpected exception.')
 
     def test_ssl_cert_subject_alt_name(self):
@@ -140,14 +140,14 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
         self.assertEqual(cert.get_subject().commonName, '0.0.0.0')
         try:
             conn = http.VerifiedHTTPSConnection('alt1.example.com', 0)
-            conn.verify_callback(None, cert, 0, 0, True)
-        except:
+            conn.verify_callback(None, cert, 0, 0, 1)
+        except Exception:
             self.fail('Unexpected exception.')
 
         try:
             conn = http.VerifiedHTTPSConnection('alt2.example.com', 0)
-            conn.verify_callback(None, cert, 0, 0, True)
-        except:
+            conn.verify_callback(None, cert, 0, 0, 1)
+        except Exception:
             self.fail('Unexpected exception.')
 
     def test_ssl_cert_mismatch(self):
@@ -165,7 +165,7 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
             self.fail('Failed to init VerifiedHTTPSConnection.')
 
         self.assertRaises(exc.SSLCertificateError,
-                          conn.verify_callback, None, cert, 0, 0, True)
+                          conn.verify_callback, None, cert, 0, 0, 1)
 
     def test_ssl_expired_cert(self):
         """
@@ -183,4 +183,49 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
             self.fail('Failed to init VerifiedHTTPSConnection.')
 
         self.assertRaises(exc.SSLCertificateError,
-                          conn.verify_callback, None, cert, 0, 0, True)
+                          conn.verify_callback, None, cert, 0, 0, 1)
+
+    def test_ssl_broken_key_file(self):
+        """
+        Test verify exception is raised.
+        """
+        cert_file = os.path.join(TEST_VAR_DIR, 'certificate.crt')
+        cacert = os.path.join(TEST_VAR_DIR, 'ca.crt')
+        key_file = 'fake.key'
+        self.assertRaises(
+            exc.SSLConfigurationError,
+            http.VerifiedHTTPSConnection, '127.0.0.1',
+            0, key_file=key_file,
+            cert_file=cert_file, cacert=cacert)
+
+    def test_ssl_init_ok_with_insecure_true(self):
+        """
+        Test VerifiedHTTPSConnection class init
+        """
+        key_file = os.path.join(TEST_VAR_DIR, 'privatekey.key')
+        cert_file = os.path.join(TEST_VAR_DIR, 'certificate.crt')
+        cacert = os.path.join(TEST_VAR_DIR, 'ca.crt')
+        try:
+            conn = http.VerifiedHTTPSConnection(
+                '127.0.0.1', 0,
+                key_file=key_file,
+                cert_file=cert_file,
+                cacert=cacert, insecure=True)
+        except exc.SSLConfigurationError:
+            self.fail('Failed to init VerifiedHTTPSConnection.')
+
+    def test_ssl_init_ok_with_ssl_compression_false(self):
+        """
+        Test VerifiedHTTPSConnection class init
+        """
+        key_file = os.path.join(TEST_VAR_DIR, 'privatekey.key')
+        cert_file = os.path.join(TEST_VAR_DIR, 'certificate.crt')
+        cacert = os.path.join(TEST_VAR_DIR, 'ca.crt')
+        try:
+            conn = http.VerifiedHTTPSConnection(
+                '127.0.0.1', 0,
+                key_file=key_file,
+                cert_file=cert_file,
+                cacert=cacert, ssl_compression=False)
+        except exc.SSLConfigurationError:
+            self.fail('Failed to init VerifiedHTTPSConnection.')