From 84ab5c5096826a6382d224271c75adee8d10e742 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Thu, 26 Apr 2018 23:33:28 -0400 Subject: [PATCH] [test] Add integration test scenario for encrypting generic type This PS adds an integration test scenario for validating that encrypting a generic document type and using it as a substitution source during document rendering works. Deckhand will now submit all generic documents to be encrypted to Barbican with a 'secret_type' of 'passphrase'. No encoding is provided Deckhand-side (i.e. base64) because encoding is deprecated in Barbican since it lead to strange behavior; Barbican will figure out what to encode the payload as automatically. For more information, see [0] and [1]. In addition, this PS handles 2 edge cases around secret payloads that are rejected by Barbican if not handled correctly by Deckhand: empty payloads and non-string type payloads [2]. For the first case Deckhand forcibly changes the document to cleartext because there is no point in encrypting a document with an empty payload. For the second case Deckhand sets overrides any previously set secret_type to 'opaque' and encodes the payload to base64 -- when it goes to render the secret it decodes the payload also using base64. Integration tests have been added to handle both edge cases described above. [0] https://bugs.launchpad.net/python-barbicanclient/+bug/1419166 [1] https://github.com/openstack/python-barbicanclient/blob/49505b9aec05ec42e0a809ec51935c2324d5f3a6/barbicanclient/v1/secrets.py#L252 [2] https://github.com/openstack/python-barbicanclient/blob/49505b9aec05ec42e0a809ec51935c2324d5f3a6/barbicanclient/v1/secrets.py#L297 Change-Id: I1964aa84ad07b6f310b39974f078b84a1dc84983 --- deckhand/barbican/driver.py | 196 ++++++++++++++++-- deckhand/common/document.py | 11 +- deckhand/control/common.py | 19 +- deckhand/db/sqlalchemy/api.py | 8 +- deckhand/engine/secrets_manager.py | 132 ++---------- .../document-render-secret-edge-cases.yaml | 134 ++++++++++++ .../gabbits/document-render-secret.yaml | 5 + .../document-substitution-secret-generic.yaml | 97 +++++++++ .../tests/unit/engine/test_secrets_manager.py | 104 ++++++++-- deckhand/types.py | 45 ++-- doc/source/api_ref.rst | 7 +- 11 files changed, 567 insertions(+), 191 deletions(-) create mode 100644 deckhand/tests/integration/gabbits/document-render-secret-edge-cases.yaml create mode 100644 deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml diff --git a/deckhand/barbican/driver.py b/deckhand/barbican/driver.py index 73085d9f..f19b389a 100644 --- a/deckhand/barbican/driver.py +++ b/deckhand/barbican/driver.py @@ -12,49 +12,213 @@ # See the License for the specific language governing permissions and # limitations under the License. +import ast +import re + import barbicanclient from oslo_log import log as logging +from oslo_serialization import base64 +from oslo_utils import uuidutils +import six from deckhand.barbican import client_wrapper -from deckhand.common import utils from deckhand import errors +from deckhand import types LOG = logging.getLogger(__name__) class BarbicanDriver(object): + _url_re = re.compile(r'http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|' + '(?:%[0-9a-fA-F][0-9a-fA-F]))+') + def __init__(self): self.barbicanclient = client_wrapper.BarbicanClientWrapper() - def create_secret(self, **kwargs): - """Create a secret.""" - secret = self.barbicanclient.call("secrets.create", **kwargs) + @classmethod + def is_barbican_ref(cls, secret_ref): + # TODO(felipemonteiro): 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. + try: + secret_uuid = secret_ref.split('/')[-1] + except Exception: + secret_uuid = None + return ( + isinstance(secret_ref, six.string_types) and + cls._url_re.match(secret_ref) and + 'secrets' in secret_ref and + uuidutils.is_uuid_like(secret_uuid) + ) + + @staticmethod + def _get_secret_type(schema): + """Get the Barbican secret type based on the following mapping: + + ``deckhand/Certificate/v1`` => certificate + ``deckhand/CertificateKey/v1`` => private + ``deckhand/CertificateAuthority/v1`` => certificate + ``deckhand/CertificateAuthorityKey/v1`` => private + ``deckhand/Passphrase/v1`` => passphrase + ``deckhand/PrivateKey/v1`` => private + ``deckhand/PublicKey/v1`` => public + Other => passphrase + + :param schema: The document's schema. + :returns: The value corresponding to the mapping above. + """ + parts = schema.split('/') + if len(parts) == 3: + namespace, kind, _ = parts + elif len(parts) == 2: + namespace, kind = parts + else: + raise ValueError( + 'Schema %s must consist of namespace/kind/version.' % schema) + + is_generic = ( + '/'.join([namespace, kind]) not in types.DOCUMENT_SECRET_TYPES + ) + + # If the document kind is not a built-in secret type, then default to + # 'passphrase'. + if is_generic: + LOG.debug('Defaulting to secret_type="passphrase" for generic ' + 'document schema %s.', schema) + return 'passphrase' + + kind = kind.lower() + + if kind in [ + 'certificateauthoritykey', 'certificatekey', 'privatekey' + ]: + return 'private' + elif kind == 'certificateauthority': + return 'certificate' + elif kind == 'publickey': + return 'public' + # NOTE(felipemonteiro): This branch below handles certificate and + # passphrase, both of which are supported secret types in Barbican. + return kind + + def _base64_encode_payload(self, secret_doc): + """Ensures secret document payload is compatible with Barbican.""" + + payload = secret_doc.data + secret_type = self._get_secret_type(secret_doc.schema) + + # NOTE(felipemonteiro): The logic for the 2 conditions below is + # enforced from Barbican's Python client. Some pre-processing and + # transformation is needed to make Barbican work with non-compatible + # formats. + if not payload and payload is not False: + # There is no point in even bothering to encrypt an empty + # body, which just leads to needless overhead, so return + # early. + LOG.info('Barbican does not accept empty payloads so ' + 'Deckhand will not encrypt document [%s, %s] %s.', + secret_doc.schema, secret_doc.layer, secret_doc.name) + secret_doc.storage_policy = types.CLEARTEXT + elif not isinstance( + payload, (six.text_type, six.binary_type)): + LOG.debug('Forcibly setting secret_type=opaque and ' + 'base64-encoding non-string payload for ' + 'document [%s, %s] %s.', secret_doc.schema, + secret_doc.layer, secret_doc.name) + # NOTE(felipemonteiro): base64-encoding the non-string payload is + # done for serialization purposes, not for security purposes. + # 'opaque' is used to avoid Barbican doing any further + # serialization server-side. + secret_type = 'opaque' + try: + payload = base64.encode_as_text(six.text_type(payload)) + except Exception: + message = ('Failed to base64-encode payload of type %s ' + 'for Barbican storage.', type(payload)) + LOG.error(message) + raise errors.UnknownSubstitutionError( + src_schema=secret_doc.schema, + src_layer=secret_doc.layer, src_name=secret_doc.name, + schema='N/A', layer='N/A', name='N/A', details=message) + return secret_type, payload + + def create_secret(self, secret_doc): + """Create a secret. + + :param secret_doc: Document with ``storagePolicy`` of "encrypted". + :type secret_doc: document.DocumentDict + :returns: Secret reference returned by Barbican + :rtype: str + """ + secret_type, payload = self._base64_encode_payload(secret_doc) + + if secret_doc.storage_policy == types.CLEARTEXT: + return payload + + # Store secret_ref in database for `secret_doc`. + kwargs = { + 'name': secret_doc['metadata']['name'], + 'secret_type': secret_type, + 'payload': payload + } + LOG.info('Storing encrypted document data in Barbican.') try: + secret = self.barbicanclient.call("secrets.create", **kwargs) secret_ref = secret.store() except (barbicanclient.exceptions.HTTPAuthError, barbicanclient.exceptions.HTTPClientError, barbicanclient.exceptions.HTTPServerError) as e: - LOG.exception(str(e)) + LOG.error('Caught %s error from Barbican, likely due to a ' + 'configuration or deployment issue.', + e.__class__.__name__) + raise errors.BarbicanException(details=str(e)) + except barbicanclient.exceptions.PayloadException as e: + LOG.error('Caught %s error from Barbican, because the secret ' + 'payload type is unsupported.', e.__class__.__name__) raise errors.BarbicanException(details=str(e)) - # NOTE(fmontei): The dictionary representation of the Secret object by - # default has keys that are not snake case -- so make them snake case. - resp = secret.to_dict() - for key in resp: - 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.""" + return secret_ref + def _base64_decode_payload(self, src_doc, dest_doc, payload): try: - return self.barbicanclient.call("secrets.get", secret_ref) + # If the secret_type is 'opaque' then this implies the + # payload was encoded to base64 previously. Reverse the + # operation. + payload = ast.literal_eval(base64.decode_as_text(payload)) + except Exception: + message = ('Failed to unencode the original payload that ' + 'presumably was encoded to base64 with ' + 'secret_type=opaque for document [%s, %s] %s.' % + src_doc.meta) + LOG.error(message) + raise errors.UnknownSubstitutionError( + src_schema=src_doc.schema, src_layer=src_doc.layer, + src_name=src_doc.name, schema=dest_doc.schema, + layer=dest_doc.layer, name=dest_doc.name, + details=message) + return payload + + def get_secret(self, src_doc, dest_doc, secret_ref): + """Get a secret.""" + try: + secret = self.barbicanclient.call("secrets.get", secret_ref) except (barbicanclient.exceptions.HTTPAuthError, barbicanclient.exceptions.HTTPClientError, barbicanclient.exceptions.HTTPServerError, ValueError) as e: LOG.exception(str(e)) raise errors.BarbicanException(details=str(e)) + + payload = secret.payload + if secret.secret_type == 'opaque': + LOG.debug('Forcibly base64-decoding original non-string payload ' + 'for document [%s, %s] %s.', *src_doc.meta) + secret = self._base64_decode_payload(src_doc, dest_doc, payload) + else: + secret = payload + + return secret diff --git a/deckhand/common/document.py b/deckhand/common/document.py index 25662e5d..2c8e2fb6 100644 --- a/deckhand/common/document.py +++ b/deckhand/common/document.py @@ -30,6 +30,11 @@ class DocumentDict(dict): Useful for accessing nested dictionary keys without having to worry about exceptions getting thrown. + .. note:: + + As a rule of thumb, setters for any metadata properties should be + avoided. Only implement or use for well-understood edge cases. + """ def __init__(self, *args, **kwargs): @@ -112,7 +117,7 @@ class DocumentDict(dict): @substitutions.setter def substitutions(self, value): - return utils.jsonpath_replace(self, value, 'metadata.substitutions') + return utils.jsonpath_replace(self, value, '.metadata.substitutions') @property def actions(self): @@ -123,6 +128,10 @@ class DocumentDict(dict): def storage_policy(self): return utils.jsonpath_parse(self, 'metadata.storagePolicy') or '' + @storage_policy.setter + def storage_policy(self, value): + return utils.jsonpath_replace(self, value, '.metadata.storagePolicy') + @property def is_encrypted(self): return self.storage_policy == 'encrypted' diff --git a/deckhand/control/common.py b/deckhand/control/common.py index 743d8b62..09375e23 100644 --- a/deckhand/control/common.py +++ b/deckhand/control/common.py @@ -56,21 +56,26 @@ def sanitize_params(allowed_params): # This maps which type should be enforced per query parameter. # Everything not included in type dict below is assumed to be a # string or a list of strings. - type_dict = {'limit': int} + type_dict = { + 'limit': { + 'func': lambda x: abs(int(x)), + 'type': int + } + } def _enforce_query_filter_type(key, val): - if key in type_dict: - cast_type = type_dict[key] + cast_func = type_dict.get(key) + if cast_func: try: - cast_val = cast_type(val) + cast_val = cast_func['func'](val) except Exception: raise falcon.HTTPInvalidParam( 'Query parameter %s must be of type %s.' % ( - key, cast_type), + key, cast_func['type']), key) - return cast_val else: - return val + cast_val = val + return cast_val def _convert_to_dict(sanitized_params, filter_key, filter_val): # Key-value pairs like metadata.label=foo=bar need to be diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index d0fe3d9e..aba17e0b 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -186,7 +186,8 @@ def documents_create(bucket_name, documents, validations=None, validation) if documents_to_delete: - LOG.debug('Deleting documents: %s.', documents_to_delete) + LOG.debug('Deleting documents: %s.', + [d.meta for d in documents_to_delete]) deleted_documents = [] for d in documents_to_delete: @@ -215,8 +216,9 @@ def documents_create(bucket_name, documents, validations=None, resp.append(doc.to_dict()) if documents_to_create: - LOG.debug('Creating documents: %s.', - [(d['schema'], d['name']) for d in documents_to_create]) + LOG.debug( + 'Creating documents: %s.', [(d['schema'], d['layer'], d['name']) + for d in documents_to_create]) for doc in documents_to_create: with session.begin(): doc['bucket_id'] = bucket['id'] diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index 1985586b..71b6ed3a 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -17,10 +17,9 @@ 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 +from deckhand.barbican.driver import BarbicanDriver from deckhand.common import document as document_wrapper from deckhand.common import utils from deckhand import errors @@ -36,10 +35,7 @@ class SecretsManager(object): Currently only supports Barbican. """ - barbican_driver = driver.BarbicanDriver() - - _url_re = re.compile(r'http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|' - '(?:%[0-9a-fA-F][0-9a-fA-F]))+') + barbican_driver = BarbicanDriver() @staticmethod def requires_encryption(document): @@ -48,76 +44,40 @@ class SecretsManager(object): document = clazz(document) return document.is_encrypted - @classmethod - def is_barbican_ref(cls, 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. - try: - secret_uuid = secret_ref.split('/')[-1] - except Exception: - secret_uuid = None - return ( - isinstance(secret_ref, six.string_types) and - cls._url_re.match(secret_ref) and - 'secrets' in secret_ref and - uuidutils.is_uuid_like(secret_uuid) - ) - @classmethod def create(cls, secret_doc): """Securely store secrets contained in ``secret_doc``. - Ordinarily, Deckhand documents are stored directly in Deckhand's - database. However, secret data (contained in the data section for the - documents with the schemas enumerated below) must be stored using a - secure storage service like Barbican. - Documents with ``metadata.storagePolicy`` == "clearText" have their secrets stored directly in Deckhand. Documents with ``metadata.storagePolicy`` == "encrypted" are stored in Barbican directly. Deckhand in turn stores the reference returned - by Barbican in Deckhand. + by Barbican in its own DB. - :param secret_doc: A Deckhand document with one of the following - schemas: + :param secret_doc: A Deckhand document with a schema that belongs to + ``types.DOCUMENT_SECRET_TYPES``. - * ``deckhand/Certificate/v1`` - * ``deckhand/CertificateKey/v1`` - * ``deckhand/Passphrase/v1`` - - :returns: Dictionary representation of - ``deckhand.db.sqlalchemy.models.DocumentSecret``. + :returns: Unecrypted data section from ``secret_doc`` if the document's + ``storagePolicy`` is "cleartext" or a Barbican secret reference + if the ``storagePolicy`` is "encrypted'. """ # 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) - encryption_type = secret_doc['metadata']['storagePolicy'] - secret_type = cls._get_secret_type(secret_doc['schema']) - created_secret = secret_doc['data'] + if secret_doc.storage_policy == types.ENCRYPTED: + payload = cls.barbican_driver.create_secret(secret_doc) + else: + payload = secret_doc.data - if encryption_type == types.ENCRYPTED: - # Store secret_ref in database for `secret_doc`. - kwargs = { - 'name': secret_doc['metadata']['name'], - 'secret_type': secret_type, - 'payload': secret_doc['data'] - } - LOG.info('Storing encrypted document data in Barbican.') - resp = cls.barbican_driver.create_secret(**kwargs) - - secret_ref = resp['secret_ref'] - created_secret = secret_ref - - return created_secret + return payload @classmethod - def get(cls, secret_ref): - """Return a secret payload from Barbican. + def get(cls, secret_ref, src_doc, dest_doc): + """Retrieve a secret payload from Barbican. Extracts {secret_uuid} from a secret reference and queries Barbican's Secrets API with it. @@ -125,43 +85,13 @@ class SecretsManager(object): :param str secret_ref: A string formatted like: "https://{barbican_host}/v1/secrets/{secret_uuid}" :returns: Secret payload from Barbican. - """ LOG.debug('Resolving Barbican secret using source document ' 'reference...') - # TODO(fmontei): Need to avoid this call if Keystone is disabled. - secret = cls.barbican_driver.get_secret(secret_ref=secret_ref) - payload = secret.payload + secret = cls.barbican_driver.get_secret(src_doc, dest_doc, + secret_ref=secret_ref) LOG.debug('Successfully retrieved Barbican secret using reference.') - return payload - - @classmethod - def _get_secret_type(cls, schema): - """Get the Barbican secret type based on the following mapping: - - ``deckhand/Certificate/v1`` => certificate - ``deckhand/CertificateKey/v1`` => private - ``deckhand/CertificateAuthority/v1`` => certificate - ``deckhand/CertificateAuthorityKey/v1`` => private - ``deckhand/Passphrase/v1`` => passphrase - ``deckhand/PrivateKey/v1`` => private - ``deckhand/PublicKey/v1`` => public - - :param schema: The document's schema. - :returns: The value corresponding to the mapping above. - """ - _schema = schema.split('/')[1].lower().strip() - if _schema in [ - 'certificateauthoritykey', 'certificatekey', 'privatekey' - ]: - return 'private' - elif _schema == 'certificateauthority': - return 'certificate' - elif _schema == 'publickey': - return 'public' - # NOTE(fmontei): This branch below handles certificate and passphrase, - # both of which are supported secret types in Barbican. - return _schema + return secret class SecretsSubstitution(object): @@ -209,7 +139,7 @@ class SecretsSubstitution(object): @staticmethod def get_encrypted_data(src_secret, src_doc, dest_doc): try: - src_secret = SecretsManager.get(src_secret) + src_secret = SecretsManager.get(src_secret, src_doc, dest_doc) except errors.BarbicanException as e: LOG.error( 'Failed to resolve a Barbican reference for substitution ' @@ -270,24 +200,6 @@ class SecretsSubstitution(object): else: LOG.warning(exc_message) - def _get_encrypted_secret(self, src_secret, src_doc, dest_doc): - 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] %s referenced in document [%s, %s] ' - '%s. Details: %s', src_doc.schema, src_doc.layer, src_doc.name, - dest_doc.schema, dest_doc.layer, dest_doc.name, - e.format_message()) - raise errors.UnknownSubstitutionError( - src_schema=src_doc.schema, src_layer=src_doc.layer, - src_name=src_doc.name, schema=dest_doc.schema, - layer=dest_doc.layer, name=dest_doc.name, - details=e.format_message()) - else: - return src_secret - def _check_src_secret_is_not_none(self, src_secret, src_path, src_doc, dest_doc): if src_secret is None: @@ -378,7 +290,7 @@ class SecretsSubstitution(object): # If the document has storagePolicy == encrypted then resolve # the Barbican reference into the actual secret. - if src_doc.is_encrypted and SecretsManager.is_barbican_ref( + if src_doc.is_encrypted and BarbicanDriver.is_barbican_ref( src_secret): src_secret = self.get_encrypted_data(src_secret, src_doc, document) diff --git a/deckhand/tests/integration/gabbits/document-render-secret-edge-cases.yaml b/deckhand/tests/integration/gabbits/document-render-secret-edge-cases.yaml new file mode 100644 index 00000000..35a80b86 --- /dev/null +++ b/deckhand/tests/integration/gabbits/document-render-secret-edge-cases.yaml @@ -0,0 +1,134 @@ +# Tests success paths for edge cases around rendering with secrets. +# +# 1. Verifies that attempting to encrypt a secret passphrase with an empty +# string skips encryption and stores the document as cleartext instead +# and that rendering the document works (which should avoid Barbican +# API call). +# 2. Verifies that attempting to encrypt any document with an incompatible +# payload type (non-str, non-binary) results in the payload being properly +# encoded and decoded by Deckhand before and after storing and retrieving +# the encrypted data. + +defaults: + request_headers: + content-type: application/x-yaml + X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN'] + response_headers: + content-type: application/x-yaml + verbose: true + +tests: + ### Scenario 1 ### + - name: attempt_create_encrypted_passphrase_empty_payload + desc: | + Attempting to create an encrypted passphrase with empty payload should + avoid encryption and return the empty payload instead. + PUT: /api/v1.0/buckets/secret/documents + status: 200 + data: |- + --- + schema: deckhand/LayeringPolicy/v1 + metadata: + schema: metadata/Control/v1 + name: layering-policy + data: + layerOrder: + - site + --- + schema: deckhand/Passphrase/v1 + metadata: + schema: metadata/Document/v1 + name: my-passphrase + storagePolicy: encrypted + layeringDefinition: + layer: site + data: '' + ... + response_multidoc_jsonpaths: + $.`len`: 2 + $.[1].metadata.name: my-passphrase + $.[1].data: '' + + - name: verify_revision_documents_returns_same_empty_payload + desc: Verify that the created document wasn't encrypted. + GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents + status: 200 + query_parameters: + metadata.name: my-passphrase + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].data: '' + + - name: verify_rendered_documents_returns_same_empty_payload + desc: | + Verify that rendering the document returns the same empty payload + which requires that the data be read directly from Deckhand's DB + rather than Barbican. + GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/rendered-documents + status: 200 + query_parameters: + metadata.name: my-passphrase + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].data: '' + + ### Scenario 2 ### + - name: create_encrypted_passphrase_with_incompatible_payload + desc: | + Attempting to encrypt a non-str/non-bytes payload should result in + it first being base64-encoded then passed to Barbican. The response + should be a Barbican reference, which indicates the scenario passed + successfully. + PUT: /api/v1.0/buckets/secret/documents + status: 200 + data: |- + --- + schema: deckhand/LayeringPolicy/v1 + metadata: + schema: metadata/Control/v1 + name: layering-policy + data: + layerOrder: + - site + --- + schema: armada/Generic/v1 + metadata: + schema: metadata/Document/v1 + name: armada-doc + storagePolicy: encrypted + layeringDefinition: + layer: site + data: + # This will be an object in memory requiring base64 encoding. + foo: bar + ... + response_multidoc_jsonpaths: + $.`len`: 2 + $.[1].metadata.name: armada-doc + # NOTE(fmontei): jsonpath-rw-ext uses a 1 character separator (rather than allowing a string) + # leading to this nastiness: + $.[1].data.`split(:, 0, 1)` + "://" + $.[1].data.`split(/, 2, 3)`: $ENVIRON['TEST_BARBICAN_URL'] + + - name: verify_revision_documents_returns_barbican_ref + desc: Verify that the encrypted document returns a Barbican ref. + GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents + status: 200 + query_parameters: + metadata.name: armada-doc + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].data.`split(:, 0, 1)` + "://" + $.[0].data.`split(/, 2, 3)`: $ENVIRON['TEST_BARBICAN_URL'] + + - name: verify_rendered_documents_returns_unencrypted_payload + desc: | + Verify that rendering the document returns the original payload which + means that Deckhand successfully encoded and decoded the non-compatible + payload using base64 encoding. + GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/rendered-documents + status: 200 + query_parameters: + metadata.name: armada-doc + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].data: + foo: bar diff --git a/deckhand/tests/integration/gabbits/document-render-secret.yaml b/deckhand/tests/integration/gabbits/document-render-secret.yaml index 4127532c..2aaa3bb4 100644 --- a/deckhand/tests/integration/gabbits/document-render-secret.yaml +++ b/deckhand/tests/integration/gabbits/document-render-secret.yaml @@ -41,6 +41,11 @@ tests: layer: site data: not-a-real-password ... + response_multidoc_jsonpaths: + $.`len`: 2 + # NOTE(fmontei): jsonpath-rw-ext uses a 1 character separator (rather than allowing a string) + # leading to this nastiness: + $.[1].data.`split(:, 0, 1)` + "://" + $.[1].data.`split(/, 2, 3)`: $ENVIRON['TEST_BARBICAN_URL'] - name: verify_rendered_documents_returns_secret_payload desc: Verify that the rendering the document returns the secret payload. diff --git a/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml b/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml new file mode 100644 index 00000000..8c53bdb2 --- /dev/null +++ b/deckhand/tests/integration/gabbits/document-substitution-secret-generic.yaml @@ -0,0 +1,97 @@ +# Tests success paths for secret substitution using a generic document type. +# This entails setting storagePolicy=encrypted for a non-built-in secret +# document. +# +# 1. Tests that creating an encrypted generic document results in a +# Barbican reference being returned. +# 2. Tests that the encrypted payload is included in the destination +# and source documents after document rendering. + +defaults: + request_headers: + content-type: application/x-yaml + X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN'] + response_headers: + content-type: application/x-yaml + verbose: true + +tests: + - name: purge + desc: Begin testing from known state. + DELETE: /api/v1.0/revisions + status: 204 + response_headers: null + + - name: encrypt_generic_document_for_secret_substitution + desc: | + Create documents using a generic document type (armada/Generic/v1) as the + substitution source with storagePolicy=encrypted. + PUT: /api/v1.0/buckets/secret/documents + status: 200 + data: |- + --- + schema: deckhand/LayeringPolicy/v1 + metadata: + schema: metadata/Control/v1 + name: layering-policy + data: + layerOrder: + - site + --- + # Generic document as substitution source. + schema: armada/Generic/v1 + metadata: + name: example-armada-cert + schema: metadata/Document/v1 + layeringDefinition: + layer: site + storagePolicy: encrypted + data: ARMADA CERTIFICATE DATA + --- + schema: armada/Chart/v1 + metadata: + schema: metadata/Document/v1 + name: armada-chart-01 + layeringDefinition: + layer: site + substitutions: + - dest: + path: .chart.values.tls.certificate + src: + schema: armada/Generic/v1 + name: example-armada-cert + path: . + data: {} + ... + + - name: verify_multiple_revision_documents_returns_secret_ref + desc: Verify that secret ref was created for example-armada-cert among multiple created documents. + GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents + status: 200 + query_parameters: + metadata.name: example-armada-cert + response_multidoc_jsonpaths: + $.`len`: 1 + # NOTE(fmontei): jsonpath-rw-ext uses a 1 character separator (rather than allowing a string) + # leading to this nastiness: + $.[0].data.`split(:, 0, 1)` + "://" + $.[0].data.`split(/, 2, 3)`: $ENVIRON['TEST_BARBICAN_URL'] + + - name: verify_secret_payload_in_destination_document + desc: Verify secret payload is injected in destination document as well as example-armada-cert. + GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/rendered-documents + status: 200 + query_parameters: + metadata.name: + - armada-chart-01 + - example-armada-cert + sort: metadata.name + response_multidoc_jsonpaths: + $.`len`: 2 + $.[0].metadata.name: armada-chart-01 + $.[0].data: + chart: + values: + tls: + certificate: ARMADA CERTIFICATE DATA + $.[1].metadata.name: example-armada-cert + $.[1].data: ARMADA CERTIFICATE DATA diff --git a/deckhand/tests/unit/engine/test_secrets_manager.py b/deckhand/tests/unit/engine/test_secrets_manager.py index 3df81d39..6eb297e0 100644 --- a/deckhand/tests/unit/engine/test_secrets_manager.py +++ b/deckhand/tests/unit/engine/test_secrets_manager.py @@ -16,9 +16,13 @@ import copy import yaml import mock +from oslo_serialization import base64 from oslo_utils import uuidutils +import six import testtools +from deckhand.barbican import driver +from deckhand.common import document as document_wrapper from deckhand.engine import secrets_manager from deckhand import errors from deckhand import factories @@ -30,38 +34,45 @@ class TestSecretsManager(test_base.TestDbBase): def setUp(self): super(TestSecretsManager, self).setUp() - self.mock_barbican_driver = self.patchobject( - secrets_manager.SecretsManager, 'barbican_driver') + self.mock_barbicanclient = self.patchobject( + secrets_manager.SecretsManager.barbican_driver, 'barbicanclient') 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.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)) + def _mock_barbican_client_call(self, payload): + def fake_call(action, *args, **kwargs): + if action == "secrets.create": + return mock.Mock(**{'store.return_value': self.secret_ref}) + elif action == "secrets.get": + return mock.Mock(payload=payload) - created_secret = secrets_manager.SecretsManager.create(secret_doc) + fake_secret_obj = self.mock_barbicanclient.call + fake_secret_obj.side_effect = fake_call + + def _test_create_secret(self, encryption_type, secret_type): + secret_payload = test_utils.rand_password() + secret_doc = self.factory.gen_test( + secret_type, encryption_type, secret_payload) + payload = secret_doc['data'] + + self._mock_barbican_client_call(payload) + secret_ref = secrets_manager.SecretsManager.create(secret_doc) if encryption_type == 'cleartext': - self.assertEqual(secret_data, created_secret) + self.assertEqual(secret_payload, secret_ref) elif encryption_type == 'encrypted': expected_kwargs = { 'name': secret_doc['metadata']['name'], - 'secret_type': secrets_manager.SecretsManager._get_secret_type( + 'secret_type': driver.BarbicanDriver._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) + self.assertEqual(self.secret_ref, secret_ref) + self.mock_barbicanclient.call.assert_called_once_with( + 'secrets.create', **expected_kwargs) - return created_secret, payload + return secret_ref, payload def test_create_cleartext_certificate(self): self._test_create_secret('cleartext', 'Certificate') @@ -102,11 +113,59 @@ class TestSecretsManager(test_base.TestDbBase): def test_retrieve_barbican_secret(self): secret_ref, expected_secret = self._test_create_secret( 'encrypted', 'Certificate') - secret_payload = secrets_manager.SecretsManager.get(secret_ref) + self.mock_barbicanclient.get_secret.return_value = ( + mock.Mock(payload=expected_secret)) + + 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_ref) + self.mock_barbicanclient.call.assert_called_with( + 'secrets.get', secret_ref) + + def test_empty_payload_skips_encryption(self): + for empty_payload in ('', {}, []): + secret_doc = self.factory.gen_test( + 'Certificate', 'encrypted', empty_payload) + + self._mock_barbican_client_call(empty_payload) + retrieved_payload = secrets_manager.SecretsManager.create( + secret_doc) + + self.assertEqual(empty_payload, retrieved_payload) + self.assertEqual('cleartext', + secret_doc['metadata']['storagePolicy']) + self.mock_barbicanclient.call.assert_not_called() + + def test_create_and_retrieve_base64_encoded_payload(self): + # Validate base64-encoded encryption. + payload = {'foo': 'bar'} + secret_doc = self.factory.gen_test( + 'Certificate', 'encrypted', payload) + + expected_payload = base64.encode_as_text(six.text_type({'foo': 'bar'})) + expected_kwargs = { + 'name': secret_doc['metadata']['name'], + 'secret_type': 'opaque', + 'payload': expected_payload + } + + self._mock_barbican_client_call(payload) + secret_ref = secrets_manager.SecretsManager.create(secret_doc) + + self.assertEqual(self.secret_ref, secret_ref) + self.assertEqual('encrypted', + secret_doc['metadata']['storagePolicy']) + self.mock_barbicanclient.call.assert_called_once_with( + "secrets.create", **expected_kwargs) + + # Validate base64-encoded decryption. + self.mock_barbicanclient.get_secret.return_value = ( + mock.Mock(payload=expected_payload, secret_type='opaque')) + + dummy_document = document_wrapper.DocumentDict({}) + retrieved_payload = secrets_manager.SecretsManager.get( + secret_ref, dummy_document, dummy_document) + self.assertEqual(payload, retrieved_payload) class TestSecretsSubstitution(test_base.TestDbBase): @@ -197,7 +256,8 @@ class TestSecretsSubstitution(test_base.TestDbBase): } self._test_doc_substitution( document_mapping, [certificate], expected_data) - mock_secrets_manager.get.assert_called_once_with(secret_ref=secret_ref) + mock_secrets_manager.get.assert_called_once_with( + secret_ref, certificate, mock.ANY) def test_create_destination_path_with_array(self): # Validate that the destination data will be populated with an array diff --git a/deckhand/types.py b/deckhand/types.py index aa7efef7..c7c8a055 100644 --- a/deckhand/types.py +++ b/deckhand/types.py @@ -12,33 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -DOCUMENT_SCHEMA_TYPES = ( - CERTIFICATE_AUTHORITY_SCHEMA, - CERTIFICATE_KEY_AUTHORITY_SCHEMA, - CERTIFICATE_SCHEMA, - CERTIFICATE_KEY_SCHEMA, - PRIVATE_KEY_SCHEMA, - PUBLIC_KEY_SCHEMA, - PASSPHRASE_SCHEMA, - DATA_SCHEMA_SCHEMA, - LAYERING_POLICY_SCHEMA, - PASSPHRASE_SCHEMA, - VALIDATION_POLICY_SCHEMA, -) = ( - 'deckhand/CertificateAuthority', - 'deckhand/CertificateAuthorityKey', - 'deckhand/Certificate', - 'deckhand/CertificateKey', - 'deckhand/PrivateKey', - 'deckhand/PublicKey', - 'deckhand/Passphrase', - 'deckhand/DataSchema', - 'deckhand/LayeringPolicy', - 'deckhand/Passphrase', - 'deckhand/ValidationPolicy', -) - - DOCUMENT_SECRET_TYPES = ( CERTIFICATE_AUTHORITY_SCHEMA, CERTIFICATE_KEY_AUTHORITY_SCHEMA, @@ -52,12 +25,26 @@ DOCUMENT_SECRET_TYPES = ( 'deckhand/CertificateAuthorityKey', 'deckhand/Certificate', 'deckhand/CertificateKey', + 'deckhand/Passphrase', 'deckhand/PrivateKey', 'deckhand/PublicKey', - 'deckhand/Passphrase' ) +DOCUMENT_SCHEMA_TYPES = ( + DATA_SCHEMA_SCHEMA, + LAYERING_POLICY_SCHEMA, + VALIDATION_POLICY_SCHEMA, +) = ( + 'deckhand/DataSchema', + 'deckhand/LayeringPolicy', + 'deckhand/ValidationPolicy', +) + + +DOCUMENT_SCHEMA_TYPES += DOCUMENT_SECRET_TYPES + + DECKHAND_VALIDATION_TYPES = ( DECKHAND_SCHEMA_VALIDATION, ) = ( @@ -70,5 +57,5 @@ ENCRYPTION_TYPES = ( ENCRYPTED ) = ( 'cleartext', - 'encrypted' + 'encrypted', ) diff --git a/doc/source/api_ref.rst b/doc/source/api_ref.rst index 6b7bcf82..c7fd34a6 100644 --- a/doc/source/api_ref.rst +++ b/doc/source/api_ref.rst @@ -71,8 +71,8 @@ Supported query string parameters: * ``metadata.name`` - string, optional * ``metadata.layeringDefinition.abstract`` - string, optional - Valid values are the "true" and "false". -* ``metadata.layeringDefinition.layer`` - string, optional - Only return documents from - the specified layer. +* ``metadata.layeringDefinition.layer`` - string, optional - Only return + documents from the specified layer. * ``metadata.label`` - string, optional, repeatable - Uses the format ``metadata.label=key=value``. Repeating this parameter indicates all requested labels must apply (AND not OR). @@ -86,7 +86,8 @@ Supported query string parameters: "asc". Controls the order in which the ``sort`` result is returned: "asc" returns sorted results in ascending order, while "desc" returns results in descending order. -* ``limit`` - int - Controls number of documents returned by this endpoint. +* ``limit`` - int, optional - Controls number of documents returned by this + endpoint. GET ``/revisions/{revision_id}/rendered-documents`` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^