airship/deckhand
Code Issues Proposed changes

Fix secret_uuid used to query Barbican's Secrets API

Browse Source 
This is to fix secrets_manager.SecretsManager.get method which
is passing in the secret reference to Barbican directly for
GET /secrets/{uuid} [0] causing Barbican to raise a
ValueError exception when it attempts to validate that
{secret_uuid} is in fact a UUID.

The fix is to extract the secret_uuid from the secret_ref returned
by Barbican before querying the GET /secrets/{uuid} API.

[0] https://docs.openstack.org/barbican/latest/api/reference/secrets.html#get-v1-secrets-uuid

Change-Id: I4db317e3ba12b4268df5b84b79be8da1da5ac2ba
This commit is contained in:
Felipe Monteiro 2018-03-22 14:45:01 +00:00
parent d86d87d16c
commit 91de02be34
6 changed files with 145 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
deckhand/barbican/driver.py
View File

@ -54,6 +54,7 @@ class BarbicanDriver(object):
return self.barbicanclient.call("secrets.get", secret_ref)
except (barbicanclient.exceptions.HTTPAuthError,
barbicanclient.exceptions.HTTPClientError,
barbicanclient.exceptions.HTTPServerError) as e:
barbicanclient.exceptions.HTTPServerError,
ValueError) as e:
LOG.exception(str(e))
raise errors.BarbicanException(details=str(e))

9
deckhand/conf/config.py
View File

