airship
/
deckhand
Code Issues Proposed changes

Fix encrypted doc rendering 

This patchset fixes a bug where Deckhand was failing to perform
substitution and layering on document sets where all the documents had a
storagePolicy of encrypted. Deckhand would attempt to substitute from an
encrypted source document, but when that document marked as encrypted,
it fails because the source doc had been redacted. The behavior now goes
as follows:

- Resolve Barbican references before layering and substitution have been
  performed so that the prior two operations don't attempt to operate on a
  Barbican reference
- After substitution, redact the destination document if it is marked as
  encrypted
- Now, after substition, we can redact the rest of the documents and
  substitutions

Change-Id: I725775d554c9eed2692fc6203c416a7119646680
This commit is contained in:
Doug Aaser 2019-09-24 14:42:11 +00:00 committed by Matt Carter
parent 460eb7fb6c
commit 2786769de5
6 changed files with 210 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
deckhand/common/utils.py
View File

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

3
deckhand/control/common.py
View File

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

7
deckhand/control/revision_documents.py
View File

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

53
deckhand/engine/layering.py
View File

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

19
deckhand/engine/secrets_manager.py
View File

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

150
deckhand/tests/unit/control/test_rendered_documents_controller.py
View File

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