diff --git a/deckhand/barbican/driver.py b/deckhand/barbican/driver.py index 8a133d71..a3fde93a 100644 --- a/deckhand/barbican/driver.py +++ b/deckhand/barbican/driver.py @@ -20,7 +20,6 @@ from oslo_config import cfg from oslo_log import log as logging from oslo_serialization import base64 from oslo_utils import excutils -import six from deckhand.barbican import cache from deckhand.barbican import client_wrapper @@ -36,67 +35,15 @@ class BarbicanDriver(object): def __init__(self): self.barbicanclient = client_wrapper.BarbicanClientWrapper() - @staticmethod - def _get_secret_type(schema): - """Get the Barbican secret type based on the following mapping: - - ``deckhand/Certificate/v1`` => certificate - ``deckhand/CertificateKey/v1`` => private - ``deckhand/CertificateAuthority/v1`` => certificate - ``deckhand/CertificateAuthorityKey/v1`` => private - ``deckhand/Passphrase/v1`` => passphrase - ``deckhand/PrivateKey/v1`` => private - ``deckhand/PublicKey/v1`` => public - Other => passphrase - - :param schema: The document's schema. - :returns: The value corresponding to the mapping above. - """ - parts = schema.split('/') - if len(parts) == 3: - namespace, kind, _ = parts - elif len(parts) == 2: - namespace, kind = parts - else: - raise ValueError( - 'Schema %s must consist of namespace/kind/version.' % schema) - - is_generic = ( - '/'.join([namespace, kind]) not in types.DOCUMENT_SECRET_TYPES - ) - - # If the document kind is not a built-in secret type, then default to - # 'passphrase'. - if is_generic: - LOG.debug('Defaulting to secret_type="passphrase" for generic ' - 'document schema %s.', schema) - return 'passphrase' - - kind = kind.lower() - - if kind in [ - 'certificateauthoritykey', 'certificatekey', 'privatekey' - ]: - return 'private' - elif kind == 'certificateauthority': - return 'certificate' - elif kind == 'publickey': - return 'public' - # NOTE(felipemonteiro): This branch below handles certificate and - # passphrase, both of which are supported secret types in Barbican. - return kind - def _base64_encode_payload(self, secret_doc): """Ensures secret document payload is compatible with Barbican.""" payload = secret_doc.data - secret_type = self._get_secret_type(secret_doc.schema) - - # NOTE(felipemonteiro): The logic for the 2 conditions below is - # enforced from Barbican's Python client. Some pre-processing and - # transformation is needed to make Barbican work with non-compatible - # formats. - if not payload and payload is not False: + secret_type = None + # Explicitly list the "empty" payloads we are refusing to store. + # We don't use ``if not payload`` because that would not encrypt + # and store something like ``data: !!int 0`` + if payload in ('', {}, [], None): # There is no point in even bothering to encrypt an empty # body, which just leads to needless overhead, so return # early. @@ -104,19 +51,14 @@ class BarbicanDriver(object): 'Deckhand will not encrypt document [%s, %s] %s.', secret_doc.schema, secret_doc.layer, secret_doc.name) secret_doc.storage_policy = types.CLEARTEXT - elif not isinstance( - payload, (six.text_type, six.binary_type)): - LOG.debug('Forcibly setting secret_type=opaque and ' - 'base64-encoding non-string payload for ' - 'document [%s, %s] %s.', secret_doc.schema, - secret_doc.layer, secret_doc.name) - # NOTE(felipemonteiro): base64-encoding the non-string payload is - # done for serialization purposes, not for security purposes. - # 'opaque' is used to avoid Barbican doing any further - # serialization server-side. + else: + LOG.debug('Setting secret_type=opaque and ' + 'base64-encoding payload of type %s for ' + 'document [%s, %s] %s.', type(payload), + secret_doc.schema, secret_doc.layer, secret_doc.name) secret_type = 'opaque' try: - payload = base64.encode_as_text(six.text_type(payload)) + payload = base64.encode_as_text(repr(payload)) except Exception: message = ('Failed to base64-encode payload of type %s ' 'for Barbican storage.', type(payload)) @@ -222,7 +164,7 @@ class BarbicanDriver(object): payload = secret.payload if secret.secret_type == 'opaque': - LOG.debug('Forcibly base64-decoding original non-string payload ' + LOG.debug('Base64-decoding original payload ' 'for document [%s, %s] %s.', *src_doc.meta) secret = self._base64_decode_payload(payload) else: diff --git a/deckhand/tests/integration/gabbits/document-crud-secret.yaml b/deckhand/tests/integration/gabbits/document-crud-secret.yaml index 5fd6be20..c94a4de2 100644 --- a/deckhand/tests/integration/gabbits/document-crud-secret.yaml +++ b/deckhand/tests/integration/gabbits/document-crud-secret.yaml @@ -65,7 +65,8 @@ tests: response_headers: content-type: application/octet-stream; charset=UTF-8 response_strings: - - not-a-real-password + # base64.b64encode(repr("not-a-real-password")) + - J25vdC1hLXJlYWwtcGFzc3dvcmQn - name: verify_revision_documents_returns_secret_ref desc: Verify that the documents for the created revision returns the secret ref. diff --git a/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml b/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml index f5ffb2c1..b317a9df 100644 --- a/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml +++ b/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml @@ -82,7 +82,7 @@ tests: $.[0].data.`split(:, 0, 1)` + "://" + $.[0].data.`split(/, 2, 3)`: $ENVIRON['TEST_BARBICAN_URL'] - name: verify_generic_secret_created_in_barbican - desc: Validate that the generic secret gets stored with secret_type passphrase. + desc: Validate that the generic secret gets stored with secret_type opaque. GET: $ENVIRON['TEST_BARBICAN_URL']/v1/secrets/$RESPONSE['$.[0].data.`split(/, 5, -1)`'] status: 200 request_headers: @@ -93,7 +93,7 @@ tests: $.status: ACTIVE $.name: example-armada-cert # Default type for documents with generic schema. - $.secret_type: passphrase + $.secret_type: opaque - name: verify_secret_payload_in_destination_document desc: Verify secret payload is injected in destination document as well as example-armada-cert. diff --git a/deckhand/tests/integration/gabbits/document-substitution-secret.yaml b/deckhand/tests/integration/gabbits/document-substitution-secret.yaml index 44946535..fcc62a38 100644 --- a/deckhand/tests/integration/gabbits/document-substitution-secret.yaml +++ b/deckhand/tests/integration/gabbits/document-substitution-secret.yaml @@ -222,13 +222,13 @@ tests: - example-private-key - example-public-key $.secrets[*].secret_type: - - certificate - - private - - certificate - - private - - passphrase - - private - - public + - opaque + - opaque + - opaque + - opaque + - opaque + - opaque + - opaque - name: verify_secret_payload_in_destination_document desc: | diff --git a/deckhand/tests/integration/gabbits/revision-delete-all-secrets.yaml b/deckhand/tests/integration/gabbits/revision-delete-all-secrets.yaml index 2b9b1768..81dbdb8f 100644 --- a/deckhand/tests/integration/gabbits/revision-delete-all-secrets.yaml +++ b/deckhand/tests/integration/gabbits/revision-delete-all-secrets.yaml @@ -62,7 +62,8 @@ tests: response_headers: content-type: application/octet-stream; charset=UTF-8 response_strings: - - not-a-real-password + # base64.b64encode(repr("not-a-real-password")) + - J25vdC1hLXJlYWwtcGFzc3dvcmQn - name: delete_all_revisions desc: Delete all revisions from Deckhand, which should delete all secrets. diff --git a/deckhand/tests/unit/engine/test_secrets_manager.py b/deckhand/tests/unit/engine/test_secrets_manager.py index 11306a8f..6f1e45a3 100644 --- a/deckhand/tests/unit/engine/test_secrets_manager.py +++ b/deckhand/tests/unit/engine/test_secrets_manager.py @@ -18,10 +18,8 @@ import yaml import mock from oslo_serialization import base64 from oslo_utils import uuidutils -import six import testtools -from deckhand.barbican import driver from deckhand.common import document as document_wrapper from deckhand.engine import secrets_manager from deckhand import errors @@ -64,9 +62,8 @@ class TestSecretsManager(test_base.DeckhandWithDBTestCase): elif encryption_type == 'encrypted': expected_kwargs = { 'name': secret_doc['metadata']['name'], - 'secret_type': driver.BarbicanDriver._get_secret_type( - 'deckhand/' + secret_type), - 'payload': payload + 'secret_type': 'opaque', + 'payload': base64.encode_as_text(repr(payload)) } self.assertEqual(self.secret_ref, secret_ref) self.mock_barbicanclient.call.assert_called_once_with( @@ -123,6 +120,8 @@ class TestSecretsManager(test_base.DeckhandWithDBTestCase): 'secrets.get', secret_ref) def test_empty_payload_skips_encryption(self): + # NOTE: Not testing for the `None` case here, because gen_test + # factory method interprets `None` as "generate a passphrase" for empty_payload in ('', {}, []): secret_doc = self.factory.gen_test( 'Certificate', 'encrypted', empty_payload) @@ -142,7 +141,7 @@ class TestSecretsManager(test_base.DeckhandWithDBTestCase): secret_doc = self.factory.gen_test( 'Certificate', 'encrypted', payload) - expected_payload = base64.encode_as_text(six.text_type({'foo': 'bar'})) + expected_payload = base64.encode_as_text(repr({'foo': 'bar'})) expected_kwargs = { 'name': secret_doc['metadata']['name'], 'secret_type': 'opaque',