@ -26,15 +26,16 @@ Barbican options for allowing Deckhand to communicate with Barbican.
""")
barbican_opts = [
# TODO(fmontei): Drop these options and related group once Keystone
# endpoint lookup is used instead.
cfg.StrOpt(
'api_endpoint',
default='http://127.0.0.1/key-manager',
sample_default='http://barbican.example.org:9311/',
help='URL override for the Barbican API endpoint.'),
]
context_opts = [
default_opts = [
cfg.BoolOpt('allow_anonymous_access', default=False,
help="""
Allow limited access to unauthenticated users.
@ -58,13 +59,13 @@ Possible values:
def register_opts(conf):
conf.register_group(barbican_group)
conf.register_opts(barbican_opts, group=barbican_group)
conf.register_opts(context_opts)
conf.register_opts(default_opts)
ks_loading.register_auth_conf_options(conf, group=barbican_group.name)
ks_loading.register_session_conf_options(conf, group=barbican_group.name)
def list_opts():
opts = {None: context_opts,
opts = {None: default_opts,
barbican_group: barbican_opts +
ks_loading.get_session_conf_options() +
ks_loading.get_auth_common_conf_options() +

3
deckhand/control/revision_documents.py
View File

@ -123,7 +123,8 @@ class RenderedDocumentsResource(api_base.BaseResource):
except (errors.LayeringPolicyNotFound,
errors.SubstitutionSourceNotFound) as e:
raise falcon.HTTPConflict(description=e.format_message())
except errors.UnknownSubstitutionError as e:
except (errors.DeckhandException,
errors.UnknownSubstitutionError) as e:
raise falcon.HTTPInternalServerError(
description=e.format_message())

4
deckhand/db/sqlalchemy/api.py
View File

@ -1029,8 +1029,8 @@ def _get_validation_policies_for_revision(revision_id, session=None):
schema=types.VALIDATION_POLICY_SCHEMA)
if not validation_policies:
# Otherwise return early.
LOG.info('Failed to find a ValidationPolicy for revision ID %s.'
'Only the "%s" results will be included in the response.',
LOG.debug('Failed to find a ValidationPolicy for revision ID %s. '
'Only the "%s" results will be included in the response.',
revision_id, types.DECKHAND_SCHEMA_VALIDATION)
validation_policies = []

114
deckhand/engine/secrets_manager.py
View File

@ -17,6 +17,7 @@ import re
from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import uuidutils
import six
from deckhand.barbican import driver
@ -39,6 +40,9 @@ class SecretsManager(object):
barbican_driver = driver.BarbicanDriver()
_url_re = re.compile(r'http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|'
'(?:%[0-9a-fA-F][0-9a-fA-F]))+')
@classmethod
def create(cls, secret_doc):
"""Securely store secrets contained in ``secret_doc``.
@ -84,10 +88,52 @@ class SecretsManager(object):
return created_secret
@classmethod
def _is_barbican_ref(cls, secret_ref):
return (
isinstance(secret_ref, six.string_types) and
cls._url_re.match(secret_ref) and 'secrets' in secret_ref
)
@classmethod
def get(cls, secret_ref):
secret = cls.barbican_driver.get_secret(secret_ref=secret_ref)
"""Return a secret payload from Barbican if ``secret_ref`` is a
Barbican secret reference or else return ``secret_ref``.
Extracts {secret_uuid} from a secret reference and queries Barbican's
Secrets API with it.
:param str secret_ref:
* String formatted like:
"https://{barbican_host}/v1/secrets/{secret_uuid}"
which results in a Barbican query.
* Any other string which results in a pass-through.
:returns: Secret payload from Barbican or ``secret_ref``.
"""
# TODO(fmontei): Query Keystone service catalog for Barbican endpoint
# and cache it if Keystone is enabled. For now, it should be enough
# to check that ``secret_ref`` is a valid URL, contains 'secrets'
# substring, ends in a UUID and that the source document from which
# the reference is extracted is encrypted.
if cls._is_barbican_ref(secret_ref):
LOG.debug('Resolving Barbican secret using source document '
'reference...')
try:
secret_uuid = secret_ref.split('/')[-1]
except Exception:
secret_uuid = None
if not uuidutils.is_uuid_like(secret_uuid):
return secret_ref
else:
return secret_ref
# TODO(fmontei): Need to avoid this call if Keystone is disabled.
secret = cls.barbican_driver.get_secret(secret_ref=secret_uuid)
payload = secret.payload
LOG.debug('Successfully retrieved Barbican secret using reference.')
return payload
@classmethod
@ -118,6 +164,10 @@ class SecretsSubstitution(object):
__slots__ = ('_fail_on_missing_sub_src', '_substitution_sources')
_insecure_reg_exps = (
re.compile(r'^.* is not of type .+$'),
)
@staticmethod
def sanitize_potential_secrets(error, document):
"""Sanitize all secret data that may have been substituted into the
@ -133,15 +183,13 @@ class SecretsSubstitution(object):
if not document.substitutions and not document.is_encrypted:
return document
insecure_reg_exps = [
re.compile(r'^.* is not of type .+$')
]
to_sanitize = copy.deepcopy(document)
safe_message = 'Sanitized to avoid exposing secret.'
# Sanitize any secrets contained in `error.message` referentially.
if error.message and any(
r.match(error.message) for r in insecure_reg_exps):
r.match(error.message)
for r in SecretsSubstitution._insecure_reg_exps):
error.message = safe_message
# Sanitize any secrets extracted from the document itself.
@ -178,10 +226,6 @@ class SecretsSubstitution(object):
self._substitution_sources.setdefault(
(document.schema, document.name), document)
def _is_barbican_ref(self, src_secret):
return (isinstance(src_secret, six.string_types) and
src_secret.startswith(CONF.barbican.api_endpoint))
def substitute_all(self, documents):
"""Substitute all documents that have a `metadata.substitutions` field.
@ -250,11 +294,23 @@ 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)
# If the document has storagePolicy == encrypted then resolve
# the Barbican reference into the actual secret.
if src_doc.is_encrypted:
try:
src_secret = SecretsManager.get(src_secret)
except errors.BarbicanException as e:
LOG.error(
'Failed to resolve a Barbican reference for '
'substitution source document [%s] %s referenced '
'in document [%s] %s. Details: %s', src_schema,
src_name, document.schema, document.name,
e.format_message())
raise errors.UnknownSubstitutionError(
src_schema=src_schema, src_name=src_name,
document_name=document.name,
document_schema=document.schema,
details=e.format_message())
dest_path = sub['dest']['path']
dest_pattern = sub['dest'].get('pattern', None)
@ -263,6 +319,7 @@ class SecretsSubstitution(object):
'into dest_path=%s, dest_pattern=%s', src_schema,
src_name, src_path, dest_path, dest_pattern)
try:
exc_message = ''
substituted_data = utils.jsonpath_replace(
document['data'], src_secret, dest_path, dest_pattern)
if (isinstance(document['data'], dict)
@ -271,35 +328,26 @@ class SecretsSubstitution(object):
elif substituted_data:
document['data'] = substituted_data
else:
message = (
exc_message = (
'Failed to create JSON path "%s" in the '
'destination document [%s] %s. No data was '
'substituted.', dest_path, document.schema,
document.name)
LOG.error(message)
raise errors.UnknownSubstitutionError(details=message)
except errors.BarbicanException as e:
LOG.error('Failed to resolve a Barbican reference for '
'substitution source document [%s] %s referenced'
' in document [%s] %s. Details: %s', src_schema,
src_name, document.schema, document.name,
e.format_message())
raise errors.UnknownSubstitutionError(
src_schema=src_schema, src_name=src_name,
document_name=document.name,
document_schema=document.schema,
details=e.format_message())
LOG.error(exc_message)
except Exception as e:
LOG.error('Unexpected exception occurred while attempting '
'substitution using source document [%s] %s '
'referenced in [%s] %s. Details: %s', src_schema,
src_name, document.schema, document.name,
six.text_type(e))
raise errors.UnknownSubstitutionError(
src_schema=src_schema, src_name=src_name,
document_name=document.name,
document_schema=document.schema,
details=six.text_type(e))
exc_message = six.text_type(e)
finally:
if exc_message:
raise errors.UnknownSubstitutionError(
src_schema=src_schema, src_name=src_name,
document_name=document.name,
document_schema=document.schema,
details=exc_message)
yield document

60
deckhand/tests/unit/engine/test_secrets_manager.py
View File

@ -16,6 +16,7 @@ import copy
import yaml
import mock
from oslo_utils import uuidutils
from deckhand.db.sqlalchemy import api as db_api
from deckhand.engine import secrets_manager
@ -30,51 +31,96 @@ class TestSecretsManager(test_base.TestDbBase):
super(TestSecretsManager, self).setUp()
self.mock_barbican_driver = self.patchobject(
secrets_manager.SecretsManager, 'barbican_driver')
self.secret_ref = 'https://path/to/fake_secret'
self.secret_ref = "https://barbican_host/v1/secrets/{secret_uuid}"\
.format(**{'secret_uuid': uuidutils.generate_uuid()})
self.mock_barbican_driver.create_secret.return_value = (
{'secret_ref': self.secret_ref})
self.secrets_manager = secrets_manager.SecretsManager()
self.factory = factories.DocumentSecretFactory()
def _test_create_secret(self, encryption_type, secret_type):
secret_data = test_utils.rand_password()
secret_doc = self.factory.gen_test(
secret_type, encryption_type, secret_data)
payload = secret_doc['data']
self.mock_barbican_driver.get_secret.return_value = (
mock.Mock(payload=payload))
created_secret = self.secrets_manager.create(secret_doc)
created_secret = secrets_manager.SecretsManager.create(secret_doc)
if encryption_type == 'cleartext':
self.assertEqual(secret_data, created_secret)
elif encryption_type == 'encrypted':
expected_kwargs = {
'name': secret_doc['metadata']['name'],
'secret_type': ('private' if secret_type == 'CertificateKey'
else secret_type.lower()),
'payload': secret_doc['data']
'secret_type': secrets_manager.SecretsManager._get_secret_type(
'deckhand/' + secret_type),
'payload': payload
}
self.mock_barbican_driver.create_secret.assert_called_once_with(
**expected_kwargs)
self.assertEqual(self.secret_ref, created_secret)
return created_secret, payload
def test_create_cleartext_certificate(self):
self._test_create_secret('cleartext', 'Certificate')
def test_create_cleartext_certificate_authority(self):
self._test_create_secret('cleartext', 'CertificateAuthority')
def test_create_cleartext_certificate_authority_key(self):
self._test_create_secret('cleartext', 'CertificateAuthorityKey')
def test_create_cleartext_certificate_key(self):
self._test_create_secret('cleartext', 'CertificateKey')
def test_create_cleartext_passphrase(self):
self._test_create_secret('cleartext', 'Passphrase')
def test_create_cleartext_private_key(self):
self._test_create_secret('cleartext', 'PrivateKey')
def test_create_encrypted_certificate(self):
self._test_create_secret('encrypted', 'Certificate')
def test_create_encrypted_certificate_authority(self):
self._test_create_secret('encrypted', 'CertificateAuthority')
def test_create_encrypted_certificate_authority_key(self):
self._test_create_secret('encrypted', 'CertificateAuthorityKey')
def test_create_encrypted_certificate_key(self):
self._test_create_secret('encrypted', 'CertificateKey')
def test_create_encrypted_passphrase(self):
self._test_create_secret('encrypted', 'Passphrase')
def test_create_encrypted_private_key(self):
self._test_create_secret('encrypted', 'PrivateKey')
def test_retrieve_barbican_secret(self):
secret_ref, expected_secret = self._test_create_secret(
'encrypted', 'Certificate')
secret_uuid = secret_ref.split('/')[-1]
secret_payload = secrets_manager.SecretsManager.get(secret_ref)
self.assertEqual(expected_secret, secret_payload)
self.mock_barbican_driver.get_secret.assert_called_once_with(
secret_ref=secret_uuid)
class TestSecretsManagerNegative(test_base.TestDbBase):
def test_retrieve_barbican_secret_bad_reference_raises_exc(self):
"""Verify that passing in an invalid reference to
``SecretsManager.get`` returns gracefully with the original argument.
"""
bad_refs = ('', 'a/b', 'a/12345', 'a/%s/b' % uuidutils.generate_uuid(),
12345, False, None, {}, [])
for bad_ref in bad_refs:
self.assertEqual(
bad_ref, secrets_manager.SecretsManager.get(bad_ref))
class TestSecretsSubstitution(test_base.TestDbBase):