From 37dae6df9f85c96be660dbb74a3eeaf42a796797 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 19 Feb 2018 17:17:19 +0000 Subject: [PATCH] Fix: Inject secret payload rather than reference into document This PS fixes Deckhand currently wrongly substituting the secret reference Barbican returns into documents, rather than the secret payload itself. Closes #19 Change-Id: I1d4eed85ed336e83a777b4343f37b10c91038342 --- deckhand/barbican/client_wrapper.py | 1 + deckhand/barbican/driver.py | 11 ++++++ deckhand/control/buckets.py | 8 +++- deckhand/engine/secrets_manager.py | 31 +++++++++++++-- .../tests/unit/engine/test_secrets_manager.py | 39 +++++++++++++++++++ entrypoint.sh | 0 6 files changed, 85 insertions(+), 5 deletions(-) mode change 100644 => 100755 entrypoint.sh diff --git a/deckhand/barbican/client_wrapper.py b/deckhand/barbican/client_wrapper.py index f7a5b971..96514e75 100644 --- a/deckhand/barbican/client_wrapper.py +++ b/deckhand/barbican/client_wrapper.py @@ -41,6 +41,7 @@ class BarbicanClientWrapper(object): def _get_client(self, retry_on_conflict=True): # If we've already constructed a valid, authed client, just return # that. + if retry_on_conflict and self._cached_client is not None: return self._cached_client diff --git a/deckhand/barbican/driver.py b/deckhand/barbican/driver.py index dd01f020..6c3bec7f 100644 --- a/deckhand/barbican/driver.py +++ b/deckhand/barbican/driver.py @@ -46,3 +46,14 @@ class BarbicanDriver(object): resp[utils.to_snake_case(key)] = resp.pop(key) resp['secret_ref'] = secret_ref return resp + + def get_secret(self, secret_ref): + """Get a secret.""" + + try: + return self.barbicanclient.call("secrets.get", secret_ref) + except (barbicanclient.exceptions.HTTPAuthError, + barbicanclient.exceptions.HTTPClientError, + barbicanclient.exceptions.HTTPServerError) as e: + LOG.exception(str(e)) + raise errors.BarbicanException(details=str(e)) diff --git a/deckhand/control/buckets.py b/deckhand/control/buckets.py index 44d37a3d..5ea19a6f 100644 --- a/deckhand/control/buckets.py +++ b/deckhand/control/buckets.py @@ -67,7 +67,13 @@ class BucketsResource(api_base.BaseResource): 'deckhand:create_encrypted_documents', req.context) break - self._prepare_secret_documents(documents) + try: + self._prepare_secret_documents(documents) + except deckhand_errors.BarbicanException as e: + LOG.error('An unknown exception occurred while trying to store ' + 'a secret in Barbican.') + raise falcon.HTTPInternalServerError( + description=e.format_message()) created_documents = self._create_revision_documents( bucket_name, documents, validations) diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index 9b6df28d..ac076798 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -36,7 +36,8 @@ class SecretsManager(object): barbican_driver = driver.BarbicanDriver() - def create(self, secret_doc): + @classmethod + def create(cls, secret_doc): """Securely store secrets contained in ``secret_doc``. Ordinarily, Deckhand documents are stored directly in Deckhand's @@ -62,7 +63,7 @@ class SecretsManager(object): ``deckhand.db.sqlalchemy.models.DocumentSecret``. """ encryption_type = secret_doc['metadata']['storagePolicy'] - secret_type = self._get_secret_type(secret_doc['schema']) + secret_type = cls._get_secret_type(secret_doc['schema']) if encryption_type == ENCRYPTED: # Store secret_ref in database for `secret_doc`. @@ -71,7 +72,7 @@ class SecretsManager(object): 'secret_type': secret_type, 'payload': secret_doc['data'] } - resp = self.barbican_driver.create_secret(**kwargs) + resp = cls.barbican_driver.create_secret(**kwargs) secret_ref = resp['secret_ref'] created_secret = secret_ref @@ -80,7 +81,14 @@ class SecretsManager(object): return created_secret - def _get_secret_type(self, schema): + @classmethod + def get(cls, secret_ref): + secret = cls.barbican_driver.get_secret(secret_ref=secret_ref) + payload = secret.payload + return payload + + @classmethod + def _get_secret_type(cls, schema): """Get the Barbican secret type based on the following mapping: ``deckhand/Certificate/v1`` => certificate @@ -150,6 +158,11 @@ class SecretsSubstitution(object): self._substitution_sources.setdefault( (document.schema, document.name), document) + def _is_barbican_ref(self, src_secret): + # TODO(fmontei): Make this more robust. + return (isinstance(src_secret, six.string_types) and + 'key-manager/v1/secrets' in src_secret) + def substitute_all(self, documents): """Substitute all documents that have a `metadata.substitutions` field. @@ -218,6 +231,12 @@ class SecretsSubstitution(object): else: src_secret = src_doc.get('data') + # Check if src_secret is Barbican secret reference. + if self._is_barbican_ref(src_secret): + LOG.debug('Resolving Barbican reference for %s.', + src_secret) + src_secret = SecretsManager.get(src_secret) + dest_path = sub['dest']['path'] dest_pattern = sub['dest'].get('pattern', None) @@ -240,6 +259,10 @@ class SecretsSubstitution(object): document.name) LOG.error(message) raise errors.UnknownSubstitutionError(details=message) + except errors.BarbicanException as e: + LOG.error('Failed to resolve a Barbican reference.') + raise errors.UnknownSubstitutionError( + details=e.format_message()) except Exception as e: LOG.error('Unexpected exception occurred while attempting ' 'secret substitution. %s', six.text_type(e)) diff --git a/deckhand/tests/unit/engine/test_secrets_manager.py b/deckhand/tests/unit/engine/test_secrets_manager.py index 528518a4..07c276dd 100644 --- a/deckhand/tests/unit/engine/test_secrets_manager.py +++ b/deckhand/tests/unit/engine/test_secrets_manager.py @@ -15,6 +15,8 @@ import copy import yaml +import mock + from deckhand.db.sqlalchemy import api as db_api from deckhand.engine import secrets_manager from deckhand import factories @@ -130,6 +132,43 @@ class TestSecretsSubstitution(test_base.TestDbBase): self._test_doc_substitution( document_mapping, [certificate], expected_data) + @mock.patch.object(secrets_manager, 'SecretsManager', autospec=True) + def test_doc_substitution_single_encrypted(self, mock_secrets_manager): + mock_secrets_manager.get.return_value = 'test-certificate' + secret_ref = test_utils.rand_uuid_hex() + + secret_ref = ("http://127.0.0.1/key-manager/v1/secrets/%s" + % secret_ref) + certificate = self.secrets_factory.gen_test( + 'Certificate', 'encrypted', data=secret_ref) + certificate['metadata']['name'] = 'example-cert' + + document_mapping = { + "_GLOBAL_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".chart.values.tls.certificate" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "example-cert", + "path": "." + } + + }] + } + expected_data = { + 'chart': { + 'values': { + 'tls': { + 'certificate': 'test-certificate' + } + } + } + } + self._test_doc_substitution( + document_mapping, [certificate], expected_data) + mock_secrets_manager.get.assert_called_once_with(secret_ref=secret_ref) + def test_create_destination_path_with_array(self): # Validate that the destination data will be populated with an array # where the data will be contained in array[0]. diff --git a/entrypoint.sh b/entrypoint.sh old mode 100644 new mode 100755