From 106038d3cd15f348eed1f5d7bb83eed56ee9e314 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Thu, 5 Apr 2018 14:40:47 -0400 Subject: [PATCH] [fix] Pass secret URI instead of UUID to barbican get_secret This is to change passing the secret URI instead of the secret UUID to barbican's get secret endpoint from which the secret itself can be extracted. While the API [0] expects a UUID the CLI instead expects a URI and the latter extracts the UUID from the URI automatically [1]. API ref: GET /v1/secrets/{uuid} Headers: Accept: application/json X-Auth-Token: {token} (or X-Project-Id: {project_id}) CLI ref: $ barbican help secret get usage: barbican secret get [-h] [-f {shell,table,value}] [-c COLUMN] [--max-width ] [--prefix PREFIX] [--decrypt] [--payload] [--payload_content_type PAYLOAD_CONTENT_TYPE] URI Retrieve a secret by providing its URI. Finally, this adds logic for ensuring that all encrypted data is retrieved and injected back into the raw documents with Barbican references, during document rendering. Currently, this process is only performed for documents with substitutions, but should also be carried out for encrypted documents themselves. [0] https://docs.openstack.org/barbican/latest/api/reference/secrets.html#get-v1-secrets-uuid [1] https://docs.openstack.org/python-barbicanclient/latest/reference/index.html#barbicanclient.v1.secrets.SecretManager.get Change-Id: I1717592b7acdedb66353c25fb5dcda2d5330196b --- deckhand/common/utils.py | 6 +- deckhand/conf/config.py | 5 +- deckhand/control/buckets.py | 16 +-- deckhand/engine/layering.py | 11 ++ deckhand/engine/secrets_manager.py | 103 ++++++++++-------- .../tests/unit/engine/test_secrets_manager.py | 16 +-- deckhand/types.py | 9 ++ 7 files changed, 93 insertions(+), 73 deletions(-) diff --git a/deckhand/common/utils.py b/deckhand/common/utils.py index a83d2a7e..2109cfaf 100644 --- a/deckhand/common/utils.py +++ b/deckhand/common/utils.py @@ -30,6 +30,8 @@ LOG = logging.getLogger(__name__) # is computationally expensive. _PATH_CACHE = dict() +_ARRAY_RE = re.compile(r'.*\[\d+\].*') + def to_camel_case(s): """Convert string to camel case.""" @@ -106,12 +108,10 @@ def _populate_data_with_attributes(jsonpath, data): # Populates ``data`` with any path specified in ``jsonpath``. For example, # if jsonpath is ".foo[0].bar.baz" then for each subpath -- foo[0], bar, # and baz -- that key will be added to ``data`` if missing. - array_re = re.compile(r'.*\[\d+\].*') - d = data for path in jsonpath.split('.')[1:]: # Handle case where an array needs to be created. - if array_re.match(path): + if _ARRAY_RE.match(path): try: path_pieces = path.split('[') path_piece = path_pieces[0] diff --git a/deckhand/conf/config.py b/deckhand/conf/config.py index 6eb8aa27..57d33641 100644 --- a/deckhand/conf/config.py +++ b/deckhand/conf/config.py @@ -40,11 +40,10 @@ default_opts = [ help=""" Allow limited access to unauthenticated users. -Assign a boolean to determine API access for unathenticated +Assign a boolean to determine API access for unauthenticated users. When set to False, the API cannot be accessed by unauthenticated users. When set to True, unauthenticated users can -access the API with read-only privileges. This however only applies -when using ContextMiddleware. +access the API with read-only privileges. Possible values: * True diff --git a/deckhand/control/buckets.py b/deckhand/control/buckets.py index 63d233c3..ca4b12a6 100644 --- a/deckhand/control/buckets.py +++ b/deckhand/control/buckets.py @@ -52,13 +52,13 @@ class BucketsResource(api_base.BaseResource): raise falcon.HTTPBadRequest(description=e.format_message()) for document in documents: - if document['metadata'].get('storagePolicy') == 'encrypted': + if secrets_manager.SecretsManager.requires_encryption(document): policy.conditional_authorize( 'deckhand:create_encrypted_documents', req.context) break try: - self._prepare_secret_documents(documents) + documents = 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.') @@ -71,13 +71,13 @@ class BucketsResource(api_base.BaseResource): resp.body = self.view_builder.list(created_documents) resp.status = falcon.HTTP_200 - def _prepare_secret_documents(self, secret_documents): + def _prepare_secret_documents(self, documents): # Encrypt data for secret documents, if any. - for document in secret_documents: - # TODO(fmontei): Move all of this to document validation directly. - if document['metadata'].get('storagePolicy') == 'encrypted': - secret_data = secrets_manager.SecretsManager.create(document) - document['data'] = secret_data + for document in documents: + if secrets_manager.SecretsManager.requires_encryption(document): + secret_ref = secrets_manager.SecretsManager.create(document) + document['data'] = secret_ref + return documents def _create_revision_documents(self, bucket_name, documents, validations): diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index 557d4de2..9aca24fe 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -678,6 +678,17 @@ class DocumentLayering(object): self.secrets_substitution.update_substitution_sources( doc.schema, doc.name, rendered_data.data) self._documents_by_index[doc.meta] = rendered_data + # Otherwise, retrieve the encrypted data for the document if its + # data has been encrypted so that future references use the actual + # secret payload, rather than the Barbican secret reference. + elif doc.is_encrypted: + encrypted_data = self.secrets_substitution.get_encrypted_data( + doc.data, doc, doc) + if not doc.is_abstract: + doc.data = encrypted_data + self.secrets_substitution.update_substitution_sources( + doc.schema, doc.name, encrypted_data) + self._documents_by_index[doc.meta] = encrypted_data # Return only concrete documents and non-replacements. return [d for d in self._sorted_documents diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index d6fb0f6c..93eada97 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -24,13 +24,11 @@ from deckhand.barbican import driver from deckhand.common import document as document_wrapper from deckhand.common import utils from deckhand import errors +from deckhand import types CONF = cfg.CONF LOG = logging.getLogger(__name__) -CLEARTEXT = 'cleartext' -ENCRYPTED = 'encrypted' - class SecretsManager(object): """Internal API resource for interacting with Barbican. @@ -43,6 +41,31 @@ class SecretsManager(object): _url_re = re.compile(r'http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|' '(?:%[0-9a-fA-F][0-9a-fA-F]))+') + @staticmethod + def requires_encryption(document): + clazz = document_wrapper.DocumentDict + if not isinstance(document, clazz): + document = clazz(document) + return document.is_encrypted + + @classmethod + def is_barbican_ref(cls, secret_ref): + # TODO(fmontei): Query Keystone service catalog for Barbican endpoint + # and cache it if Keystone is enabled. For now, it should be enough + # to check that ``secret_ref`` is a valid URL, contains 'secrets' + # substring, ends in a UUID and that the source document from which + # the reference is extracted is encrypted. + try: + secret_uuid = secret_ref.split('/')[-1] + except Exception: + secret_uuid = None + return ( + isinstance(secret_ref, six.string_types) and + cls._url_re.match(secret_ref) and + 'secrets' in secret_ref and + uuidutils.is_uuid_like(secret_uuid) + ) + @classmethod def create(cls, secret_doc): """Securely store secrets contained in ``secret_doc``. @@ -75,67 +98,39 @@ class SecretsManager(object): encryption_type = secret_doc['metadata']['storagePolicy'] secret_type = cls._get_secret_type(secret_doc['schema']) + created_secret = secret_doc['data'] - if encryption_type == ENCRYPTED: + if encryption_type == types.ENCRYPTED: # Store secret_ref in database for `secret_doc`. kwargs = { 'name': secret_doc['metadata']['name'], 'secret_type': secret_type, 'payload': secret_doc['data'] } + LOG.info('Storing encrypted document data in Barbican.') resp = cls.barbican_driver.create_secret(**kwargs) secret_ref = resp['secret_ref'] created_secret = secret_ref - elif encryption_type == CLEARTEXT: - created_secret = secret_doc['data'] return created_secret - @classmethod - def _is_barbican_ref(cls, secret_ref): - return ( - isinstance(secret_ref, six.string_types) and - cls._url_re.match(secret_ref) and 'secrets' in secret_ref - ) - @classmethod def get(cls, secret_ref): - """Return a secret payload from Barbican if ``secret_ref`` is a - Barbican secret reference or else return ``secret_ref``. + """Return a secret payload from Barbican. Extracts {secret_uuid} from a secret reference and queries Barbican's Secrets API with it. - :param str secret_ref: + :param str secret_ref: A string formatted like: + "https://{barbican_host}/v1/secrets/{secret_uuid}" + :returns: Secret payload from Barbican. - * String formatted like: - "https://{barbican_host}/v1/secrets/{secret_uuid}" - which results in a Barbican query. - * Any other string which results in a pass-through. - - :returns: Secret payload from Barbican or ``secret_ref``. """ - - # TODO(fmontei): Query Keystone service catalog for Barbican endpoint - # and cache it if Keystone is enabled. For now, it should be enough - # to check that ``secret_ref`` is a valid URL, contains 'secrets' - # substring, ends in a UUID and that the source document from which - # the reference is extracted is encrypted. - if cls._is_barbican_ref(secret_ref): - LOG.debug('Resolving Barbican secret using source document ' - 'reference...') - try: - secret_uuid = secret_ref.split('/')[-1] - except Exception: - secret_uuid = None - if not uuidutils.is_uuid_like(secret_uuid): - return secret_ref - else: - return secret_ref - + LOG.debug('Resolving Barbican secret using source document ' + 'reference...') # TODO(fmontei): Need to avoid this call if Keystone is disabled. - secret = cls.barbican_driver.get_secret(secret_ref=secret_uuid) + secret = cls.barbican_driver.get_secret(secret_ref=secret_ref) payload = secret.payload LOG.debug('Successfully retrieved Barbican secret using reference.') return payload @@ -205,6 +200,25 @@ class SecretsSubstitution(object): return to_sanitize + @staticmethod + def get_encrypted_data(src_secret, src_doc, dest_doc): + try: + src_secret = SecretsManager.get(src_secret) + except errors.BarbicanException as e: + LOG.error( + 'Failed to resolve a Barbican reference for substitution ' + 'source document [%s, %s] %s referenced in document [%s, %s] ' + '%s. Details: %s', src_doc.schema, src_doc.layer, src_doc.name, + dest_doc.schema, dest_doc.layer, dest_doc.name, + e.format_message()) + raise errors.UnknownSubstitutionError( + src_schema=src_doc.schema, src_layer=src_doc.layer, + src_name=src_doc.name, schema=dest_doc.schema, + layer=dest_doc.layer, name=dest_doc.name, + details=e.format_message()) + else: + return src_secret + def __init__(self, substitution_sources=None, fail_on_missing_sub_src=True): """SecretSubstitution constructor. @@ -358,9 +372,10 @@ class SecretsSubstitution(object): # If the document has storagePolicy == encrypted then resolve # the Barbican reference into the actual secret. - if src_doc.is_encrypted: - src_secret = self._get_encrypted_secret( - src_secret, src_doc, document) + if src_doc.is_encrypted and SecretsManager.is_barbican_ref( + src_secret): + src_secret = self.get_encrypted_data(src_secret, src_doc, + document) dest_path = sub['dest']['path'] dest_pattern = sub['dest'].get('pattern', None) diff --git a/deckhand/tests/unit/engine/test_secrets_manager.py b/deckhand/tests/unit/engine/test_secrets_manager.py index 779f9b19..e1dc8939 100644 --- a/deckhand/tests/unit/engine/test_secrets_manager.py +++ b/deckhand/tests/unit/engine/test_secrets_manager.py @@ -102,25 +102,11 @@ class TestSecretsManager(test_base.TestDbBase): def test_retrieve_barbican_secret(self): secret_ref, expected_secret = self._test_create_secret( 'encrypted', 'Certificate') - secret_uuid = secret_ref.split('/')[-1] secret_payload = secrets_manager.SecretsManager.get(secret_ref) self.assertEqual(expected_secret, secret_payload) self.mock_barbican_driver.get_secret.assert_called_once_with( - secret_ref=secret_uuid) - - -class TestSecretsManagerNegative(test_base.TestDbBase): - - def test_retrieve_barbican_secret_bad_reference_raises_exc(self): - """Verify that passing in an invalid reference to - ``SecretsManager.get`` returns gracefully with the original argument. - """ - bad_refs = ('', 'a/b', 'a/12345', 'a/%s/b' % uuidutils.generate_uuid(), - 12345, False, None, {}, []) - for bad_ref in bad_refs: - self.assertEqual( - bad_ref, secrets_manager.SecretsManager.get(bad_ref)) + secret_ref=secret_ref) class TestSecretsSubstitution(test_base.TestDbBase): diff --git a/deckhand/types.py b/deckhand/types.py index 065a7364..aa7efef7 100644 --- a/deckhand/types.py +++ b/deckhand/types.py @@ -63,3 +63,12 @@ DECKHAND_VALIDATION_TYPES = ( ) = ( 'deckhand-schema-validation', ) + + +ENCRYPTION_TYPES = ( + CLEARTEXT, + ENCRYPTED +) = ( + 'cleartext', + 'encrypted' +)