Barbican driver simplification

Under some circumstances, the payloads retrieved from Barbican do not
match what was stored. This primarily affects surrounding whitespace[0],
but the implications for passphrases are significant, and even for PEM
encoded data, a difference in whitespace in a configmap is enough to
trigger a chart upgrade.

In general, the effort to align Deckhand document types with Barbican
secret types adds complexity without tangible benefit. Barbican does no
enforcement of the contents of the data, and if it did, that could lead
to further incompatibilities.

This change uses the 'opaque' secret type for all secret document types.
Before storage (or caching), the payload is serialized using `repr`, and
base64 encoded. Upon retrieval, the payload is base64 decoded and parsed
back into an object with `ast.literal_eval`.

[0]: https://storyboard.openstack.org/#!/story/2007017

Change-Id: I9c2f3427f52a87aad718f95160cf688db35e1b83
This commit is contained in:
Phil Sphicas 2020-01-09 06:55:16 +00:00
parent fa15c0b582
commit 4ccb4368ce
6 changed files with 30 additions and 87 deletions

View File

@ -20,7 +20,6 @@ from oslo_config import cfg
from oslo_log import log as logging
from oslo_serialization import base64
from oslo_utils import excutils
import six
from deckhand.barbican import cache
from deckhand.barbican import client_wrapper
@ -36,67 +35,15 @@ class BarbicanDriver(object):
def __init__(self):
self.barbicanclient = client_wrapper.BarbicanClientWrapper()
@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:
secret_type = None
# Explicitly list the "empty" payloads we are refusing to store.
# We don't use ``if not payload`` because that would not encrypt
# and store something like ``data: !!int 0``
if payload in ('', {}, [], None):
# There is no point in even bothering to encrypt an empty
# body, which just leads to needless overhead, so return
# early.
@ -104,19 +51,14 @@ class BarbicanDriver(object):
'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.
else:
LOG.debug('Setting secret_type=opaque and '
'base64-encoding payload of type %s for '
'document [%s, %s] %s.', type(payload),
secret_doc.schema, secret_doc.layer, secret_doc.name)
secret_type = 'opaque'
try:
payload = base64.encode_as_text(six.text_type(payload))
payload = base64.encode_as_text(repr(payload))
except Exception:
message = ('Failed to base64-encode payload of type %s '
'for Barbican storage.', type(payload))
@ -222,7 +164,7 @@ class BarbicanDriver(object):
payload = secret.payload
if secret.secret_type == 'opaque':
LOG.debug('Forcibly base64-decoding original non-string payload '
LOG.debug('Base64-decoding original payload '
'for document [%s, %s] %s.', *src_doc.meta)
secret = self._base64_decode_payload(payload)
else:

View File

@ -65,7 +65,8 @@ tests:
response_headers:
content-type: application/octet-stream; charset=UTF-8
response_strings:
- not-a-real-password
# base64.b64encode(repr("not-a-real-password"))
- J25vdC1hLXJlYWwtcGFzc3dvcmQn
- name: verify_revision_documents_returns_secret_ref
desc: Verify that the documents for the created revision returns the secret ref.

View File

@ -82,7 +82,7 @@ tests:
$.[0].data.`split(:, 0, 1)` + "://" + $.[0].data.`split(/, 2, 3)`: $ENVIRON['TEST_BARBICAN_URL']
- name: verify_generic_secret_created_in_barbican
desc: Validate that the generic secret gets stored with secret_type passphrase.
desc: Validate that the generic secret gets stored with secret_type opaque.
GET: $ENVIRON['TEST_BARBICAN_URL']/v1/secrets/$RESPONSE['$.[0].data.`split(/, 5, -1)`']
status: 200
request_headers:
@ -93,7 +93,7 @@ tests:
$.status: ACTIVE
$.name: example-armada-cert
# Default type for documents with generic schema.
$.secret_type: passphrase
$.secret_type: opaque
- name: verify_secret_payload_in_destination_document
desc: Verify secret payload is injected in destination document as well as example-armada-cert.

View File

@ -222,13 +222,13 @@ tests:
- example-private-key
- example-public-key
$.secrets[*].secret_type:
- certificate
- private
- certificate
- private
- passphrase
- private
- public
- opaque
- opaque
- opaque
- opaque
- opaque
- opaque
- opaque
- name: verify_secret_payload_in_destination_document
desc: |

View File

@ -62,7 +62,8 @@ tests:
response_headers:
content-type: application/octet-stream; charset=UTF-8
response_strings:
- not-a-real-password
# base64.b64encode(repr("not-a-real-password"))
- J25vdC1hLXJlYWwtcGFzc3dvcmQn
- name: delete_all_revisions
desc: Delete all revisions from Deckhand, which should delete all secrets.

View File

@ -18,10 +18,8 @@ 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
@ -64,9 +62,8 @@ class TestSecretsManager(test_base.DeckhandWithDBTestCase):
elif encryption_type == 'encrypted':
expected_kwargs = {
'name': secret_doc['metadata']['name'],
'secret_type': driver.BarbicanDriver._get_secret_type(
'deckhand/' + secret_type),
'payload': payload
'secret_type': 'opaque',
'payload': base64.encode_as_text(repr(payload))
}
self.assertEqual(self.secret_ref, secret_ref)
self.mock_barbicanclient.call.assert_called_once_with(
@ -123,6 +120,8 @@ class TestSecretsManager(test_base.DeckhandWithDBTestCase):
'secrets.get', secret_ref)
def test_empty_payload_skips_encryption(self):
# NOTE: Not testing for the `None` case here, because gen_test
# factory method interprets `None` as "generate a passphrase"
for empty_payload in ('', {}, []):
secret_doc = self.factory.gen_test(
'Certificate', 'encrypted', empty_payload)
@ -142,7 +141,7 @@ class TestSecretsManager(test_base.DeckhandWithDBTestCase):
secret_doc = self.factory.gen_test(
'Certificate', 'encrypted', payload)
expected_payload = base64.encode_as_text(six.text_type({'foo': 'bar'}))
expected_payload = base64.encode_as_text(repr({'foo': 'bar'}))
expected_kwargs = {
'name': secret_doc['metadata']['name'],
'secret_type': 'opaque',