From 76b894b47a9dcd44098fd59eb22eb2fee599a7f9 Mon Sep 17 00:00:00 2001 From: Stuart McLaren Date: Wed, 25 Jan 2012 11:34:34 +0000 Subject: [PATCH] Don't force client to supply SSL cert/key Fix for bug 921494. Clients should be able to connect using SSL without specifying a client-side key or cert. Change-Id: Iad65d1c77543184cfe7faa8aa07926a87cb14b43 --- glance/common/client.py | 51 ++++++++++++++++++------------- glance/tests/unit/test_clients.py | 37 +++++++++++++++------- 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/glance/common/client.py b/glance/common/client.py index c08ec9a6ef..1d3c6da1c0 100644 --- a/glance/common/client.py +++ b/glance/common/client.py @@ -210,39 +210,46 @@ def __init__(self, host, port=None, use_ssl=False, auth_tok=None, self.connect_kwargs = {} if use_ssl: - if not key_file: - if not os.environ.get('GLANCE_CLIENT_KEY_FILE'): - msg = _("You have selected to use SSL in connecting, " - "however you have failed to supply either a " - "key_file parameter or set the " - "GLANCE_CLIENT_KEY_FILE environ variable") - raise exception.ClientConnectionError(msg) + if key_file is None: key_file = os.environ.get('GLANCE_CLIENT_KEY_FILE') + if cert_file is None: + cert_file = os.environ.get('GLANCE_CLIENT_CERT_FILE') + if ca_file is None: + ca_file = os.environ.get('GLANCE_CLIENT_CA_FILE') + + # Check that key_file/cert_file are either both set or both unset + if cert_file is not None and key_file is None: + msg = _("You have selected to use SSL in connecting, " + "and you have supplied a cert, " + "however you have failed to supply either a " + "key_file parameter or set the " + "GLANCE_CLIENT_KEY_FILE environ variable") + raise exception.ClientConnectionError(msg) + + if key_file is not None and cert_file is None: + msg = _("You have selected to use SSL in connecting, " + "and you have supplied a key, " + "however you have failed to supply either a " + "cert_file parameter or set the " + "GLANCE_CLIENT_CERT_FILE environ variable") + raise exception.ClientConnectionError(msg) - if not os.path.exists(key_file): + if key_file is not None and not os.path.exists(key_file): msg = _("The key file you specified %s does not " "exist") % key_file raise exception.ClientConnectionError(msg) self.connect_kwargs['key_file'] = key_file - if not cert_file: - if not os.environ.get('GLANCE_CLIENT_CERT_FILE'): - msg = _("You have selected to use SSL in connecting, " - "however you have failed to supply either a " - "cert_file parameter or set the " - "GLANCE_CLIENT_CERT_FILE environ variable") - raise exception.ClientConnectionError(msg) - cert_file = os.environ.get('GLANCE_CLIENT_CERT_FILE') - - if not os.path.exists(cert_file): - msg = _("The key file you specified %s does not " + if cert_file is not None and not os.path.exists(cert_file): + msg = _("The cert file you specified %s does not " "exist") % cert_file raise exception.ClientConnectionError(msg) self.connect_kwargs['cert_file'] = cert_file - if not ca_file: - ca_file = os.environ.get('GLANCE_CLIENT_CA_FILE') - + if ca_file is not None and not os.path.exists(ca_file): + msg = _("The CA file you specified %s does not " + "exist") % ca_file + raise exception.ClientConnectionError(msg) self.connect_kwargs['ca_file'] = ca_file def set_auth_token(self, auth_tok): diff --git a/glance/tests/unit/test_clients.py b/glance/tests/unit/test_clients.py index c51af4d602..bef7b3940e 100644 --- a/glance/tests/unit/test_clients.py +++ b/glance/tests/unit/test_clients.py @@ -59,18 +59,22 @@ def test_bad_address(self): def test_ssl_no_key_file(self): """ Test that when doing SSL connection, a key file is - required + required if a cert file has been specified """ try: - c = client.Client("0.0.0.0", use_ssl=True) + with tempfile.NamedTemporaryFile() as cert_file: + cert_file.write("bogus-cert") + cert_file.flush() + c = client.Client("0.0.0.0", use_ssl=True, + cert_file=cert_file.name) except exception.ClientConnectionError: return self.fail("Did not raise ClientConnectionError") def test_ssl_non_existing_key_file(self): """ - Test that when doing SSL connection, a key file is - required to exist + Test that when doing SSL connection, a specified key + file is required to exist """ try: c = client.Client("0.0.0.0", use_ssl=True, @@ -82,11 +86,11 @@ def test_ssl_non_existing_key_file(self): def test_ssl_no_cert_file(self): """ Test that when doing SSL connection, a cert file is - required + required if a key file has been specified """ try: with tempfile.NamedTemporaryFile() as key_file: - key_file.write("bogus") + key_file.write("bogus-key") key_file.flush() c = client.Client("0.0.0.0", use_ssl=True, key_file=key_file.name) @@ -97,11 +101,11 @@ def test_ssl_no_cert_file(self): def test_ssl_non_existing_cert_file(self): """ Test that when doing SSL connection, a cert file is - required to exist + required to exist if specified """ try: with tempfile.NamedTemporaryFile() as key_file: - key_file.write("bogus") + key_file.write("bogus-key") key_file.flush() c = client.Client("0.0.0.0", use_ssl=True, key_file=key_file.name, @@ -110,17 +114,28 @@ def test_ssl_non_existing_cert_file(self): return self.fail("Did not raise ClientConnectionError") + def test_ssl_non_existing_ca_file(self): + """ + Test that when doing SSL connection, a specified CA file exists + """ + try: + c = client.Client("0.0.0.0", use_ssl=True, + ca_file='nonexistingfile') + except exception.ClientConnectionError: + return + self.fail("Did not raise ClientConnectionError") + def test_ssl_optional_ca_file(self): """ Test that when doing SSL connection, a cert file and key file are - required to exist, but a CA file is optional. + required to exist if specified, but a CA file is optional. """ try: with tempfile.NamedTemporaryFile() as key_file: - key_file.write("bogus") + key_file.write("bogus-key") key_file.flush() with tempfile.NamedTemporaryFile() as cert_file: - cert_file.write("bogus") + cert_file.write("bogus-cert") cert_file.flush() c = client.Client("0.0.0.0", use_ssl=True, key_file=key_file.name,