From 9abb4ea5518dfe07ba1d0563834cfc6f6b9ba167 Mon Sep 17 00:00:00 2001 From: Mikael Arguedas Date: Sat, 2 May 2020 11:08:47 +0200 Subject: [PATCH 1/3] make load_cert a utility and reuse it everywhere Signed-off-by: Mikael Arguedas --- sros2/sros2/api/_key.py | 3 +-- sros2/sros2/api/_utilities.py | 10 +++++++--- .../commands/security/verbs/test_create_key.py | 18 ++++++------------ .../security/verbs/test_create_keystore.py | 15 +++++++-------- 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/sros2/sros2/api/_key.py b/sros2/sros2/api/_key.py index 0911ece1..30565017 100644 --- a/sros2/sros2/api/_key.py +++ b/sros2/sros2/api/_key.py @@ -126,8 +126,7 @@ def _is_key_name_valid(name): def _create_key_and_cert( keystore_ca_cert_path, keystore_ca_key_path, identity, cert_path, key_path): # Load the CA cert and key from disk - with open(keystore_ca_cert_path, 'rb') as f: - ca_cert = x509.load_pem_x509_certificate(f.read(), cryptography_backend()) + ca_cert = _utilities.load_cert(keystore_ca_cert_path) with open(keystore_ca_key_path, 'rb') as f: ca_key = serialization.load_pem_private_key(f.read(), None, cryptography_backend()) diff --git a/sros2/sros2/api/_utilities.py b/sros2/sros2/api/_utilities.py index ec522d9c..287ee1de 100644 --- a/sros2/sros2/api/_utilities.py +++ b/sros2/sros2/api/_utilities.py @@ -51,9 +51,7 @@ def get_keystore_path_from_env(): def create_smime_signed_file(cert_path, key_path, unsigned_file_path, signed_file_path): # Load the CA cert and key from disk - with open(cert_path, 'rb') as cert_file: - cert = x509.load_pem_x509_certificate( - cert_file.read(), cryptography_backend()) + cert = load_cert(cert_path) with open(key_path, 'rb') as key_file: private_key = serialization.load_pem_private_key( @@ -125,6 +123,12 @@ def write_cert(cert, cert_path, *, encoding=serialization.Encoding.PEM): f.write(cert.public_bytes(encoding=encoding)) +def load_cert(cert_path): + with open(cert_path, 'rb') as cert_file: + return x509.load_pem_x509_certificate( + cert_file.read(), cryptography_backend()) + + def _sign_bytes(cert, key, byte_string): # Using two flags here to get the output required: # - PKCS7_DETACHED: Use cleartext signing diff --git a/sros2/test/sros2/commands/security/verbs/test_create_key.py b/sros2/test/sros2/commands/security/verbs/test_create_key.py index fe70116d..7b96794e 100644 --- a/sros2/test/sros2/commands/security/verbs/test_create_key.py +++ b/sros2/test/sros2/commands/security/verbs/test_create_key.py @@ -27,7 +27,7 @@ from ros2cli import cli -from sros2.api import _keystore +from sros2.api import _keystore, _utilities from sros2.policy import get_transport_schema @@ -49,12 +49,6 @@ def enclave_keys_dir(tmpdir_factory): return enclave_dir -def load_cert(path): - with open(path, 'rb') as f: - pem_data = f.read() - return x509.load_pem_x509_certificate(pem_data, default_backend()) - - def load_csr(path): with open(path, 'rb') as f: pem_data = f.read() @@ -101,7 +95,7 @@ def test_create_key(enclave_keys_dir): def test_cert_pem(enclave_keys_dir): - cert = load_cert(enclave_keys_dir / 'cert.pem') + cert = _utilities.load_cert(enclave_keys_dir / 'cert.pem') check_common_name(cert.subject, u'/test_enclave') check_common_name(cert.issuer, _keystore._DEFAULT_COMMON_NAME) @@ -123,7 +117,7 @@ def test_cert_pem(enclave_keys_dir): assert value.path_length is None # Verify this cert is indeed signed by the keystore CA - signatory = load_cert(enclave_keys_dir / 'identity_ca.cert.pem') + signatory = _utilities.load_cert(enclave_keys_dir / 'identity_ca.cert.pem') assert verify_signature(cert, signatory) @@ -138,7 +132,7 @@ def test_governance_p7s(enclave_keys_dir): def test_identity_ca_cert_pem(enclave_keys_dir): - cert = load_cert(enclave_keys_dir / 'identity_ca.cert.pem') + cert = _utilities.load_cert(enclave_keys_dir / 'identity_ca.cert.pem') check_common_name(cert.subject, _keystore._DEFAULT_COMMON_NAME) check_common_name(cert.issuer, _keystore._DEFAULT_COMMON_NAME) @@ -171,9 +165,9 @@ def test_permissions_xml(enclave_keys_dir): def test_permissions_ca_cert_pem(enclave_keys_dir): - cert = load_cert(enclave_keys_dir / 'permissions_ca.cert.pem') + cert = _utilities.load_cert(enclave_keys_dir / 'permissions_ca.cert.pem') check_common_name(cert.subject, _keystore._DEFAULT_COMMON_NAME) check_common_name(cert.issuer, _keystore._DEFAULT_COMMON_NAME) - signatory = load_cert(enclave_keys_dir / 'identity_ca.cert.pem') + signatory = _utilities.load_cert(enclave_keys_dir / 'identity_ca.cert.pem') assert verify_signature(cert, signatory) diff --git a/sros2/test/sros2/commands/security/verbs/test_create_keystore.py b/sros2/test/sros2/commands/security/verbs/test_create_keystore.py index 297232e7..868385f4 100644 --- a/sros2/test/sros2/commands/security/verbs/test_create_keystore.py +++ b/sros2/test/sros2/commands/security/verbs/test_create_keystore.py @@ -22,7 +22,7 @@ import pytest from ros2cli import cli -from sros2.api import _keystore +from sros2.api import _keystore, _utilities # This fixture will run once for the entire module (as opposed to once per test) @@ -65,13 +65,12 @@ def test_create_keystore(keystore_dir): def test_ca_cert(keystore_dir): - with (keystore_dir / 'public' / 'ca.cert.pem').open('rb') as f: - cert = x509.load_pem_x509_certificate(f.read(), cryptography_backend()) - names = cert.subject.get_attributes_for_oid(x509.oid.NameOID.COMMON_NAME) - assert len(names) == 1 - assert names[0].value == _keystore._DEFAULT_COMMON_NAME - names = cert.subject.get_attributes_for_oid(x509.oid.NameOID.ORGANIZATION_NAME) - assert len(names) == 0 + cert = _utilities.load_cert(keystore_dir / 'public' / 'ca.cert.pem') + names = cert.subject.get_attributes_for_oid(x509.oid.NameOID.COMMON_NAME) + assert len(names) == 1 + assert names[0].value == _keystore._DEFAULT_COMMON_NAME + names = cert.subject.get_attributes_for_oid(x509.oid.NameOID.ORGANIZATION_NAME) + assert len(names) == 0 def test_ca_key(keystore_dir): From 04555f008ce5c3f30cd79abca82958fbdf5f5a09 Mon Sep 17 00:00:00 2001 From: Mikael Arguedas Date: Sat, 2 May 2020 11:09:24 +0200 Subject: [PATCH 2/3] use same validity period in permissions and cert Signed-off-by: Mikael Arguedas --- sros2/sros2/api/_permission.py | 6 ++++++ sros2/sros2/policy/templates/dds/permissions.xsl | 6 ++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/sros2/sros2/api/_permission.py b/sros2/sros2/api/_permission.py index 1c45eab7..e1e8beb1 100644 --- a/sros2/sros2/api/_permission.py +++ b/sros2/sros2/api/_permission.py @@ -60,6 +60,12 @@ def create_permission_file(path, domain_id, policy_element): permissions_xsd = etree.XMLSchema(etree.parse(permissions_xsd_path)) kwargs = {} + + cert_path = os.path.join(os.path.dirname(path), 'cert.pem') + cert_content = _utilities.load_cert(cert_path) + kwargs['not_valid_before'] = etree.XSLT.strparam(cert_content.not_valid_before.isoformat()) + kwargs['not_valid_after'] = etree.XSLT.strparam(cert_content.not_valid_after.isoformat()) + if get_rmw_implementation_identifier() in _RMW_WITH_ROS_GRAPH_INFO_TOPIC: kwargs['allow_ros_discovery_topic'] = etree.XSLT.strparam('1') permissions_xml = permissions_xsl(policy_element, **kwargs) diff --git a/sros2/sros2/policy/templates/dds/permissions.xsl b/sros2/sros2/policy/templates/dds/permissions.xsl index a6287bfb..e99f535a 100644 --- a/sros2/sros2/policy/templates/dds/permissions.xsl +++ b/sros2/sros2/policy/templates/dds/permissions.xsl @@ -6,11 +6,13 @@ + + - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + + From 2a1b84a9084425f4ef4a786decd403f21efc99c5 Mon Sep 17 00:00:00 2001 From: Mikael Arguedas Date: Sat, 2 May 2020 11:10:15 +0200 Subject: [PATCH 3/3] update validity_dates in tests to match new defaults Signed-off-by: Mikael Arguedas --- .../permissions/add_two_ints/permissions.xml | 8 +++--- .../minimal_action/permissions.xml | 8 +++--- .../permissions/sample/permissions.xml | 28 +++++++++---------- .../single_context/permissions.xml | 4 +-- .../talker_listener/permissions.xml | 8 +++--- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/sros2/test/policies/permissions/add_two_ints/permissions.xml b/sros2/test/policies/permissions/add_two_ints/permissions.xml index aecfa35c..fc7f7d0d 100644 --- a/sros2/test/policies/permissions/add_two_ints/permissions.xml +++ b/sros2/test/policies/permissions/add_two_ints/permissions.xml @@ -3,8 +3,8 @@ CN=/add_two_ints/add_two_ints_server - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 @@ -54,8 +54,8 @@ CN=/add_two_ints/add_two_ints_client - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 diff --git a/sros2/test/policies/permissions/minimal_action/permissions.xml b/sros2/test/policies/permissions/minimal_action/permissions.xml index ef4c3a1c..2a0b4766 100644 --- a/sros2/test/policies/permissions/minimal_action/permissions.xml +++ b/sros2/test/policies/permissions/minimal_action/permissions.xml @@ -3,8 +3,8 @@ CN=/minimal_action/minimal_action_server - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 @@ -60,8 +60,8 @@ CN=/minimal_action/minimal_action_client - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 diff --git a/sros2/test/policies/permissions/sample/permissions.xml b/sros2/test/policies/permissions/sample/permissions.xml index c079887b..e9cac69d 100644 --- a/sros2/test/policies/permissions/sample/permissions.xml +++ b/sros2/test/policies/permissions/sample/permissions.xml @@ -3,8 +3,8 @@ CN=/talker_listener/talker - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 @@ -53,8 +53,8 @@ CN=/talker_listener/listener - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 @@ -103,8 +103,8 @@ CN=/add_two_ints/add_two_ints_server - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 @@ -154,8 +154,8 @@ CN=/add_two_ints/add_two_ints_client - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 @@ -205,8 +205,8 @@ CN=/minimal_action/minimal_action_server - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 @@ -262,8 +262,8 @@ CN=/minimal_action/minimal_action_client - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 @@ -319,8 +319,8 @@ CN=/sample_policy/admin - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 diff --git a/sros2/test/policies/permissions/single_context/permissions.xml b/sros2/test/policies/permissions/single_context/permissions.xml index f17837bc..180dcc47 100644 --- a/sros2/test/policies/permissions/single_context/permissions.xml +++ b/sros2/test/policies/permissions/single_context/permissions.xml @@ -3,8 +3,8 @@ CN=/single_enclave - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 diff --git a/sros2/test/policies/permissions/talker_listener/permissions.xml b/sros2/test/policies/permissions/talker_listener/permissions.xml index 03bf4876..df53632d 100644 --- a/sros2/test/policies/permissions/talker_listener/permissions.xml +++ b/sros2/test/policies/permissions/talker_listener/permissions.xml @@ -3,8 +3,8 @@ CN=/talker_listener/talker - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00 @@ -53,8 +53,8 @@ CN=/talker_listener/listener - 2013-10-26T00:00:00 - 2023-10-26T22:45:30 + 2020-05-01T00:00:00 + 2030-05-01T00:00:00