diff --git a/deckhand/common/document.py b/deckhand/common/document.py index c4d8f996..739cfc3d 100644 --- a/deckhand/common/document.py +++ b/deckhand/common/document.py @@ -173,9 +173,8 @@ class DocumentDict(dict): return [DocumentDict(d) for d in documents] @classmethod - def redact(cls, input): - return hashlib.sha256(json.dumps(input) - .encode('utf-8')).hexdigest() + def redact(cls, field): + return hashlib.sha256(json.dumps(field).encode('utf-8')).hexdigest() def document_dict_representer(dumper, data): diff --git a/deckhand/common/utils.py b/deckhand/common/utils.py index eed2f8f3..281e1618 100644 --- a/deckhand/common/utils.py +++ b/deckhand/common/utils.py @@ -393,23 +393,23 @@ def redact_document(document): """ d = _to_document(document) if d.is_encrypted: - document['data'] = document_dict.redact(d.data) - # FIXME(felipemonteiro): This block should be out-dented by 4 spaces - # because cleartext documents that substitute from encrypted documents - # should be subject to this redaction as well. However, doing this - # will result in substitution failures; the solution is to add a - # helper to :class:`deckhand.common.DocumentDict` that checks whether - # its metadata.substitutions is redacted - if so, skips substitution. - if d.substitutions: - subs = d.substitutions - for s in subs: - s['src']['path'] = document_dict.redact(s['src']['path']) - s['dest']['path'] = document_dict.redact(s['dest']['path']) - document['metadata']['substitutions'] = subs - return document + d.data = document_dict.redact(d.data) + for s in d.substitutions: + s['src']['path'] = document_dict.redact(s['src']['path']) + s['dest']['path'] = document_dict.redact(s['dest']['path']) + return d def redact_documents(documents): + """Redact sensitive data for each document in ``documents``. + + Sensitive data includes ``data``, ``substitutions[n].src.path``, and + ``substitutions[n].dest.path`` fields. + + :param list[dict] documents: List of documents whose data to redact. + :returns: Documents with redacted sensitive data. + :rtype: list[dict] + """ return [redact_document(d) for d in documents] diff --git a/deckhand/control/common.py b/deckhand/control/common.py index 73727e27..b741f20b 100644 --- a/deckhand/control/common.py +++ b/deckhand/control/common.py @@ -148,6 +148,18 @@ def invalidate_cache_data(): def get_rendered_docs(revision_id, cleartext_secrets=False, **filters): + """Helper for retrieving rendered documents for ``revision_id``. + + Retrieves raw documents from DB, renders them, and returns rendered result + set. + + :param int revision_id: Revision ID whose documents to render. + :param bool cleartext_secrets: Whether to show unencrypted data as + cleartext. + :param filters: Filters used for retrieving raw documents from DB. + :returns: List of rendered documents. + :rtype: list[dict] + """ data = _retrieve_documents_for_rendering(revision_id, **filters) documents = document_wrapper.DocumentDict.from_list(data) encryption_sources = _resolve_encrypted_data(documents) @@ -158,7 +170,8 @@ def get_rendered_docs(revision_id, cleartext_secrets=False, **filters): return engine.render( revision_id, documents, - encryption_sources=encryption_sources) + encryption_sources=encryption_sources, + cleartext_secrets=cleartext_secrets) except (errors.BarbicanClientException, errors.BarbicanServerException, errors.InvalidDocumentLayer, diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index a3a72b38..8c12de74 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -384,7 +384,8 @@ class DocumentLayering(object): documents, validate=True, fail_on_missing_sub_src=True, - encryption_sources=None): + encryption_sources=None, + cleartext_secrets=False): """Contructor for ``DocumentLayering``. :param layering_policy: The document with schema @@ -405,6 +406,9 @@ class DocumentLayering(object): actual unecrypted data. If encrypting data with Barbican, the reference will be a Barbican secret reference. :type encryption_sources: dict + :param cleartext_secrets: Whether to show unencrypted data as + cleartext. + :type cleartext_secrets: bool :raises LayeringPolicyNotFound: If no LayeringPolicy was found among list of ``documents``. @@ -482,7 +486,8 @@ class DocumentLayering(object): self.secrets_substitution = secrets_manager.SecretsSubstitution( substitution_sources, encryption_sources=encryption_sources, - fail_on_missing_sub_src=fail_on_missing_sub_src) + fail_on_missing_sub_src=fail_on_missing_sub_src, + cleartext_secrets=cleartext_secrets) self._sorted_documents = self._topologically_sort_documents( substitution_sources) diff --git a/deckhand/engine/render.py b/deckhand/engine/render.py index 86f75487..8d714820 100644 --- a/deckhand/engine/render.py +++ b/deckhand/engine/render.py @@ -24,7 +24,8 @@ __all__ = ('render', 'validate_render') -def render(revision_id, documents, encryption_sources=None): +def render(revision_id, documents, encryption_sources=None, + cleartext_secrets=False): """Render revision documents for ``revision_id`` using raw ``documents``. :param revision_id: Key used for caching rendered documents by. @@ -37,6 +38,8 @@ def render(revision_id, documents, encryption_sources=None): actual unecrypted data. If encrypting data with Barbican, the reference will be a Barbican secret reference. :type encryption_sources: dict + :param cleartext_secrets: Whether to show unencrypted data as cleartext. + :type cleartext_secrets: bool :returns: Rendered documents for ``revision_id``. :rtype: List[dict] @@ -49,7 +52,8 @@ def render(revision_id, documents, encryption_sources=None): revision_id, documents, encryption_sources=encryption_sources, - validate=False) + validate=False, + cleartext_secrets=cleartext_secrets) def validate_render(revision_id, rendered_documents, validator): diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index 0ef1081f..860aacf6 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -20,7 +20,7 @@ from oslo_log import log as logging import six from deckhand.barbican.driver import BarbicanDriver -from deckhand.common import document as document_wrapper +from deckhand.common.document import DocumentDict as dd from deckhand.common import utils from deckhand import errors @@ -38,7 +38,7 @@ class SecretsManager(object): @staticmethod def requires_encryption(document): - clazz = document_wrapper.DocumentDict + clazz = dd if not isinstance(document, clazz): document = clazz(document) return document.is_encrypted @@ -64,8 +64,8 @@ class SecretsManager(object): # TODO(fmontei): Look into POSTing Deckhand metadata into Barbican's # Secrets Metadata API to make it easier to track stale secrets from # prior revisions that need to be deleted. - if not isinstance(secret_doc, document_wrapper.DocumentDict): - secret_doc = document_wrapper.DocumentDict(secret_doc) + if not isinstance(secret_doc, dd): + secret_doc = dd(secret_doc) if secret_doc.is_encrypted: payload = cls.barbican_driver.create_secret(secret_doc) @@ -100,8 +100,8 @@ class SecretsManager(object): :returns: None """ - if not isinstance(document, document_wrapper.DocumentDict): - document = document_wrapper.DocumentDict(document) + if not isinstance(document, dd): + document = dd(document) secret_ref = document.data if document.is_encrypted and document.has_barbican_ref: @@ -116,7 +116,7 @@ class SecretsSubstitution(object): """Class for document substitution logic for YAML files.""" __slots__ = ('_fail_on_missing_sub_src', '_substitution_sources', - '_encryption_sources') + '_encryption_sources', '_cleartext_secrets') _insecure_reg_exps = ( re.compile(r'^.* is not of type .+$'), @@ -169,7 +169,9 @@ class SecretsSubstitution(object): return self._encryption_sources[secret_ref] def __init__(self, substitution_sources=None, - fail_on_missing_sub_src=True, encryption_sources=None): + fail_on_missing_sub_src=True, + encryption_sources=None, + cleartext_secrets=False): """SecretSubstitution constructor. This class will automatically detect documents that require @@ -187,6 +189,9 @@ class SecretsSubstitution(object): actual unecrypted data. If encrypting data with Barbican, the reference will be a Barbican secret reference. :type encryption_sources: dict + :param cleartext_secrets: Whether to show unencrypted data as + cleartext. + :type cleartext_secrets: bool """ # This maps a 2-tuple of (schema, name) to a document from which the @@ -196,14 +201,15 @@ class SecretsSubstitution(object): self._substitution_sources = {} self._encryption_sources = encryption_sources or {} self._fail_on_missing_sub_src = fail_on_missing_sub_src + self._cleartext_secrets = cleartext_secrets if isinstance(substitution_sources, dict): self._substitution_sources = substitution_sources else: self._substitution_sources = dict() for document in substitution_sources: - if not isinstance(document, document_wrapper.DocumentDict): - document = document_wrapper.DocumentDict(document) + if not isinstance(document, dd): + document = dd(document) if document.schema and document.name: self._substitution_sources.setdefault( (document.schema, document.name), document) @@ -235,6 +241,43 @@ class SecretsSubstitution(object): src_doc.name, dest_doc.schema, dest_doc.layer, dest_doc.name) + def _substitute_one(self, document, src_doc, src_secret, dest_path, + dest_pattern, dest_recurse=None): + dest_recurse = dest_recurse or {} + exc_message = '' + try: + substituted_data = utils.jsonpath_replace( + document['data'], src_secret, dest_path, + pattern=dest_pattern, recurse=dest_recurse) + if (isinstance(document['data'], dict) and + isinstance(substituted_data, dict)): + document['data'].update(substituted_data) + elif substituted_data: + document['data'] = substituted_data + else: + exc_message = ( + 'Failed to create JSON path "%s" in the ' + 'destination document [%s, %s] %s. ' + 'No data was substituted.' % ( + dest_path, document.schema, + document.layer, document.name)) + except Exception as e: + LOG.error('Unexpected exception occurred ' + 'while attempting ' + 'substitution using ' + 'source document [%s, %s] %s ' + 'referenced in [%s, %s] %s. Details: %s', + src_doc.schema, src_doc.name, src_doc.layer, + document.schema, document.layer, + document.name, + six.text_type(e)) + exc_message = six.text_type(e) + finally: + if exc_message: + self._handle_unknown_substitution_exc( + exc_message, src_doc, document) + return document + def substitute_all(self, documents): """Substitute all documents that have a `metadata.substitutions` field. @@ -259,8 +302,8 @@ class SecretsSubstitution(object): documents = [documents] for document in documents: - if not isinstance(document, document_wrapper.DocumentDict): - document = document_wrapper.DocumentDict(document) + if not isinstance(document, dd): + document = dd(document) # If the document has substitutions include it. if document.substitutions: documents_to_substitute.append(document) @@ -322,43 +365,26 @@ class SecretsSubstitution(object): dest_pattern = each_dest_path.get('pattern', None) dest_recurse = each_dest_path.get('recurse', {}) + # If the source document is encrypted and cleartext_secrets + # is False, then redact the substitution metadata in the + # destination document to prevent reverse-engineering of + # where the sensitive data came from. + if src_doc.is_encrypted and not self._cleartext_secrets: + sub['src']['path'] = dd.redact(src_path) + sub['dest']['path'] = dd.redact(dest_path) + LOG.debug('Substituting from schema=%s layer=%s name=%s ' 'src_path=%s into dest_path=%s, dest_pattern=%s', src_schema, src_doc.layer, src_name, src_path, dest_path, dest_pattern) - try: - exc_message = '' - substituted_data = utils.jsonpath_replace( - document['data'], src_secret, dest_path, - pattern=dest_pattern, recurse=dest_recurse) - if (isinstance(document['data'], dict) and - isinstance(substituted_data, dict)): - document['data'].update(substituted_data) - elif substituted_data: - document['data'] = substituted_data - else: - exc_message = ( - 'Failed to create JSON path "%s" in the ' - 'destination document [%s, %s] %s. ' - 'No data was substituted.' % ( - dest_path, document.schema, - document.layer, document.name)) - except Exception as e: - LOG.error('Unexpected exception occurred ' - 'while attempting ' - 'substitution using ' - 'source document [%s, %s] %s ' - 'referenced in [%s, %s] %s. Details: %s', - src_schema, src_name, src_doc.layer, - document.schema, document.layer, - document.name, - six.text_type(e)) - exc_message = six.text_type(e) - finally: - if exc_message: - self._handle_unknown_substitution_exc( - exc_message, src_doc, document) + document = self._substitute_one( + document, + src_doc=src_doc, + src_secret=src_secret, + dest_path=dest_path, + dest_pattern=dest_pattern, + dest_recurse=dest_recurse) yield document diff --git a/deckhand/tests/unit/control/test_rendered_documents_controller.py b/deckhand/tests/unit/control/test_rendered_documents_controller.py index b5fdf4cb..0ed4c88f 100644 --- a/deckhand/tests/unit/control/test_rendered_documents_controller.py +++ b/deckhand/tests/unit/control/test_rendered_documents_controller.py @@ -16,6 +16,7 @@ import yaml import mock +from deckhand.common.document import DocumentDict as dd from deckhand.control import revision_documents from deckhand.engine import secrets_manager from deckhand import errors @@ -200,6 +201,10 @@ class TestRenderedDocumentsController(test_base.BaseControllerTest): class TestRenderedDocumentsControllerRedaction(test_base.BaseControllerTest): def _test_list_rendered_documents(self, cleartext_secrets): + """Validates that destination document that substitutes from an + encrypted document is appropriately redacted when ``cleartext_secrets`` + is True. + """ rules = { 'deckhand:list_cleartext_documents': '@', 'deckhand:list_encrypted_documents': '@', @@ -215,6 +220,7 @@ class TestRenderedDocumentsControllerRedaction(test_base.BaseControllerTest): certificate_data = 'sample-certificate' certificate_ref = ('http://127.0.0.1/key-manager/v1/secrets/%s' % test_utils.rand_uuid_hex()) + redacted_data = dd.redact(certificate_ref) doc1 = { 'data': certificate_data, @@ -228,6 +234,11 @@ class TestRenderedDocumentsControllerRedaction(test_base.BaseControllerTest): 'layer': 'site'}, 'storagePolicy': 'encrypted', 'replacement': False}} + original_substitutions = [ + {'dest': {'path': '.'}, + 'src': {'schema': 'deckhand/Certificate/v1', + 'name': 'example-cert', 'path': '.'}} + ] doc2 = {'data': {}, 'schema': 'example/Kind/v1', 'name': 'deckhand-global', 'layer': 'global', 'metadata': { @@ -236,10 +247,8 @@ class TestRenderedDocumentsControllerRedaction(test_base.BaseControllerTest): 'layeringDefinition': {'abstract': False, 'layer': 'global'}, 'name': 'deckhand-global', - 'schema': 'metadata/Document/v1', 'substitutions': [ - {'dest': {'path': '.'}, - 'src': {'schema': 'deckhand/Certificate/v1', - 'name': 'example-cert', 'path': '.'}}], + 'schema': 'metadata/Document/v1', + 'substitutions': original_substitutions, 'replacement': False}} payload = [layering_policy, doc1, doc2] @@ -279,10 +288,15 @@ class TestRenderedDocumentsControllerRedaction(test_base.BaseControllerTest): self.assertTrue(all(map(lambda x: x['data'] == certificate_data, rendered_documents))) else: - # Expected redacted data for both documents to be returned - + # Expect redacted data for both documents to be returned - # because the destination document should receive redacted data. - self.assertTrue(all(map(lambda x: x['data'] != certificate_data, + self.assertTrue(all(map(lambda x: x['data'] == redacted_data, rendered_documents))) + destination_doc = next(iter(filter( + lambda x: x['metadata']['name'] == 'deckhand-global', + rendered_documents))) + substitutions = destination_doc['metadata']['substitutions'] + self.assertNotEqual(original_substitutions, substitutions) def test_list_rendered_documents_cleartext_secrets_true(self): self._test_list_rendered_documents(cleartext_secrets=True) diff --git a/deckhand/tests/unit/engine/test_secrets_manager.py b/deckhand/tests/unit/engine/test_secrets_manager.py index ac733afb..7af40607 100644 --- a/deckhand/tests/unit/engine/test_secrets_manager.py +++ b/deckhand/tests/unit/engine/test_secrets_manager.py @@ -176,7 +176,8 @@ class TestSecretsSubstitution(test_base.TestDbBase): self.secrets_factory = factories.DocumentSecretFactory() def _test_doc_substitution(self, document_mapping, substitution_sources, - expected_data, encryption_sources=None): + expected_data, encryption_sources=None, + cleartext_secrets=True): payload = self.document_factory.gen_test(document_mapping, global_abstract=False) bucket_name = test_utils.rand_name('bucket') @@ -188,7 +189,8 @@ class TestSecretsSubstitution(test_base.TestDbBase): secret_substitution = secrets_manager.SecretsSubstitution( encryption_sources=encryption_sources, - substitution_sources=substitution_sources) + substitution_sources=substitution_sources, + cleartext_secrets=cleartext_secrets) substituted_docs = list(secret_substitution.substitute_all(documents)) self.assertIn(expected_document, substituted_docs)