Merge "Fix encrypted doc rendering"
This commit is contained in:
commit
90f653bc0f
deckhand
common
control
engine
tests/unit/control
@ -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):
|
||||
|
@ -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,
|
||||
|
@ -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)]
|
||||
|
@ -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
|
||||
|
@ -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):
|
||||
|
@ -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
|
||||
|
Loading…
x
Reference in New Issue
Block a user