diff --git a/deckhand/common/utils.py b/deckhand/common/utils.py index 2645e43d..362b99f6 100644 --- a/deckhand/common/utils.py +++ b/deckhand/common/utils.py @@ -394,13 +394,17 @@ def redact_document(document): :returns: Document with redacted data. :rtype: dict """ - d = _to_document(document) - if d.is_encrypted: - 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 + doc = _to_document(document) + if doc.is_encrypted: + doc.data = document_dict.redact(doc.data) + for sub in doc.substitutions: + sub['src']['path'] = document_dict.redact(sub['src']['path']) + if isinstance(sub['dest'], list): + for dest in sub['dest']: + dest['path'] = document_dict.redact(dest['path']) + else: + sub['dest']['path'] = document_dict.redact(sub['dest']['path']) + return doc def redact_documents(documents): diff --git a/deckhand/control/common.py b/deckhand/control/common.py index b741f20b..b33bc395 100644 --- a/deckhand/control/common.py +++ b/deckhand/control/common.py @@ -23,7 +23,6 @@ import six from deckhand.barbican import cache as barbican_cache from deckhand.common import document as document_wrapper -from deckhand.common import utils from deckhand.db.sqlalchemy import api as db_api from deckhand import engine from deckhand.engine import cache as engine_cache @@ -164,8 +163,6 @@ def get_rendered_docs(revision_id, cleartext_secrets=False, **filters): documents = document_wrapper.DocumentDict.from_list(data) encryption_sources = _resolve_encrypted_data(documents) - if not cleartext_secrets: - documents = utils.redact_documents(documents) try: return engine.render( revision_id, diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index 122ced04..5a4fc014 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -55,6 +55,8 @@ class RevisionDocumentsResource(api_base.BaseResource): sort_by = req.params.pop('sort', None) limit = req.params.pop('limit', None) cleartext_secrets = req.get_param_as_bool('cleartext-secrets') + if cleartext_secrets is None: + cleartext_secrets = True req.params.pop('cleartext-secrets', None) filters = req.params.copy() @@ -114,6 +116,8 @@ class RenderedDocumentsResource(api_base.BaseResource): filters['metadata.storagePolicy'].append('encrypted') cleartext_secrets = req.get_param_as_bool('cleartext-secrets') + if cleartext_secrets is None: + cleartext_secrets = True req.params.pop('cleartext-secrets', None) rendered_documents, cache_hit = common.get_rendered_docs( revision_id, cleartext_secrets, **filters) @@ -137,6 +141,9 @@ class RenderedDocumentsResource(api_base.BaseResource): limit = req.params.pop('limit', None) user_filters = req.params.copy() + if not cleartext_secrets: + rendered_documents = utils.redact_documents(rendered_documents) + rendered_documents = [ d for d in rendered_documents if utils.deepfilter( d, **user_filters)] diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index d2d199a6..4160c8e3 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -623,6 +623,24 @@ class DocumentLayering(object): if doc.is_control: continue + # 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. + if doc.is_encrypted and doc.has_barbican_ref: + encrypted_data = self.secrets_substitution\ + .get_unencrypted_data( + secret_ref=doc.data, + src_doc=doc, + dest_doc=doc + ) + if not doc.is_abstract: + doc.data = encrypted_data + self.secrets_substitution.update_substitution_sources( + meta=doc.meta, + data=encrypted_data + ) + self._documents_by_index[doc.meta].data = encrypted_data + LOG.debug("Rendering document %s:%s:%s", *doc.meta) if doc.parent_selector: @@ -633,15 +651,15 @@ class DocumentLayering(object): parent = self._documents_by_index[parent_meta] if doc.actions: - rendered_data = parent + rendered_doc = parent # Apply each action to the current document. for action in doc.actions: LOG.debug('Applying action %s to document with ' 'schema=%s, layer=%s, name=%s.', action, *doc.meta) try: - rendered_data = self._apply_action( - action, doc, rendered_data) + rendered_doc = self._apply_action( + action, doc, rendered_doc) except Exception: with excutils.save_and_reraise_exception(): try: @@ -649,10 +667,10 @@ class DocumentLayering(object): doc, parent, action) except Exception: # nosec pass - doc.data = rendered_data.data + doc.data = rendered_doc.data self.secrets_substitution.update_substitution_sources( - doc.meta, rendered_data.data) - self._documents_by_index[doc.meta] = rendered_data + doc.meta, rendered_doc.data) + self._documents_by_index[doc.meta] = rendered_doc else: LOG.debug( 'Skipped layering for document [%s, %s] %s which ' @@ -664,7 +682,7 @@ class DocumentLayering(object): # inherit from it, but only update the document's data if concrete. if doc.substitutions: try: - substituted_data = list( + substituted_doc = list( self.secrets_substitution.substitute_all(doc)) except Exception: with excutils.save_and_reraise_exception(): @@ -672,25 +690,14 @@ class DocumentLayering(object): self._log_data_for_substitution_failure(doc) except Exception: # nosec pass - if substituted_data: - rendered_data = substituted_data[0] + if substituted_doc: + rendered_doc = substituted_doc[0] # Update the actual document data if concrete. - doc.data = rendered_data.data + doc.data = rendered_doc.data if not doc.has_replacement: self.secrets_substitution.update_substitution_sources( - doc.meta, 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 and doc.has_barbican_ref: - encrypted_data = self.secrets_substitution\ - .get_unencrypted_data(doc.data, doc, doc) - if not doc.is_abstract: - doc.data = encrypted_data - self.secrets_substitution.update_substitution_sources( - doc.meta, encrypted_data) - self._documents_by_index[doc.meta] = encrypted_data + doc.meta, rendered_doc.data) + self._documents_by_index[doc.meta] = rendered_doc # NOTE: Since the child-replacement is always prioritized, before # other children, as soon as the child-replacement layers with the diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index a4c8f5c8..6ee035ee 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -247,13 +247,13 @@ class SecretsSubstitution(object): exc_message = '' try: substituted_data = utils.jsonpath_replace( - document['data'], src_secret, dest_path, + document.data, src_secret, dest_path, pattern=dest_pattern, recurse=dest_recurse) - if (isinstance(document['data'], dict) and + if (isinstance(document.data, dict) and isinstance(substituted_data, dict)): - document['data'].update(substituted_data) + document.data.update(substituted_data) elif substituted_data: - document['data'] = substituted_data + document.data = substituted_data else: exc_message = ( 'Failed to create JSON path "%s" in the ' @@ -313,6 +313,7 @@ class SecretsSubstitution(object): for d in documents_to_substitute])) for document in documents_to_substitute: + redact_dest = False LOG.debug('Checking for substitutions for document [%s, %s] %s.', *document.meta) for sub in document.substitutions: @@ -337,6 +338,9 @@ class SecretsSubstitution(object): LOG.warning(message) continue + if src_doc.is_encrypted: + redact_dest = True + # If the data is a dictionary, retrieve the nested secret # via jsonpath_parse, else the secret is the primitive/string # stored in the data section itself. @@ -391,6 +395,13 @@ class SecretsSubstitution(object): dest_pattern=dest_pattern, dest_recurse=dest_recurse) + # If we just substituted from an encrypted document + # into a cleartext document, we need to redact the + # dest document as well so the secret stays hidden + if (not document.is_encrypted and redact_dest and not + self._cleartext_secrets): + document.storage_policy = 'encrypted' + yield document def update_substitution_sources(self, meta, data): diff --git a/deckhand/tests/unit/control/test_rendered_documents_controller.py b/deckhand/tests/unit/control/test_rendered_documents_controller.py index 0ed4c88f..9a022c21 100644 --- a/deckhand/tests/unit/control/test_rendered_documents_controller.py +++ b/deckhand/tests/unit/control/test_rendered_documents_controller.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import re import yaml import mock @@ -220,7 +221,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) + redacted_data = dd.redact(certificate_data) doc1 = { 'data': certificate_data, @@ -305,8 +306,151 @@ class TestRenderedDocumentsControllerRedaction(test_base.BaseControllerTest): self._test_list_rendered_documents(cleartext_secrets=False) -class TestRenderedDocumentsControllerNegative( - test_base.BaseControllerTest): +class TestRenderedDocumentsControllerEncrypted(test_base.BaseControllerTest): + + def test_substitution_rendered_documents_all_encrypted(self): + """Validates that substitution still functions correctly when all + the documents have a storagePolicy of encrypted + """ + rules = { + 'deckhand:list_cleartext_documents': '@', + 'deckhand:list_encrypted_documents': '@', + 'deckhand:create_cleartext_documents': '@', + 'deckhand:create_encrypted_documents': '@' + } + + self.policy.set_rules(rules) + + doc_factory = factories.DocumentFactory(1, [1]) + + layering_policy = doc_factory.gen_test({})[0] + layering_policy['data']['layerOrder'] = ['global', 'site'] + password_data = 'sample-super-secret-password' + original_data = 'http://admin:PASSWORD@service-name:8080/v1' + doc1_ref = ('http://127.0.0.1/key-manager/v1/secrets/%s' + % test_utils.rand_uuid_hex()) + doc2_ref = ('http://127.0.0.1/key-manager/v1/secrets/%s' + % test_utils.rand_uuid_hex()) + dest_data = { + 'url': original_data + } + subbed_data = { + 'url': re.sub('PASSWORD', password_data, original_data) + } + redacted_password = dd.redact(password_data) + redacted_data = dd.redact(subbed_data) + + original_substitutions = [ + { + 'dest': { + 'path': '.url', + 'pattern': 'PASSWORD' + }, + 'src': { + 'schema': 'deckhand/Passphrase/v1', + 'name': 'example-password', + 'path': '.' + } + } + ] + + # source + doc1 = { + 'data': password_data, + 'schema': 'deckhand/Passphrase/v1', + 'name': 'example-password', + 'layer': 'site', + 'metadata': { + 'labels': { + 'site': 'site1' + }, + 'storagePolicy': 'encrypted', + 'layeringDefinition': { + 'abstract': False, + 'layer': 'site' + }, + 'name': 'example-password', + 'schema': 'metadata/Document/v1', + 'replacement': False + } + } + + # destination + doc2 = { + 'data': dest_data, + 'schema': 'example/Kind/v1', + 'name': 'deckhand-global', + 'layer': 'global', + 'metadata': { + 'labels': { + 'global': 'global1' + }, + 'storagePolicy': 'encrypted', + 'layeringDefinition': { + 'abstract': False, + 'layer': 'global' + }, + 'name': 'deckhand-global', + 'schema': 'metadata/Document/v1', + 'substitutions': original_substitutions, + 'replacement': False + } + } + + payload = [layering_policy, doc1, doc2] + + # Create both documents and mock out SecretsManager.create to return + # a fake Barbican ref. + with mock.patch.object( # noqa + secrets_manager.SecretsManager, 'create', + side_effect=[doc1_ref, doc2_ref] + ): + resp = self.app.simulate_put( + '/api/v1.0/buckets/mop/documents', + headers={'Content-Type': 'application/x-yaml'}, + body=yaml.safe_dump_all(payload)) + self.assertEqual(200, resp.status_code) + revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][ + 'revision'] + + # Retrieve rendered documents and simulate a Barbican lookup by + # returning both the password and the destination data + with mock.patch( # noqa + 'deckhand.control.common._resolve_encrypted_data', + return_value={ + doc1_ref: password_data, + doc2_ref: dest_data + } + ): + resp = self.app.simulate_get( + '/api/v1.0/revisions/%s/rendered-documents' % revision_id, + headers={'Content-Type': 'application/x-yaml'}, + params={ + 'metadata.name': ['example-password', 'deckhand-global'], + 'cleartext-secrets': False + }, + params_csv=False) + + self.assertEqual(200, resp.status_code) + rendered_documents = list(yaml.safe_load_all(resp.text)) + self.assertEqual(2, len(rendered_documents)) + + # Expect redacted data for all documents to be returned - + # because the destination documents should receive redacted data. + data = list(map(lambda x: x['data'], rendered_documents)) + self.assertTrue(redacted_password in data) + self.assertTrue(redacted_data in data) + + # Expect the substitutions to be redacted since both docs are + # marked as encrypted + destination_doc = next(iter(filter( + lambda x: x['metadata']['name'] == 'deckhand-global', + rendered_documents))) + substitutions = destination_doc['metadata']['substitutions'] + self.assertNotEqual(original_substitutions, substitutions) + + +class TestRenderedDocumentsControllerNegative(test_base.BaseControllerTest): def test_rendered_documents_fail_schema_validation(self): """Validates that when fully rendered documents fail basic schema