[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] 49505b9aec/barbicanclient/v1/secrets.py (L252)
[2] 49505b9aec/barbicanclient/v1/secrets.py (L297)

Change-Id: I1964aa84ad07b6f310b39974f078b84a1dc84983

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):
+    @classmethod
-        """Create a secret."""
+    def is_barbican_ref(cls, secret_ref):
-        secret = self.barbicanclient.call("secrets.create", **kwargs)
+        # 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
+        return secret_ref
-        # 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."""
 
+    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

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'

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_func = type_dict.get(key)
-                    cast_type = type_dict[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

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.',
+        LOG.debug(
-                  [(d['schema'], d['name']) for d in documents_to_create])
+            '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']

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()
+    barbican_driver = BarbicanDriver()
-
-    _url_re = re.compile(r'http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|'
-                         '(?:%[0-9a-fA-F][0-9a-fA-F]))+')
 
     @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
+        :param secret_doc: A Deckhand document with a schema that belongs to
-            schemas:
+            ``types.DOCUMENT_SECRET_TYPES``.
 
-                * ``deckhand/Certificate/v1``
+        :returns: Unecrypted data section from ``secret_doc`` if the document's
-                * ``deckhand/CertificateKey/v1``
+            ``storagePolicy`` is "cleartext" or a Barbican secret reference
-                * ``deckhand/Passphrase/v1``
+            if the ``storagePolicy`` is "encrypted'.
-
-        :returns: Dictionary representation of
-            ``deckhand.db.sqlalchemy.models.DocumentSecret``.
         """
         # 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']
+        if secret_doc.storage_policy == types.ENCRYPTED:
-        secret_type = cls._get_secret_type(secret_doc['schema'])
+            payload = cls.barbican_driver.create_secret(secret_doc)
-        created_secret = secret_doc['data']
+        else:
+            payload = secret_doc.data
 
-        if encryption_type == types.ENCRYPTED:
+        return payload
-            # 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
 
     @classmethod
-    def get(cls, secret_ref):
+    def get(cls, secret_ref, src_doc, dest_doc):
-        """Return a secret payload from Barbican.
+        """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(src_doc, dest_doc,
-        secret = cls.barbican_driver.get_secret(secret_ref=secret_ref)
+                                                secret_ref=secret_ref)
-        payload = secret.payload
         LOG.debug('Successfully retrieved Barbican secret using reference.')
-        return payload
+        return secret
-
-    @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
 
 
 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)

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

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.

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

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(
+        self.mock_barbicanclient = self.patchobject(
-            secrets_manager.SecretsManager, 'barbican_driver')
+            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):
+    def _mock_barbican_client_call(self, payload):
-        secret_data = test_utils.rand_password()
+        def fake_call(action, *args, **kwargs):
-        secret_doc = self.factory.gen_test(
+            if action == "secrets.create":
-            secret_type, encryption_type, secret_data)
+                return mock.Mock(**{'store.return_value': self.secret_ref})
-        payload = secret_doc['data']
+            elif action == "secrets.get":
-        self.mock_barbican_driver.get_secret.return_value = (
+                return mock.Mock(payload=payload)
-            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(
+            self.assertEqual(self.secret_ref, secret_ref)
-                **expected_kwargs)
+            self.mock_barbicanclient.call.assert_called_once_with(
-            self.assertEqual(self.secret_ref, created_secret)
+                '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(
+        self.mock_barbicanclient.call.assert_called_with(
-            secret_ref=secret_ref)
+            '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

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',
 )

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
+* ``metadata.layeringDefinition.layer`` - string, optional - Only return
-  the specified layer.
+  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``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^