From 7defe473d2e1111ba1738610cbecaffd6ff6151d Mon Sep 17 00:00:00 2001 From: "anthony.bellino" Date: Tue, 16 Oct 2018 21:04:55 +0000 Subject: [PATCH] Redact rendered Documents - Uses the rendered-documents endpoint - Adds a query parameter ?cleartext-secrets - Adds unit tests, updates integration tests Change-Id: I02423b9bf7456008d707b3cd91edc4fc281fa5fc --- deckhand/common/utils.py | 12 +++ deckhand/control/common.py | 10 +- deckhand/control/revision_documents.py | 32 ++++--- deckhand/control/revisions.py | 8 +- deckhand/engine/layering.py | 2 +- .../document-render-secret-edge-cases.yaml | 1 + .../gabbits/document-render-secret.yaml | 1 + .../document-substitution-secret-generic.yaml | 1 + .../gabbits/document-substitution-secret.yaml | 1 + deckhand/tests/unit/common/test_utils.py | 46 +++++++++ .../test_rendered_documents_controller.py | 95 +++++++++++++++++++ 11 files changed, 188 insertions(+), 21 deletions(-) diff --git a/deckhand/common/utils.py b/deckhand/common/utils.py index 8e68e110..eed2f8f3 100644 --- a/deckhand/common/utils.py +++ b/deckhand/common/utils.py @@ -385,9 +385,21 @@ def deepfilter(dct, **filters): def redact_document(document): + """Redact ``data`` and ``substitutions`` sections for ``document``. + + :param dict document: Document whose data to redact. + :returns: Document with redacted data. + :rtype: dict + """ 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: diff --git a/deckhand/control/common.py b/deckhand/control/common.py index 75a2a8d2..73727e27 100644 --- a/deckhand/control/common.py +++ b/deckhand/control/common.py @@ -23,6 +23,7 @@ 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 @@ -130,7 +131,9 @@ def sanitize_params(allowed_params): else: sanitized_params[key] = param_val - func_args = func_args + (sanitized_params,) + req.params.clear() + req.params.update(sanitized_params) + return func(self, req, *func_args, **func_kwargs) return wrapper @@ -144,10 +147,13 @@ def invalidate_cache_data(): engine_cache.invalidate() -def get_rendered_docs(revision_id, **filters): +def get_rendered_docs(revision_id, cleartext_secrets=False, **filters): data = _retrieve_documents_for_rendering(revision_id, **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 db39097c..122ced04 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -40,7 +40,7 @@ class RevisionDocumentsResource(api_base.BaseResource): 'schema', 'metadata.name', 'metadata.layeringDefinition.abstract', 'metadata.layeringDefinition.layer', 'metadata.label', 'status.bucket', 'order', 'sort', 'limit', 'cleartext-secrets']) - def on_get(self, req, resp, sanitized_params, revision_id): + def on_get(self, req, resp, revision_id): """Returns all documents for a `revision_id`. Returns a multi-document YAML response containing all the documents @@ -51,12 +51,13 @@ class RevisionDocumentsResource(api_base.BaseResource): include_encrypted = policy.conditional_authorize( 'deckhand:list_encrypted_documents', req.context, do_raise=False) - order_by = sanitized_params.pop('order', None) - sort_by = sanitized_params.pop('sort', None) - limit = sanitized_params.pop('limit', None) - cleartext_secrets = sanitized_params.pop('cleartext-secrets', None) + order_by = req.params.pop('order', None) + sort_by = req.params.pop('sort', None) + limit = req.params.pop('limit', None) + cleartext_secrets = req.get_param_as_bool('cleartext-secrets') + req.params.pop('cleartext-secrets', None) - filters = sanitized_params.copy() + filters = req.params.copy() filters['metadata.storagePolicy'] = ['cleartext'] if include_encrypted: filters['metadata.storagePolicy'].append('encrypted') @@ -69,7 +70,7 @@ class RevisionDocumentsResource(api_base.BaseResource): LOG.exception(six.text_type(e)) raise falcon.HTTPNotFound(description=e.format_message()) - if cleartext_secrets not in [True, 'true', 'True']: + if not cleartext_secrets: documents = utils.redact_documents(documents) # Sorts by creation date by default. @@ -100,8 +101,9 @@ class RenderedDocumentsResource(api_base.BaseResource): @policy.authorize('deckhand:list_cleartext_documents') @common.sanitize_params([ 'schema', 'metadata.name', 'metadata.layeringDefinition.layer', - 'metadata.label', 'status.bucket', 'order', 'sort', 'limit']) - def on_get(self, req, resp, sanitized_params, revision_id): + 'metadata.label', 'status.bucket', 'order', 'sort', 'limit', + 'cleartext-secrets']) + def on_get(self, req, resp, revision_id): include_encrypted = policy.conditional_authorize( 'deckhand:list_encrypted_documents', req.context, do_raise=False) filters = { @@ -111,8 +113,10 @@ class RenderedDocumentsResource(api_base.BaseResource): if include_encrypted: filters['metadata.storagePolicy'].append('encrypted') + cleartext_secrets = req.get_param_as_bool('cleartext-secrets') + req.params.pop('cleartext-secrets', None) rendered_documents, cache_hit = common.get_rendered_docs( - revision_id, **filters) + revision_id, cleartext_secrets, **filters) # If the rendered documents result set is cached, then post-validation # for that result set has already been performed successfully, so it @@ -128,10 +132,10 @@ class RenderedDocumentsResource(api_base.BaseResource): # involved in rendering. User filters can only be applied once all # documents have been rendered. Note that `layering` module only # returns concrete documents, so no filtering for that is needed here. - order_by = sanitized_params.pop('order', None) - sort_by = sanitized_params.pop('sort', None) - limit = sanitized_params.pop('limit', None) - user_filters = sanitized_params.copy() + order_by = req.params.pop('order', None) + sort_by = req.params.pop('sort', None) + limit = req.params.pop('limit', None) + user_filters = req.params.copy() rendered_documents = [ d for d in rendered_documents if utils.deepfilter( diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index c9f3d8df..fd66823a 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -64,11 +64,11 @@ class RevisionsResource(api_base.BaseResource): @policy.authorize('deckhand:list_revisions') @common.sanitize_params(['tag', 'order', 'sort']) - def _list_revisions(self, req, resp, sanitized_params): - order_by = sanitized_params.pop('order', None) - sort_by = sanitized_params.pop('sort', None) + def _list_revisions(self, req, resp): + order_by = req.params.pop('order', None) + sort_by = req.params.pop('sort', None) - revisions = db_api.revision_get_all(**sanitized_params) + revisions = db_api.revision_get_all(**req.params) if sort_by: revisions = utils.multisort(revisions, sort_by, order_by) diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index 52c5aefb..a3a72b38 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -708,7 +708,7 @@ class DocumentLayering(object): # 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: + 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: diff --git a/deckhand/tests/integration/gabbits/document-render-secret-edge-cases.yaml b/deckhand/tests/integration/gabbits/document-render-secret-edge-cases.yaml index c1f63792..43742959 100644 --- a/deckhand/tests/integration/gabbits/document-render-secret-edge-cases.yaml +++ b/deckhand/tests/integration/gabbits/document-render-secret-edge-cases.yaml @@ -185,6 +185,7 @@ tests: content-type: application/x-yaml query_parameters: metadata.name: armada-doc + cleartext-secrets: true response_multidoc_jsonpaths: $.`len`: 1 $.[0].data: diff --git a/deckhand/tests/integration/gabbits/document-render-secret.yaml b/deckhand/tests/integration/gabbits/document-render-secret.yaml index ab6e0596..abccfd43 100644 --- a/deckhand/tests/integration/gabbits/document-render-secret.yaml +++ b/deckhand/tests/integration/gabbits/document-render-secret.yaml @@ -52,6 +52,7 @@ tests: GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/rendered-documents status: 200 query_parameters: + cleartext-secrets: true metadata.name: my-passphrase response_multidoc_jsonpaths: $.`len`: 1 diff --git a/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml b/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml index 9e282bba..f5ffb2c1 100644 --- a/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml +++ b/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml @@ -100,6 +100,7 @@ tests: GET: /api/v1.0/revisions/$HISTORY['encrypt_generic_document_for_secret_substitution'].$RESPONSE['$.[0].status.revision']/rendered-documents status: 200 query_parameters: + cleartext-secrets: true metadata.name: - armada-chart-01 - example-armada-cert diff --git a/deckhand/tests/integration/gabbits/document-substitution-secret.yaml b/deckhand/tests/integration/gabbits/document-substitution-secret.yaml index e7b81382..44946535 100644 --- a/deckhand/tests/integration/gabbits/document-substitution-secret.yaml +++ b/deckhand/tests/integration/gabbits/document-substitution-secret.yaml @@ -242,6 +242,7 @@ tests: response_headers: content-type: application/x-yaml query_parameters: + cleartext-secrets: true sort: 'metadata.name' response_multidoc_jsonpaths: $.`len`: 9 diff --git a/deckhand/tests/unit/common/test_utils.py b/deckhand/tests/unit/common/test_utils.py index 42928de4..e04590f4 100644 --- a/deckhand/tests/unit/common/test_utils.py +++ b/deckhand/tests/unit/common/test_utils.py @@ -12,14 +12,17 @@ # See the License for the specific language governing permissions and # limitations under the License. +import hashlib import jsonpath_ng import mock +from oslo_serialization import jsonutils as json from testtools.matchers import Equals from testtools.matchers import MatchesAny from deckhand.common import utils from deckhand import errors +from deckhand import factories from deckhand.tests.unit import base as test_base @@ -241,3 +244,46 @@ class TestJSONPathUtilsCaching(test_base.DeckhandTestCase): # in case CI jobs clash.) self.assertThat( self.jsonpath_call_count, MatchesAny(Equals(0), Equals(1))) + + +class TestRedactDocuments(test_base.DeckhandTestCase): + """Validate Redact function works""" + + def test_redact_rendered_document(self): + + self.factory = factories.DocumentSecretFactory() + mapping = { + "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, + "_GLOBAL_SUBSTITUTIONS_1_": [{ + "dest": { + "path": ".c" + }, + "src": { + "schema": "deckhand/Certificate/v1", + "name": "global-cert", + "path": "." + } + }] + } + data = mapping['_GLOBAL_DATA_1_']['data'] + doc_factory = factories.DocumentFactory(1, [1]) + document = doc_factory.gen_test( + mapping, global_abstract=False)[-1] + document['metadata']['storagePolicy'] = 'encrypted' + + with mock.patch.object(hashlib, 'sha256', autospec=True, + return_value=mock.sentinel.redacted)\ + as mock_sha256: + redacted = mock.MagicMock() + mock_sha256.return_value = redacted + redacted.hexdigest.return_value = json.dumps(data) + mock.sentinel.redacted = redacted.hexdigest.return_value + redacted_doc = utils.redact_document(document) + + self.assertEqual(mock.sentinel.redacted, redacted_doc['data']) + self.assertEqual(mock.sentinel.redacted, + redacted_doc['metadata']['substitutions'][0] + ['src']['path']) + self.assertEqual(mock.sentinel.redacted, + redacted_doc['metadata']['substitutions'][0] + ['dest']['path']) diff --git a/deckhand/tests/unit/control/test_rendered_documents_controller.py b/deckhand/tests/unit/control/test_rendered_documents_controller.py index d671f799..b5fdf4cb 100644 --- a/deckhand/tests/unit/control/test_rendered_documents_controller.py +++ b/deckhand/tests/unit/control/test_rendered_documents_controller.py @@ -20,6 +20,7 @@ from deckhand.control import revision_documents from deckhand.engine import secrets_manager from deckhand import errors from deckhand import factories +from deckhand.tests import test_utils from deckhand.tests.unit.control import base as test_base from deckhand import types @@ -196,6 +197,100 @@ class TestRenderedDocumentsController(test_base.BaseControllerTest): self.assertEqual([4, 4], second_revision_ids) +class TestRenderedDocumentsControllerRedaction(test_base.BaseControllerTest): + + def _test_list_rendered_documents(self, cleartext_secrets): + 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'] + certificate_data = 'sample-certificate' + certificate_ref = ('http://127.0.0.1/key-manager/v1/secrets/%s' + % test_utils.rand_uuid_hex()) + + doc1 = { + 'data': certificate_data, + 'schema': 'deckhand/Certificate/v1', 'name': 'example-cert', + 'layer': 'site', + 'metadata': { + 'schema': 'metadata/Document/v1', + 'name': 'example-cert', + 'layeringDefinition': { + 'abstract': False, + 'layer': 'site'}, 'storagePolicy': 'encrypted', + 'replacement': False}} + + doc2 = {'data': {}, 'schema': 'example/Kind/v1', + 'name': 'deckhand-global', 'layer': 'global', + 'metadata': { + 'labels': {'global': 'global1'}, + 'storagePolicy': 'cleartext', + 'layeringDefinition': {'abstract': False, + 'layer': 'global'}, + 'name': 'deckhand-global', + 'schema': 'metadata/Document/v1', 'substitutions': [ + {'dest': {'path': '.'}, + 'src': {'schema': 'deckhand/Certificate/v1', + 'name': 'example-cert', 'path': '.'}}], + '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', + return_value=certificate_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 + # causing the actual certificate data to be returned. + with mock.patch.object(secrets_manager.SecretsManager, 'get', # noqa + return_value=certificate_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-cert', 'deckhand-global'], + 'cleartext-secrets': str(cleartext_secrets) + }, + params_csv=False) + + self.assertEqual(200, resp.status_code) + rendered_documents = list(yaml.safe_load_all(resp.text)) + self.assertEqual(2, len(rendered_documents)) + + if cleartext_secrets is True: + # Expect the cleartext data to be returned. + self.assertTrue(all(map(lambda x: x['data'] == certificate_data, + rendered_documents))) + else: + # Expected 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, + rendered_documents))) + + def test_list_rendered_documents_cleartext_secrets_true(self): + self._test_list_rendered_documents(cleartext_secrets=True) + + def test_list_rendered_documents_cleartext_secrets_false(self): + self._test_list_rendered_documents(cleartext_secrets=False) + + class TestRenderedDocumentsControllerNegative( test_base.BaseControllerTest):