diff --git a/octavia/api/v2/controllers/base.py b/octavia/api/v2/controllers/base.py index 805d75af68..60b334ff17 100644 --- a/octavia/api/v2/controllers/base.py +++ b/octavia/api/v2/controllers/base.py @@ -246,6 +246,8 @@ class BaseController(pecan.rest.RestController): try: self.cert_manager.set_acls(context, ref) self.cert_manager.get_cert(context, ref, check_only=True) + except exceptions.UnreadablePKCS12: + raise except Exception: bad_refs.append(ref) diff --git a/octavia/certificates/common/pkcs12.py b/octavia/certificates/common/pkcs12.py index c5870e01a5..1e3aba4604 100644 --- a/octavia/certificates/common/pkcs12.py +++ b/octavia/certificates/common/pkcs12.py @@ -21,12 +21,16 @@ from cryptography.hazmat.primitives import serialization from OpenSSL import crypto from octavia.certificates.common import cert +from octavia.common import exceptions class PKCS12Cert(cert.Cert): """Representation of a Cert for local storage.""" def __init__(self, certbag): - p12 = crypto.load_pkcs12(certbag) + try: + p12 = crypto.load_pkcs12(certbag) + except crypto.Error as e: + raise exceptions.UnreadablePKCS12(error=str(e)) self.certificate = p12.get_certificate() self.intermediates = p12.get_ca_certificates() self.private_key = p12.get_privatekey() diff --git a/octavia/certificates/manager/barbican.py b/octavia/certificates/manager/barbican.py index 0293d4a49e..a6b36399f6 100644 --- a/octavia/certificates/manager/barbican.py +++ b/octavia/certificates/manager/barbican.py @@ -112,6 +112,8 @@ class BarbicanCertManager(cert_mgr.CertManager): try: cert_secret = connection.secrets.get(secret_ref=cert_ref) return pkcs12.PKCS12Cert(cert_secret.payload) + except exceptions.UnreadablePKCS12: + raise except Exception: # If our get fails, try with the legacy driver. # TODO(rm_work): Remove this code when the deprecation cycle for diff --git a/octavia/common/exceptions.py b/octavia/common/exceptions.py index a806171431..28d8bad8c0 100644 --- a/octavia/common/exceptions.py +++ b/octavia/common/exceptions.py @@ -128,6 +128,13 @@ class UnreadableCert(OctaviaException): message = _("Could not read X509 from PEM") +class UnreadablePKCS12(APIException): + msg = _("The PKCS12 bundle is unreadable. Please check the PKCS12 bundle " + "validity. In addition, make sure it does not require a pass " + "phrase. Error: %(error)s") + code = 400 + + class MisMatchedKey(OctaviaException): message = _("Key and x509 certificate do not match") diff --git a/octavia/tests/unit/certificates/manager/test_barbican.py b/octavia/tests/unit/certificates/manager/test_barbican.py index c0369e3f32..365465e344 100644 --- a/octavia/tests/unit/certificates/manager/test_barbican.py +++ b/octavia/tests/unit/certificates/manager/test_barbican.py @@ -16,6 +16,7 @@ import uuid from barbicanclient.v1 import secrets import mock +from OpenSSL import crypto import octavia.certificates.common.barbican as barbican_common import octavia.certificates.common.cert as cert @@ -138,6 +139,22 @@ class TestBarbicanManager(base.TestCase): sorted(data.get_intermediates())) self.assertIsNone(data.get_private_key_passphrase()) + @mock.patch('OpenSSL.crypto.load_pkcs12') + def test_get_cert_bad_pkcs12(self, mock_load_pkcs12): + + mock_load_pkcs12.side_effect = [crypto.Error] + + # Mock out the client + self.bc.secrets.get.return_value = self.secret_pkcs12 + + # Test bad pkcs12 bundle re-raises UnreadablePKCS12 + self.assertRaises(exceptions.UnreadablePKCS12, + self.cert_manager.get_cert, + context=self.context, + cert_ref=self.secret_ref, + resource_ref=self.secret_ref, + service_name='Octavia') + def test_delete_cert_legacy(self): # Attempt to deregister as a consumer self.cert_manager.delete_cert(