From 53b22fa2ee2be22cf3f005ead06eacdc6dc87fe8 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Mon, 6 Jun 2016 17:12:59 +0100 Subject: [PATCH 01/12] Issue 249: Only initialise the global SSL context if one wasn't manually specified when creating the connection. Fixes exceptions if the cert.pem file included with the hyper package isn't included in a distribution (such as a generated exe) --- hyper/tls.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/hyper/tls.py b/hyper/tls.py index 44cc8be6..61b08cc5 100644 --- a/hyper/tls.py +++ b/hyper/tls.py @@ -29,14 +29,16 @@ def wrap_socket(sock, server_hostname, ssl_context=None, force_proto=None): A vastly simplified SSL wrapping function. We'll probably extend this to do more things later. """ - global _context - - # create the singleton SSLContext we use - if _context is None: # pragma: no cover - _context = init_context() - - # if an SSLContext is provided then use it instead of default context - _ssl_context = ssl_context or _context + if ssl_context: + # if an SSLContext is provided then use it instead of default context + _ssl_context = ssl_context + else: + global _context + + # create the singleton SSLContext we use + if _context is None: # pragma: no cover + _context = init_context() + _ssl_context = _context # the spec requires SNI support ssl_sock = _ssl_context.wrap_socket(sock, server_hostname=server_hostname) From 951f5314e181fab5896f94054d6e6cbc9a9aa401 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Mon, 6 Jun 2016 17:15:45 +0100 Subject: [PATCH 02/12] A more helpful error for anyone that tries to distribute (such as using Py2Exe) without manually specifying a certificate. --- hyper/tls.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hyper/tls.py b/hyper/tls.py index 61b08cc5..24314391 100644 --- a/hyper/tls.py +++ b/hyper/tls.py @@ -96,9 +96,12 @@ def init_context(cert_path=None, cert=None, cert_password=None): encrypted and no password is needed. :returns: An ``SSLContext`` correctly set up for HTTP/2. """ + cafile = cert_path or cert_loc + if not cafile or not path.exists(cafile): + raise Exception, "No certificate found at " + str(cafile) + ". Either ensure the default cert.pem file is included in the distribution or provide a custom certificate when creating the connection." context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) context.set_default_verify_paths() - context.load_verify_locations(cafile=cert_path or cert_loc) + context.load_verify_locations(cafile=cafile) context.verify_mode = ssl.CERT_REQUIRED context.check_hostname = True From 8120e736801bf2f0d5f01f4572a99cf5f31b40a1 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Thu, 9 Jun 2016 18:52:39 +0100 Subject: [PATCH 03/12] Comply with CI build rules. --- hyper/tls.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hyper/tls.py b/hyper/tls.py index 24314391..0535c0f0 100644 --- a/hyper/tls.py +++ b/hyper/tls.py @@ -98,7 +98,11 @@ def init_context(cert_path=None, cert=None, cert_password=None): """ cafile = cert_path or cert_loc if not cafile or not path.exists(cafile): - raise Exception, "No certificate found at " + str(cafile) + ". Either ensure the default cert.pem file is included in the distribution or provide a custom certificate when creating the connection." + errMsg = ("No certificate found at " + str(cafile) + ". Either " + + "ensure the default cert.pem file is included in the distribution " + + "or provide a custom certificate when creating the connection.") + raise Exception(errMsg) + context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) context.set_default_verify_paths() context.load_verify_locations(cafile=cafile) From a9dd05d5dcaf54030370bb9d1a2216bbae4cdc3e Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Thu, 9 Jun 2016 19:34:20 +0100 Subject: [PATCH 04/12] E128 Fix alignment of line continuation. --- hyper/tls.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hyper/tls.py b/hyper/tls.py index 0535c0f0..66f0628d 100644 --- a/hyper/tls.py +++ b/hyper/tls.py @@ -99,8 +99,9 @@ def init_context(cert_path=None, cert=None, cert_password=None): cafile = cert_path or cert_loc if not cafile or not path.exists(cafile): errMsg = ("No certificate found at " + str(cafile) + ". Either " + - "ensure the default cert.pem file is included in the distribution " + - "or provide a custom certificate when creating the connection.") + "ensure the default cert.pem file is included in the " + + "distribution or provide a custom certificate when " + + "creating the connection.") raise Exception(errMsg) context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) From 4a1ebd67ff350a858ec6a86fb7b9a30f651ebeb6 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Thu, 9 Jun 2016 21:59:31 +0100 Subject: [PATCH 05/12] Use a custom exception. Hoisted global _context declaration. Added test cases. --- hyper/common/exceptions.py | 7 ++++++ hyper/tls.py | 9 ++++---- test/test_SSLContext.py | 44 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/hyper/common/exceptions.py b/hyper/common/exceptions.py index be15fc97..268431ab 100644 --- a/hyper/common/exceptions.py +++ b/hyper/common/exceptions.py @@ -64,3 +64,10 @@ def __init__(self, negotiated, sock): super(HTTPUpgrade, self).__init__() self.negotiated = negotiated self.sock = sock + + +class MissingCertFile(Exception): + """ + The certificate file could not be found. + """ + pass diff --git a/hyper/tls.py b/hyper/tls.py index 66f0628d..c5a7ff8f 100644 --- a/hyper/tls.py +++ b/hyper/tls.py @@ -6,7 +6,7 @@ Contains the TLS/SSL logic for use in hyper. """ import os.path as path - +from common.exceptions import MissingCertFile from .compat import ignore_missing, ssl @@ -29,12 +29,13 @@ def wrap_socket(sock, server_hostname, ssl_context=None, force_proto=None): A vastly simplified SSL wrapping function. We'll probably extend this to do more things later. """ + + global _context + if ssl_context: # if an SSLContext is provided then use it instead of default context _ssl_context = ssl_context else: - global _context - # create the singleton SSLContext we use if _context is None: # pragma: no cover _context = init_context() @@ -102,7 +103,7 @@ def init_context(cert_path=None, cert=None, cert_password=None): "ensure the default cert.pem file is included in the " + "distribution or provide a custom certificate when " + "creating the connection.") - raise Exception(errMsg) + raise MissingCertFile(errMsg) context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) context.set_default_verify_paths() diff --git a/test/test_SSLContext.py b/test/test_SSLContext.py index b03975d4..1287c3a4 100644 --- a/test/test_SSLContext.py +++ b/test/test_SSLContext.py @@ -14,6 +14,7 @@ CLIENT_CERT_FILE = os.path.join(TEST_CERTS_DIR, 'client.crt') CLIENT_KEY_FILE = os.path.join(TEST_CERTS_DIR, 'client.key') CLIENT_PEM_FILE = os.path.join(TEST_CERTS_DIR, 'nopassword.pem') +MISSING_PEM_FILE = os.path.join(TEST_CERTS_DIR, 'missing.pem') class TestSSLContext(object): @@ -60,3 +61,46 @@ def test_client_certificates(self): cert=(CLIENT_CERT_FILE, CLIENT_KEY_FILE), cert_password=b'abc123') hyper.tls.init_context(cert=CLIENT_PEM_FILE) + + def test_HTTPConnection_with_missing_certs(self): + # Clear any prevously created global context + hyper.tls._context = None + backup_cert_loc = hyper.tls.cert_loc + hyper.tls.cert_loc = MISSING_PEM_FILE + + succeeded = False + threwExpectedException = False + try: + HTTPConnection('http2bin.org', 443) + succeeded = True + except hyper.common.exceptions.MissingCertFile: + threwExpectedException = True + except: + pass + + hyper.tls.cert_loc = backup_cert_loc + + assert not succeeded + assert threwExpectedException + + def test_HTTPConnection_with_missing_certs_and_custom_context(self): + # Clear any prevously created global context + hyper.tls._context = None + backup_cert_loc = hyper.tls.cert_loc + hyper.tls.cert_loc = MISSING_PEM_FILE + + context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) + context.set_default_verify_paths() + context.verify_mode = ssl.CERT_REQUIRED + context.check_hostname = True + context.set_npn_protocols(['h2', 'h2-15']) + context.options |= ssl.OP_NO_COMPRESSION + + conn = HTTPConnection('http2bin.org', 443, ssl_context=context) + + hyper.tls.cert_loc = backup_cert_loc + + assert conn.ssl_context.check_hostname + assert conn.ssl_context.verify_mode == ssl.CERT_REQUIRED + assert conn.ssl_context.options & ssl.OP_NO_COMPRESSION != 0 + From 37e122dced2c4e190ba4ae18d2d445a2f9829af1 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Thu, 9 Jun 2016 22:10:25 +0100 Subject: [PATCH 06/12] W391: Remove blank line. --- test/test_SSLContext.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_SSLContext.py b/test/test_SSLContext.py index 1287c3a4..4f246a34 100644 --- a/test/test_SSLContext.py +++ b/test/test_SSLContext.py @@ -103,4 +103,3 @@ def test_HTTPConnection_with_missing_certs_and_custom_context(self): assert conn.ssl_context.check_hostname assert conn.ssl_context.verify_mode == ssl.CERT_REQUIRED assert conn.ssl_context.options & ssl.OP_NO_COMPRESSION != 0 - From 515d74cc2ee7e11a96847df3dc82f0b9e8705e61 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Thu, 9 Jun 2016 22:37:45 +0100 Subject: [PATCH 07/12] Test the right connection type. --- test/test_SSLContext.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_SSLContext.py b/test/test_SSLContext.py index 4f246a34..18813e36 100644 --- a/test/test_SSLContext.py +++ b/test/test_SSLContext.py @@ -71,7 +71,7 @@ def test_HTTPConnection_with_missing_certs(self): succeeded = False threwExpectedException = False try: - HTTPConnection('http2bin.org', 443) + hyper.HTTP20Connection('http2bin.org', 443) succeeded = True except hyper.common.exceptions.MissingCertFile: threwExpectedException = True @@ -96,7 +96,7 @@ def test_HTTPConnection_with_missing_certs_and_custom_context(self): context.set_npn_protocols(['h2', 'h2-15']) context.options |= ssl.OP_NO_COMPRESSION - conn = HTTPConnection('http2bin.org', 443, ssl_context=context) + conn = hyper.HTTP20Connection('http2bin.org', 443, ssl_context=context) hyper.tls.cert_loc = backup_cert_loc From 4dbe8331d4e64d75fca8783342b10375d72f5d75 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Thu, 9 Jun 2016 23:00:58 +0100 Subject: [PATCH 08/12] Need to make an actual request to make connection. Backup & restore the global context to hopefully fix subsequent test failures. --- test/test_SSLContext.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/test_SSLContext.py b/test/test_SSLContext.py index 18813e36..e368a99e 100644 --- a/test/test_SSLContext.py +++ b/test/test_SSLContext.py @@ -64,6 +64,7 @@ def test_client_certificates(self): def test_HTTPConnection_with_missing_certs(self): # Clear any prevously created global context + backup_context = hyper.tls._context hyper.tls._context = None backup_cert_loc = hyper.tls.cert_loc hyper.tls.cert_loc = MISSING_PEM_FILE @@ -71,7 +72,8 @@ def test_HTTPConnection_with_missing_certs(self): succeeded = False threwExpectedException = False try: - hyper.HTTP20Connection('http2bin.org', 443) + conn = hyper.HTTP20Connection('http2bin.org', 443) + conn.request('GET', '/', None, {}) succeeded = True except hyper.common.exceptions.MissingCertFile: threwExpectedException = True @@ -79,12 +81,14 @@ def test_HTTPConnection_with_missing_certs(self): pass hyper.tls.cert_loc = backup_cert_loc + hyper.tls._context = backup_context assert not succeeded assert threwExpectedException def test_HTTPConnection_with_missing_certs_and_custom_context(self): # Clear any prevously created global context + backup_context = hyper.tls._context hyper.tls._context = None backup_cert_loc = hyper.tls.cert_loc hyper.tls.cert_loc = MISSING_PEM_FILE @@ -97,8 +101,10 @@ def test_HTTPConnection_with_missing_certs_and_custom_context(self): context.options |= ssl.OP_NO_COMPRESSION conn = hyper.HTTP20Connection('http2bin.org', 443, ssl_context=context) + conn.request('GET', '/', None, {}) hyper.tls.cert_loc = backup_cert_loc + hyper.tls._context = backup_context assert conn.ssl_context.check_hostname assert conn.ssl_context.verify_mode == ssl.CERT_REQUIRED From 03bd71091a63b64268503fda63905a42f8689992 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Thu, 9 Jun 2016 23:24:08 +0100 Subject: [PATCH 09/12] Fix import. --- hyper/tls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hyper/tls.py b/hyper/tls.py index c5a7ff8f..53b35b6b 100644 --- a/hyper/tls.py +++ b/hyper/tls.py @@ -6,7 +6,7 @@ Contains the TLS/SSL logic for use in hyper. """ import os.path as path -from common.exceptions import MissingCertFile +from .common.exceptions import MissingCertFile from .compat import ignore_missing, ssl From ccfd6c46f5282412891d4377655c76d7a7fb8794 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Fri, 10 Jun 2016 19:26:26 +0100 Subject: [PATCH 10/12] Conform with code style standards. Descope test case to avoid failing on unrelated issues. --- hyper/tls.py | 2 +- test/test_SSLContext.py | 44 +++++------------------------------------ 2 files changed, 6 insertions(+), 40 deletions(-) diff --git a/hyper/tls.py b/hyper/tls.py index 53b35b6b..4244e200 100644 --- a/hyper/tls.py +++ b/hyper/tls.py @@ -99,7 +99,7 @@ def init_context(cert_path=None, cert=None, cert_password=None): """ cafile = cert_path or cert_loc if not cafile or not path.exists(cafile): - errMsg = ("No certificate found at " + str(cafile) + ". Either " + + err_msg = ("No certificate found at " + str(cafile) + ". Either " + "ensure the default cert.pem file is included in the " + "distribution or provide a custom certificate when " + "creating the connection.") diff --git a/test/test_SSLContext.py b/test/test_SSLContext.py index e368a99e..768016c9 100644 --- a/test/test_SSLContext.py +++ b/test/test_SSLContext.py @@ -62,50 +62,16 @@ def test_client_certificates(self): cert_password=b'abc123') hyper.tls.init_context(cert=CLIENT_PEM_FILE) - def test_HTTPConnection_with_missing_certs(self): - # Clear any prevously created global context - backup_context = hyper.tls._context - hyper.tls._context = None - backup_cert_loc = hyper.tls.cert_loc - hyper.tls.cert_loc = MISSING_PEM_FILE - + def test_missing_certs(self): succeeded = False - threwExpectedException = False + threw_expected_exception = False try: - conn = hyper.HTTP20Connection('http2bin.org', 443) - conn.request('GET', '/', None, {}) + conn = hyper.tls.init_context(MISSING_PEM_FILE) succeeded = True except hyper.common.exceptions.MissingCertFile: - threwExpectedException = True + threw_expected_exception = True except: pass - hyper.tls.cert_loc = backup_cert_loc - hyper.tls._context = backup_context - assert not succeeded - assert threwExpectedException - - def test_HTTPConnection_with_missing_certs_and_custom_context(self): - # Clear any prevously created global context - backup_context = hyper.tls._context - hyper.tls._context = None - backup_cert_loc = hyper.tls.cert_loc - hyper.tls.cert_loc = MISSING_PEM_FILE - - context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) - context.set_default_verify_paths() - context.verify_mode = ssl.CERT_REQUIRED - context.check_hostname = True - context.set_npn_protocols(['h2', 'h2-15']) - context.options |= ssl.OP_NO_COMPRESSION - - conn = hyper.HTTP20Connection('http2bin.org', 443, ssl_context=context) - conn.request('GET', '/', None, {}) - - hyper.tls.cert_loc = backup_cert_loc - hyper.tls._context = backup_context - - assert conn.ssl_context.check_hostname - assert conn.ssl_context.verify_mode == ssl.CERT_REQUIRED - assert conn.ssl_context.options & ssl.OP_NO_COMPRESSION != 0 + assert threw_expected_exception From 60fb2f3e9373df6b3022de748cdd175f1f144a81 Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Fri, 10 Jun 2016 19:51:46 +0100 Subject: [PATCH 11/12] Fix build errors. --- hyper/tls.py | 2 +- test/test_SSLContext.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hyper/tls.py b/hyper/tls.py index 4244e200..6319abd7 100644 --- a/hyper/tls.py +++ b/hyper/tls.py @@ -103,7 +103,7 @@ def init_context(cert_path=None, cert=None, cert_password=None): "ensure the default cert.pem file is included in the " + "distribution or provide a custom certificate when " + "creating the connection.") - raise MissingCertFile(errMsg) + raise MissingCertFile(err_msg) context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) context.set_default_verify_paths() diff --git a/test/test_SSLContext.py b/test/test_SSLContext.py index 768016c9..4add16f3 100644 --- a/test/test_SSLContext.py +++ b/test/test_SSLContext.py @@ -66,7 +66,7 @@ def test_missing_certs(self): succeeded = False threw_expected_exception = False try: - conn = hyper.tls.init_context(MISSING_PEM_FILE) + hyper.tls.init_context(MISSING_PEM_FILE) succeeded = True except hyper.common.exceptions.MissingCertFile: threw_expected_exception = True From bc0738bdf7afdc1236e8154868352b58850fdf1a Mon Sep 17 00:00:00 2001 From: JasonGowthorpe Date: Fri, 10 Jun 2016 20:04:13 +0100 Subject: [PATCH 12/12] Fix indentation. --- hyper/tls.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hyper/tls.py b/hyper/tls.py index 6319abd7..422b001c 100644 --- a/hyper/tls.py +++ b/hyper/tls.py @@ -100,9 +100,9 @@ def init_context(cert_path=None, cert=None, cert_password=None): cafile = cert_path or cert_loc if not cafile or not path.exists(cafile): err_msg = ("No certificate found at " + str(cafile) + ". Either " + - "ensure the default cert.pem file is included in the " + - "distribution or provide a custom certificate when " + - "creating the connection.") + "ensure the default cert.pem file is included in the " + + "distribution or provide a custom certificate when " + + "creating the connection.") raise MissingCertFile(err_msg) context = ssl.SSLContext(ssl.PROTOCOL_SSLv23)