Merge "fix: Redact secondhand substitutions of sensitive data"

This commit is contained in:
Zuul 2018-10-29 17:13:25 +00:00 committed by Gerrit Code Review
commit 56e606bf4b
8 changed files with 137 additions and 74 deletions

View File

@ -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):

View File

@ -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]

View File

@ -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,

View File

@ -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)

View File

@ -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):

View File

@ -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

View File

@ -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)

View File

@ -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)