Delete secret references from Barbican when deleting all revisions
This patchset adds logic to delete all secret references from Barbican when deleting all Deckhand revisions -- because this action effectively purges the Deckhand database. Consequently, to avoid stale secrets lingering about in Barbican, this action should clean up those secret references too. Change-Id: I88782404bd7f6d953cae343f21231b2943eaf7d2
This commit is contained in:
parent
29894ee854
commit
1c394a2bdc
@ -200,3 +200,17 @@ class BarbicanDriver(object):
|
|||||||
secret = payload
|
secret = payload
|
||||||
|
|
||||||
return secret
|
return secret
|
||||||
|
|
||||||
|
def delete_secret(self, secret_ref):
|
||||||
|
"""Delete a secret."""
|
||||||
|
try:
|
||||||
|
return self.barbicanclient.call("secrets.delete", secret_ref)
|
||||||
|
except (barbicanclient.exceptions.HTTPAuthError,
|
||||||
|
barbicanclient.exceptions.HTTPServerError) as e:
|
||||||
|
LOG.exception(str(e))
|
||||||
|
raise errors.BarbicanException(details=str(e))
|
||||||
|
except barbicanclient.exceptions.HTTPClientError as e:
|
||||||
|
if e.status_code == 404:
|
||||||
|
LOG.warning('Could not delete secret %s because it was not '
|
||||||
|
'found. Assuming it no longer exists.', secret_ref)
|
||||||
|
raise
|
||||||
|
@ -21,6 +21,7 @@ from deckhand.control import base as api_base
|
|||||||
from deckhand.control import common
|
from deckhand.control import common
|
||||||
from deckhand.control.views import revision as revision_view
|
from deckhand.control.views import revision as revision_view
|
||||||
from deckhand.db.sqlalchemy import api as db_api
|
from deckhand.db.sqlalchemy import api as db_api
|
||||||
|
from deckhand.engine import secrets_manager
|
||||||
from deckhand import errors
|
from deckhand import errors
|
||||||
from deckhand import policy
|
from deckhand import policy
|
||||||
|
|
||||||
@ -74,7 +75,19 @@ class RevisionsResource(api_base.BaseResource):
|
|||||||
resp.status = falcon.HTTP_200
|
resp.status = falcon.HTTP_200
|
||||||
resp.body = self.view_builder.list(revisions)
|
resp.body = self.view_builder.list(revisions)
|
||||||
|
|
||||||
|
def _delete_all_barbican_secrets(self):
|
||||||
|
filters = {'metadata.storagePolicy': 'encrypted'}
|
||||||
|
# NOTE(felipemonteiro): Don't pass `unique_only` because we want
|
||||||
|
# all unique secret references (just the data section), not unique
|
||||||
|
# documents, which considers all attributes.
|
||||||
|
encrypted_documents = db_api.document_get_all(**filters)
|
||||||
|
|
||||||
|
for document in encrypted_documents:
|
||||||
|
secrets_manager.SecretsManager.delete(document)
|
||||||
|
|
||||||
@policy.authorize('deckhand:delete_revisions')
|
@policy.authorize('deckhand:delete_revisions')
|
||||||
def on_delete(self, req, resp):
|
def on_delete(self, req, resp):
|
||||||
|
self._delete_all_barbican_secrets()
|
||||||
|
|
||||||
db_api.revision_delete_all()
|
db_api.revision_delete_all()
|
||||||
resp.status = falcon.HTTP_204
|
resp.status = falcon.HTTP_204
|
||||||
|
@ -586,6 +586,7 @@ def revision_delete_all():
|
|||||||
:returns: None
|
:returns: None
|
||||||
"""
|
"""
|
||||||
engine = get_engine()
|
engine = get_engine()
|
||||||
|
|
||||||
if engine.name == 'postgresql':
|
if engine.name == 'postgresql':
|
||||||
# NOTE(fmontei): While cascade should delete all data from all tables,
|
# NOTE(fmontei): While cascade should delete all data from all tables,
|
||||||
# we also need to reset the index to 1 for each table.
|
# we also need to reset the index to 1 for each table.
|
||||||
@ -658,7 +659,7 @@ def revision_documents_get(revision_id=None, include_history=True,
|
|||||||
ID is ``None``, then retrieve the latest revision, if one exists.
|
ID is ``None``, then retrieve the latest revision, if one exists.
|
||||||
:param include_history: Return all documents for revision history prior
|
:param include_history: Return all documents for revision history prior
|
||||||
and up to current revision, if ``True``. Default is ``True``.
|
and up to current revision, if ``True``. Default is ``True``.
|
||||||
:param unique_only: Return only unique documents if ``True. Default is
|
:param unique_only: Return only unique documents if ``True``. Default is
|
||||||
``True``.
|
``True``.
|
||||||
:param session: Database session object.
|
:param session: Database session object.
|
||||||
:param filters: Key-value pairs used for filtering out revision documents.
|
:param filters: Key-value pairs used for filtering out revision documents.
|
||||||
|
@ -91,6 +91,26 @@ class SecretsManager(object):
|
|||||||
LOG.debug('Successfully retrieved Barbican secret using reference.')
|
LOG.debug('Successfully retrieved Barbican secret using reference.')
|
||||||
return secret
|
return secret
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def delete(cls, document):
|
||||||
|
"""Delete a secret from Barbican.
|
||||||
|
|
||||||
|
:param dict document: Document with secret_ref in ``data`` section with
|
||||||
|
format: "https://{barbican_host}/v1/secrets/{secret_uuid}"
|
||||||
|
:returns: None
|
||||||
|
|
||||||
|
"""
|
||||||
|
if not isinstance(document, document_wrapper.DocumentDict):
|
||||||
|
document = document_wrapper.DocumentDict(document)
|
||||||
|
|
||||||
|
secret_ref = document.data
|
||||||
|
if document.is_encrypted and document.has_barbican_ref:
|
||||||
|
LOG.debug('Deleting Barbican secret: %s.', secret_ref)
|
||||||
|
cls.barbican_driver.delete_secret(secret_ref=secret_ref)
|
||||||
|
else:
|
||||||
|
LOG.warning('%s is not a valid Barbican secret. Could not delete.',
|
||||||
|
secret_ref)
|
||||||
|
|
||||||
|
|
||||||
class SecretsSubstitution(object):
|
class SecretsSubstitution(object):
|
||||||
"""Class for document substitution logic for YAML files."""
|
"""Class for document substitution logic for YAML files."""
|
||||||
|
@ -21,6 +21,7 @@ import tempfile
|
|||||||
import yaml
|
import yaml
|
||||||
|
|
||||||
from gabbi import driver
|
from gabbi import driver
|
||||||
|
from gabbi.handlers import core
|
||||||
from gabbi.handlers import jsonhandler
|
from gabbi.handlers import jsonhandler
|
||||||
|
|
||||||
TEST_DIR = None
|
TEST_DIR = None
|
||||||
@ -105,5 +106,9 @@ def load_tests(loader, tests, pattern):
|
|||||||
# the same content-type, the one that is earliest in the list will be
|
# the same content-type, the one that is earliest in the list will be
|
||||||
# used. Thus, we cannot specify multiple content handlers for handling
|
# used. Thus, we cannot specify multiple content handlers for handling
|
||||||
# list/dictionary responses from the server using different handlers.
|
# list/dictionary responses from the server using different handlers.
|
||||||
content_handlers=[MultidocJsonpaths],
|
content_handlers=[
|
||||||
|
core.StringResponseHandler, # For parsing text/plain
|
||||||
|
jsonhandler.JSONHandler, # For parsing application/json
|
||||||
|
MultidocJsonpaths # For parsing application/x-yaml
|
||||||
|
],
|
||||||
verbose=True)
|
verbose=True)
|
||||||
|
@ -0,0 +1,87 @@
|
|||||||
|
# Tests success paths for deleting all revisions:
|
||||||
|
#
|
||||||
|
# 1. Tests that deleting all revisions purges secret references from
|
||||||
|
# Barbican.
|
||||||
|
|
||||||
|
defaults:
|
||||||
|
verbose: true
|
||||||
|
|
||||||
|
tests:
|
||||||
|
- name: purge
|
||||||
|
desc: Begin testing from known state.
|
||||||
|
DELETE: /api/v1.0/revisions
|
||||||
|
status: 204
|
||||||
|
request_headers:
|
||||||
|
content-type: application/x-yaml
|
||||||
|
X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN']
|
||||||
|
response_headers: null
|
||||||
|
|
||||||
|
- name: create_encrypted_passphrase
|
||||||
|
desc: Create passphrase with storagePolicy=encrypted
|
||||||
|
PUT: /api/v1.0/buckets/secret/documents
|
||||||
|
status: 200
|
||||||
|
request_headers:
|
||||||
|
content-type: application/x-yaml
|
||||||
|
X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN']
|
||||||
|
response_headers:
|
||||||
|
content-type: application/x-yaml
|
||||||
|
data: |-
|
||||||
|
---
|
||||||
|
schema: deckhand/Passphrase/v1
|
||||||
|
metadata:
|
||||||
|
schema: metadata/Document/v1
|
||||||
|
name: my-passphrase
|
||||||
|
layeringDefinition:
|
||||||
|
layer: fake
|
||||||
|
storagePolicy: encrypted
|
||||||
|
data: not-a-real-password
|
||||||
|
...
|
||||||
|
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: validate_secret_exists_in_barbican
|
||||||
|
desc: Validate that the secret ref exists in Barbican
|
||||||
|
GET: $ENVIRON['TEST_BARBICAN_URL']/v1/secrets/$HISTORY['create_encrypted_passphrase'].$RESPONSE['$.[0].data.`split(/, 5, -1)`']
|
||||||
|
status: 200
|
||||||
|
request_headers:
|
||||||
|
X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN']
|
||||||
|
response_headers:
|
||||||
|
content-type: application/json; charset=UTF-8
|
||||||
|
response_json_paths:
|
||||||
|
$.status: ACTIVE
|
||||||
|
|
||||||
|
- name: validate_secret_payload_matches_in_barbican
|
||||||
|
desc: Validate that the secret itself matches in Barbican
|
||||||
|
GET: $ENVIRON['TEST_BARBICAN_URL']/v1/secrets/$HISTORY['create_encrypted_passphrase'].$RESPONSE['$.[0].data.`split(/, 5, -1)`']/payload
|
||||||
|
status: 200
|
||||||
|
request_headers:
|
||||||
|
X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN']
|
||||||
|
response_headers:
|
||||||
|
content-type: application/octet-stream; charset=UTF-8
|
||||||
|
response_strings:
|
||||||
|
- not-a-real-password
|
||||||
|
|
||||||
|
- name: delete_all_revisions
|
||||||
|
desc: Delete all revisions from Deckhand, which should delete all secrets.
|
||||||
|
DELETE: /api/v1.0/revisions
|
||||||
|
status: 204
|
||||||
|
request_headers:
|
||||||
|
content-type: application/x-yaml
|
||||||
|
X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN']
|
||||||
|
response_headers: null
|
||||||
|
|
||||||
|
- name: validate_all_secrets_deleted_from_barbican
|
||||||
|
desc: |-
|
||||||
|
Validate that deleting all revisions deletes all secrets from Barbican.
|
||||||
|
GET: $ENVIRON['TEST_BARBICAN_URL']/v1/secrets
|
||||||
|
status: 200
|
||||||
|
request_headers:
|
||||||
|
X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN']
|
||||||
|
response_headers:
|
||||||
|
content-type: application/json; charset=UTF-8
|
||||||
|
response_json_paths:
|
||||||
|
$.secrets.`len`: 0
|
||||||
|
$.secrets: []
|
@ -12,12 +12,53 @@
|
|||||||
# See the License for the specific language governing permissions and
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
|
import os
|
||||||
|
|
||||||
|
import mock
|
||||||
import yaml
|
import yaml
|
||||||
|
|
||||||
|
from deckhand.engine import secrets_manager
|
||||||
from deckhand import factories
|
from deckhand import factories
|
||||||
|
from deckhand.tests import test_utils
|
||||||
from deckhand.tests.unit.control import base as test_base
|
from deckhand.tests.unit.control import base as test_base
|
||||||
|
|
||||||
|
|
||||||
|
class TestRevisionsController(test_base.BaseControllerTest):
|
||||||
|
|
||||||
|
def test_delete_revisions_also_deletes_barbican_secrets(self):
|
||||||
|
rules = {'deckhand:create_cleartext_documents': '@',
|
||||||
|
'deckhand:create_encrypted_documents': '@',
|
||||||
|
'deckhand:delete_revisions': '@'}
|
||||||
|
self.policy.set_rules(rules)
|
||||||
|
|
||||||
|
resource_path = os.path.join(os.getcwd(), 'deckhand', 'tests', 'unit',
|
||||||
|
'resources', 'sample_passphrase.yaml')
|
||||||
|
with open(resource_path) as f:
|
||||||
|
encrypted_document = f.read()
|
||||||
|
|
||||||
|
fake_secret_ref = test_utils.rand_barbican_ref()
|
||||||
|
with mock.patch.object(secrets_manager, 'SecretsManager',
|
||||||
|
autospec=True) as mock_secrets_mgr:
|
||||||
|
mock_secrets_mgr.create.return_value = fake_secret_ref
|
||||||
|
|
||||||
|
resp = self.app.simulate_put(
|
||||||
|
'/api/v1.0/buckets/mop/documents',
|
||||||
|
headers={'Content-Type': 'application/x-yaml'},
|
||||||
|
body=encrypted_document)
|
||||||
|
self.assertEqual(200, resp.status_code)
|
||||||
|
|
||||||
|
with mock.patch.object(secrets_manager.SecretsManager,
|
||||||
|
'barbican_driver', autospec=True) \
|
||||||
|
as m_barbican_driver:
|
||||||
|
resp = self.app.simulate_delete(
|
||||||
|
'/api/v1.0/revisions',
|
||||||
|
headers={'Content-Type': 'application/x-yaml'})
|
||||||
|
|
||||||
|
self.assertEqual(204, resp.status_code)
|
||||||
|
m_barbican_driver.delete_secret.assert_called_once_with(
|
||||||
|
fake_secret_ref)
|
||||||
|
|
||||||
|
|
||||||
class TestRevisionsControllerNegativeRBAC(test_base.BaseControllerTest):
|
class TestRevisionsControllerNegativeRBAC(test_base.BaseControllerTest):
|
||||||
"""Test suite for validating negative RBAC scenarios for revisions
|
"""Test suite for validating negative RBAC scenarios for revisions
|
||||||
controller.
|
controller.
|
||||||
|
Loading…
Reference in New Issue
Block a user