Fix: Inject secret payload rather than reference into document
This PS fixes Deckhand currently wrongly substituting the secret reference Barbican returns into documents, rather than the secret payload itself. Closes #19 Change-Id: I1d4eed85ed336e83a777b4343f37b10c91038342
This commit is contained in:
parent
2bc0c07b01
commit
37dae6df9f
@ -41,6 +41,7 @@ class BarbicanClientWrapper(object):
|
||||
def _get_client(self, retry_on_conflict=True):
|
||||
# If we've already constructed a valid, authed client, just return
|
||||
# that.
|
||||
|
||||
if retry_on_conflict and self._cached_client is not None:
|
||||
return self._cached_client
|
||||
|
||||
|
@ -46,3 +46,14 @@ class BarbicanDriver(object):
|
||||
resp[utils.to_snake_case(key)] = resp.pop(key)
|
||||
resp['secret_ref'] = secret_ref
|
||||
return resp
|
||||
|
||||
def get_secret(self, secret_ref):
|
||||
"""Get a secret."""
|
||||
|
||||
try:
|
||||
return self.barbicanclient.call("secrets.get", secret_ref)
|
||||
except (barbicanclient.exceptions.HTTPAuthError,
|
||||
barbicanclient.exceptions.HTTPClientError,
|
||||
barbicanclient.exceptions.HTTPServerError) as e:
|
||||
LOG.exception(str(e))
|
||||
raise errors.BarbicanException(details=str(e))
|
||||
|
@ -67,7 +67,13 @@ class BucketsResource(api_base.BaseResource):
|
||||
'deckhand:create_encrypted_documents', req.context)
|
||||
break
|
||||
|
||||
self._prepare_secret_documents(documents)
|
||||
try:
|
||||
self._prepare_secret_documents(documents)
|
||||
except deckhand_errors.BarbicanException as e:
|
||||
LOG.error('An unknown exception occurred while trying to store '
|
||||
'a secret in Barbican.')
|
||||
raise falcon.HTTPInternalServerError(
|
||||
description=e.format_message())
|
||||
|
||||
created_documents = self._create_revision_documents(
|
||||
bucket_name, documents, validations)
|
||||
|
@ -36,7 +36,8 @@ class SecretsManager(object):
|
||||
|
||||
barbican_driver = driver.BarbicanDriver()
|
||||
|
||||
def create(self, secret_doc):
|
||||
@classmethod
|
||||
def create(cls, secret_doc):
|
||||
"""Securely store secrets contained in ``secret_doc``.
|
||||
|
||||
Ordinarily, Deckhand documents are stored directly in Deckhand's
|
||||
@ -62,7 +63,7 @@ class SecretsManager(object):
|
||||
``deckhand.db.sqlalchemy.models.DocumentSecret``.
|
||||
"""
|
||||
encryption_type = secret_doc['metadata']['storagePolicy']
|
||||
secret_type = self._get_secret_type(secret_doc['schema'])
|
||||
secret_type = cls._get_secret_type(secret_doc['schema'])
|
||||
|
||||
if encryption_type == ENCRYPTED:
|
||||
# Store secret_ref in database for `secret_doc`.
|
||||
@ -71,7 +72,7 @@ class SecretsManager(object):
|
||||
'secret_type': secret_type,
|
||||
'payload': secret_doc['data']
|
||||
}
|
||||
resp = self.barbican_driver.create_secret(**kwargs)
|
||||
resp = cls.barbican_driver.create_secret(**kwargs)
|
||||
|
||||
secret_ref = resp['secret_ref']
|
||||
created_secret = secret_ref
|
||||
@ -80,7 +81,14 @@ class SecretsManager(object):
|
||||
|
||||
return created_secret
|
||||
|
||||
def _get_secret_type(self, schema):
|
||||
@classmethod
|
||||
def get(cls, secret_ref):
|
||||
secret = cls.barbican_driver.get_secret(secret_ref=secret_ref)
|
||||
payload = secret.payload
|
||||
return payload
|
||||
|
||||
@classmethod
|
||||
def _get_secret_type(cls, schema):
|
||||
"""Get the Barbican secret type based on the following mapping:
|
||||
|
||||
``deckhand/Certificate/v1`` => certificate
|
||||
@ -150,6 +158,11 @@ class SecretsSubstitution(object):
|
||||
self._substitution_sources.setdefault(
|
||||
(document.schema, document.name), document)
|
||||
|
||||
def _is_barbican_ref(self, src_secret):
|
||||
# TODO(fmontei): Make this more robust.
|
||||
return (isinstance(src_secret, six.string_types) and
|
||||
'key-manager/v1/secrets' in src_secret)
|
||||
|
||||
def substitute_all(self, documents):
|
||||
"""Substitute all documents that have a `metadata.substitutions` field.
|
||||
|
||||
@ -218,6 +231,12 @@ class SecretsSubstitution(object):
|
||||
else:
|
||||
src_secret = src_doc.get('data')
|
||||
|
||||
# Check if src_secret is Barbican secret reference.
|
||||
if self._is_barbican_ref(src_secret):
|
||||
LOG.debug('Resolving Barbican reference for %s.',
|
||||
src_secret)
|
||||
src_secret = SecretsManager.get(src_secret)
|
||||
|
||||
dest_path = sub['dest']['path']
|
||||
dest_pattern = sub['dest'].get('pattern', None)
|
||||
|
||||
@ -240,6 +259,10 @@ class SecretsSubstitution(object):
|
||||
document.name)
|
||||
LOG.error(message)
|
||||
raise errors.UnknownSubstitutionError(details=message)
|
||||
except errors.BarbicanException as e:
|
||||
LOG.error('Failed to resolve a Barbican reference.')
|
||||
raise errors.UnknownSubstitutionError(
|
||||
details=e.format_message())
|
||||
except Exception as e:
|
||||
LOG.error('Unexpected exception occurred while attempting '
|
||||
'secret substitution. %s', six.text_type(e))
|
||||
|
@ -15,6 +15,8 @@
|
||||
import copy
|
||||
import yaml
|
||||
|
||||
import mock
|
||||
|
||||
from deckhand.db.sqlalchemy import api as db_api
|
||||
from deckhand.engine import secrets_manager
|
||||
from deckhand import factories
|
||||
@ -130,6 +132,43 @@ class TestSecretsSubstitution(test_base.TestDbBase):
|
||||
self._test_doc_substitution(
|
||||
document_mapping, [certificate], expected_data)
|
||||
|
||||
@mock.patch.object(secrets_manager, 'SecretsManager', autospec=True)
|
||||
def test_doc_substitution_single_encrypted(self, mock_secrets_manager):
|
||||
mock_secrets_manager.get.return_value = 'test-certificate'
|
||||
secret_ref = test_utils.rand_uuid_hex()
|
||||
|
||||
secret_ref = ("http://127.0.0.1/key-manager/v1/secrets/%s"
|
||||
% secret_ref)
|
||||
certificate = self.secrets_factory.gen_test(
|
||||
'Certificate', 'encrypted', data=secret_ref)
|
||||
certificate['metadata']['name'] = 'example-cert'
|
||||
|
||||
document_mapping = {
|
||||
"_GLOBAL_SUBSTITUTIONS_1_": [{
|
||||
"dest": {
|
||||
"path": ".chart.values.tls.certificate"
|
||||
},
|
||||
"src": {
|
||||
"schema": "deckhand/Certificate/v1",
|
||||
"name": "example-cert",
|
||||
"path": "."
|
||||
}
|
||||
|
||||
}]
|
||||
}
|
||||
expected_data = {
|
||||
'chart': {
|
||||
'values': {
|
||||
'tls': {
|
||||
'certificate': 'test-certificate'
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
self._test_doc_substitution(
|
||||
document_mapping, [certificate], expected_data)
|
||||
mock_secrets_manager.get.assert_called_once_with(secret_ref=secret_ref)
|
||||
|
||||
def test_create_destination_path_with_array(self):
|
||||
# Validate that the destination data will be populated with an array
|
||||
# where the data will be contained in array[0].
|
||||
|
0
entrypoint.sh
Normal file → Executable file
0
entrypoint.sh
Normal file → Executable file
Loading…
Reference in New Issue
Block a user