From 197c907dffa753a5ae8313ab8a07a5b831d56b91 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Tue, 26 Feb 2019 13:48:44 +0100 Subject: [PATCH 1/8] Add test for Keystone security-checklist action --- zaza/charm_tests/keystone/tests.py | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/zaza/charm_tests/keystone/tests.py b/zaza/charm_tests/keystone/tests.py index 3a0c4b0..6c0ebb6 100644 --- a/zaza/charm_tests/keystone/tests.py +++ b/zaza/charm_tests/keystone/tests.py @@ -152,6 +152,37 @@ class CharmOperationTest(BaseKeystoneTest): .format(pprint.pformat(unit_repo), pprint.pformat(lead_repo))) + def test_security_checklist(self): + """Verify expected state with security-checklist""" + logging.info('Running `security-checklist` action on Keystone leader unit') + action = zaza.model.run_action_on_leader( + 'keystone', + 'security-checklist', + action_params={}) + assert action.data["status"] == "failed", \ + "Security check is expected to not pass by default" + results = action.data['results'] + expected_failures = [ + 'check-max-request-body-size', + 'disable-admin-token', + 'uses-sha256-for-hashing-tokens', + 'validate-file-ownership', + 'validate-file-permissions', + ] + expected_pass = [ + 'uses-fernet-token-after-default', + 'insecure-debug-is-false', + ] + for key, value in results.items(): + if key in expected_failures: + assert "FAIL" in value, "Unexpected test pass: {}".format(key) + if key in expected_pass: + self.assertEqual(value, + "PASS", + "Unexpected failure: {}".format(key)) + assert results['uses-fernet-token-after-default'] == 'PASS' + assert results['insecure-debug-is-false'] == 'PASS' + class AuthenticationAuthorizationTest(BaseKeystoneTest): """Keystone authentication and authorization tests.""" From efea4db4448cd63d136ab7be2b16cea627448a54 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Tue, 26 Feb 2019 14:03:13 +0100 Subject: [PATCH 2/8] migrate generic test features to utils --- zaza/charm_tests/keystone/tests.py | 32 ++++++++++++------------------ zaza/utilities/generic.py | 27 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/zaza/charm_tests/keystone/tests.py b/zaza/charm_tests/keystone/tests.py index 6c0ebb6..05db548 100644 --- a/zaza/charm_tests/keystone/tests.py +++ b/zaza/charm_tests/keystone/tests.py @@ -24,6 +24,7 @@ import zaza.model import zaza.utilities.exceptions as zaza_exceptions import zaza.utilities.juju as juju_utils import zaza.utilities.openstack as openstack_utils +import zaza.utilities.generic as generic_utils from zaza.charm_tests.keystone import ( BaseKeystoneTest, @@ -153,15 +154,7 @@ class CharmOperationTest(BaseKeystoneTest): pprint.pformat(lead_repo))) def test_security_checklist(self): - """Verify expected state with security-checklist""" - logging.info('Running `security-checklist` action on Keystone leader unit') - action = zaza.model.run_action_on_leader( - 'keystone', - 'security-checklist', - action_params={}) - assert action.data["status"] == "failed", \ - "Security check is expected to not pass by default" - results = action.data['results'] + """Verify expected state with security-checklist.""" expected_failures = [ 'check-max-request-body-size', 'disable-admin-token', @@ -169,19 +162,20 @@ class CharmOperationTest(BaseKeystoneTest): 'validate-file-ownership', 'validate-file-permissions', ] - expected_pass = [ + expected_passes = [ 'uses-fernet-token-after-default', 'insecure-debug-is-false', ] - for key, value in results.items(): - if key in expected_failures: - assert "FAIL" in value, "Unexpected test pass: {}".format(key) - if key in expected_pass: - self.assertEqual(value, - "PASS", - "Unexpected failure: {}".format(key)) - assert results['uses-fernet-token-after-default'] == 'PASS' - assert results['insecure-debug-is-false'] == 'PASS' + + logging.info('Running `security-checklist` action' + ' on Keystone leader unit') + generic_utils.audit_assertions( + zaza.model.run_action_on_leader( + 'keystone', + 'security-checklist', + action_params={}), + expected_passes, + expected_failures) class AuthenticationAuthorizationTest(BaseKeystoneTest): diff --git a/zaza/utilities/generic.py b/zaza/utilities/generic.py index e9e9e4c..72c7d21 100644 --- a/zaza/utilities/generic.py +++ b/zaza/utilities/generic.py @@ -68,6 +68,33 @@ def get_network_config(net_topology, ignore_env_vars=False, return net_info +def audit_assertions(action, expected_passes, expected_failures=None): + """Check expected assertion failures in security-checklist actions. + + :param action: Action object from running the security-checklist action + :type action: juju.action.Action + :param expected_passes: List of test names that are expected to pass + :type expected_passes: List(str) + :param expected_failures: List of test names that are expected to fail + :type expexted_failures: List(str) + """ + if expected_failures is None: + expected_failures = [] + if expected_failures: + assert action.data["status"] == "failed", \ + "Security check is not expected to pass by default" + else: + assert action.data["status"] == "completed", \ + "Security check is expected to pass by default" + + results = action.data['results'] + for key, value in results.items(): + if key in expected_failures: + assert "FAIL" in value, "Unexpected test pass: {}".format(key) + if key in expected_passes: + assert value == "PASS", "Unexpected failure: {}".format(key) + + def get_pkg_version(application, pkg): """Return package version. From a97296e32ab54aa001ec8f4f9abf7e0d4272ba30 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Wed, 27 Feb 2019 08:24:36 +0100 Subject: [PATCH 3/8] Move audit validation to test_utils --- zaza/charm_tests/keystone/tests.py | 4 ++-- zaza/charm_tests/test_utils.py | 30 ++++++++++++++++++++++++++++++ zaza/utilities/generic.py | 27 --------------------------- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/zaza/charm_tests/keystone/tests.py b/zaza/charm_tests/keystone/tests.py index 05db548..f958ede 100644 --- a/zaza/charm_tests/keystone/tests.py +++ b/zaza/charm_tests/keystone/tests.py @@ -24,8 +24,8 @@ import zaza.model import zaza.utilities.exceptions as zaza_exceptions import zaza.utilities.juju as juju_utils import zaza.utilities.openstack as openstack_utils -import zaza.utilities.generic as generic_utils +import zaza.charm_tests.test_utils as test_utils from zaza.charm_tests.keystone import ( BaseKeystoneTest, DEMO_DOMAIN, @@ -169,7 +169,7 @@ class CharmOperationTest(BaseKeystoneTest): logging.info('Running `security-checklist` action' ' on Keystone leader unit') - generic_utils.audit_assertions( + test_utils.audit_assertions( zaza.model.run_action_on_leader( 'keystone', 'security-checklist', diff --git a/zaza/charm_tests/test_utils.py b/zaza/charm_tests/test_utils.py index 22de8ed..a28094e 100644 --- a/zaza/charm_tests/test_utils.py +++ b/zaza/charm_tests/test_utils.py @@ -38,6 +38,36 @@ def skipIfNotHA(service_name): return _skipIfNotHA_inner_1 +def audit_assertions(action, + expected_passes, + expected_failures=None, + expected_to_pass=True): + """Check expected assertion failures in security-checklist actions. + + :param action: Action object from running the security-checklist action + :type action: juju.action.Action + :param expected_passes: List of test names that are expected to pass + :type expected_passes: List(str) + :param expected_failures: List of test names that are expected to fail + :type expexted_failures: List(str) + """ + if expected_failures is None: + expected_failures = [] + if expected_to_pass: + assert action.data["status"] == "completed", \ + "Security check is expected to pass by default" + else: + assert action.data["status"] == "failed", \ + "Security check is not expected to pass by default" + + results = action.data['results'] + for key, value in results.items(): + if key in expected_failures: + assert "FAIL" in value, "Unexpected test pass: {}".format(key) + if key in expected_passes: + assert value == "PASS", "Unexpected failure: {}".format(key) + + class OpenStackBaseTest(unittest.TestCase): """Generic helpers for testing OpenStack API charms.""" diff --git a/zaza/utilities/generic.py b/zaza/utilities/generic.py index 72c7d21..e9e9e4c 100644 --- a/zaza/utilities/generic.py +++ b/zaza/utilities/generic.py @@ -68,33 +68,6 @@ def get_network_config(net_topology, ignore_env_vars=False, return net_info -def audit_assertions(action, expected_passes, expected_failures=None): - """Check expected assertion failures in security-checklist actions. - - :param action: Action object from running the security-checklist action - :type action: juju.action.Action - :param expected_passes: List of test names that are expected to pass - :type expected_passes: List(str) - :param expected_failures: List of test names that are expected to fail - :type expexted_failures: List(str) - """ - if expected_failures is None: - expected_failures = [] - if expected_failures: - assert action.data["status"] == "failed", \ - "Security check is not expected to pass by default" - else: - assert action.data["status"] == "completed", \ - "Security check is expected to pass by default" - - results = action.data['results'] - for key, value in results.items(): - if key in expected_failures: - assert "FAIL" in value, "Unexpected test pass: {}".format(key) - if key in expected_passes: - assert value == "PASS", "Unexpected failure: {}".format(key) - - def get_pkg_version(application, pkg): """Return package version. From e9136c7e38280f082da285f38511786b9d5c0f26 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Wed, 27 Feb 2019 12:06:15 +0100 Subject: [PATCH 4/8] do not expect keystone security-audit to pass --- zaza/charm_tests/keystone/tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zaza/charm_tests/keystone/tests.py b/zaza/charm_tests/keystone/tests.py index f958ede..a0041cb 100644 --- a/zaza/charm_tests/keystone/tests.py +++ b/zaza/charm_tests/keystone/tests.py @@ -175,7 +175,8 @@ class CharmOperationTest(BaseKeystoneTest): 'security-checklist', action_params={}), expected_passes, - expected_failures) + expected_failures, + expected_to_pass=False) class AuthenticationAuthorizationTest(BaseKeystoneTest): From 770655a87a3563cf3f1a443a5a5c8bc18ca0d4a3 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Wed, 27 Feb 2019 14:45:19 +0100 Subject: [PATCH 5/8] fix type typo --- zaza/charm_tests/test_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zaza/charm_tests/test_utils.py b/zaza/charm_tests/test_utils.py index a28094e..1b9f65c 100644 --- a/zaza/charm_tests/test_utils.py +++ b/zaza/charm_tests/test_utils.py @@ -47,9 +47,9 @@ def audit_assertions(action, :param action: Action object from running the security-checklist action :type action: juju.action.Action :param expected_passes: List of test names that are expected to pass - :type expected_passes: List(str) + :type expected_passes: List[str] :param expected_failures: List of test names that are expected to fail - :type expexted_failures: List(str) + :type expexted_failures: List[str] """ if expected_failures is None: expected_failures = [] From 2268fc8006d0940639dcc7d52cff930ff56a5f1f Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Thu, 28 Feb 2019 12:22:29 +0100 Subject: [PATCH 6/8] add document about AssertionError --- zaza/charm_tests/test_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/zaza/charm_tests/test_utils.py b/zaza/charm_tests/test_utils.py index 1b9f65c..e92e62e 100644 --- a/zaza/charm_tests/test_utils.py +++ b/zaza/charm_tests/test_utils.py @@ -50,6 +50,7 @@ def audit_assertions(action, :type expected_passes: List[str] :param expected_failures: List of test names that are expected to fail :type expexted_failures: List[str] + :raises: AssertionError if the assertion fails. """ if expected_failures is None: expected_failures = [] From 16c282b22086fc8cb559f7f0bc4f756e214b6579 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Fri, 1 Mar 2019 09:12:49 +0100 Subject: [PATCH 7/8] Add keystone SecurityTests class --- zaza/charm_tests/keystone/tests.py | 59 +++++++++++++++++------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/zaza/charm_tests/keystone/tests.py b/zaza/charm_tests/keystone/tests.py index a0041cb..72cf885 100644 --- a/zaza/charm_tests/keystone/tests.py +++ b/zaza/charm_tests/keystone/tests.py @@ -153,31 +153,6 @@ class CharmOperationTest(BaseKeystoneTest): .format(pprint.pformat(unit_repo), pprint.pformat(lead_repo))) - def test_security_checklist(self): - """Verify expected state with security-checklist.""" - expected_failures = [ - 'check-max-request-body-size', - 'disable-admin-token', - 'uses-sha256-for-hashing-tokens', - 'validate-file-ownership', - 'validate-file-permissions', - ] - expected_passes = [ - 'uses-fernet-token-after-default', - 'insecure-debug-is-false', - ] - - logging.info('Running `security-checklist` action' - ' on Keystone leader unit') - test_utils.audit_assertions( - zaza.model.run_action_on_leader( - 'keystone', - 'security-checklist', - action_params={}), - expected_passes, - expected_failures, - expected_to_pass=False) - class AuthenticationAuthorizationTest(BaseKeystoneTest): """Keystone authentication and authorization tests.""" @@ -350,3 +325,37 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest): openrc.update( {'OS_AUTH_URL': 'http://{}:5000/v3'.format(ip)}) _validate_token_data(openrc) + + +class SecurityTests(BaseKeystoneTest): + """Keystone security tests tests.""" + + @classmethod + def setUpClass(cls): + """Run class setup for running Keystone aa-tests.""" + super(SecurityTests, cls).setUpClass() + + def test_security_checklist(self): + """Verify expected state with security-checklist.""" + expected_failures = [ + 'check-max-request-body-size', + 'disable-admin-token', + 'uses-sha256-for-hashing-tokens', + 'validate-file-ownership', + 'validate-file-permissions', + ] + expected_passes = [ + 'uses-fernet-token-after-default', + 'insecure-debug-is-false', + ] + + logging.info('Running `security-checklist` action' + ' on Keystone leader unit') + test_utils.audit_assertions( + zaza.model.run_action_on_leader( + 'keystone', + 'security-checklist', + action_params={}), + expected_passes, + expected_failures, + expected_to_pass=False) From 401d99d14f5ec6d63c31cecbbf20d5d05000c2e6 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Wed, 6 Mar 2019 08:25:56 +0100 Subject: [PATCH 8/8] Add comment describing upcoming fixes to failures --- zaza/charm_tests/keystone/tests.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/zaza/charm_tests/keystone/tests.py b/zaza/charm_tests/keystone/tests.py index 72cf885..fb22a26 100644 --- a/zaza/charm_tests/keystone/tests.py +++ b/zaza/charm_tests/keystone/tests.py @@ -337,6 +337,9 @@ class SecurityTests(BaseKeystoneTest): def test_security_checklist(self): """Verify expected state with security-checklist.""" + # Changes fixing the below expected failures will be made following + # this initial work to get validation in. There will be bugs targeted + # to each one and resolved independently where possible. expected_failures = [ 'check-max-request-body-size', 'disable-admin-token',