Merge "fix: Redact secondhand substitutions of sensitive data"
This commit is contained in:
commit
56e606bf4b
|
@ -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):
|
||||
|
|
|
@ -396,23 +396,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]
|
||||
|
||||
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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):
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
Loading…
Reference in New Issue