Use matching validity dates for cert and permissions (#205)

* make load_cert a utility and reuse it everywhere Signed-off-by: Mikael Arguedas <[email protected]> * use same validity period in permissions and cert Signed-off-by: Mikael Arguedas <[email protected]> * update validity_dates in tests to match new defaults Signed-off-by: Mikael Arguedas <[email protected]>
ros2 · May 4, 2020 · 498d9db · 498d9db
1 parent 2eb0f71
commit 498d9db
Show file tree

Hide file tree

Showing 11 changed files with 59 additions and 55 deletions.
diff --git 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/_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/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/sros2/policy/templates/dds/permissions.xsl b/sros2/sros2/policy/templates/dds/permissions.xsl
@@ -6,11 +6,13 @@
  <xsl:output omit-xml-declaration="yes" indent="yes"/>
  <xsl:strip-space elements="*"/>
 
+<xsl:param name="not_valid_before" select="'2020-05-01T00:00:00'"/>
+<xsl:param name="not_valid_after" select="'2030-05-01T00:00:00'"/>
 
 <xsl:variable name="template_validity">
   <validity>
-    <not_before>2013-10-26T00:00:00</not_before>
-    <not_after>2023-10-26T22:45:30</not_after>
+    <not_before><xsl:value-of select="$not_valid_before" /></not_before>
+    <not_after><xsl:value-of select="$not_valid_after" /></not_after>
   </validity>
 </xsl:variable>
 

diff --git a/sros2/test/policies/permissions/add_two_ints/permissions.xml b/sros2/test/policies/permissions/add_two_ints/permissions.xml
@@ -3,8 +3,8 @@
     <grant name="/add_two_ints/add_two_ints_server">
       <subject_name>CN=/add_two_ints/add_two_ints_server</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>
@@ -54,8 +54,8 @@
     <grant name="/add_two_ints/add_two_ints_client">
       <subject_name>CN=/add_two_ints/add_two_ints_client</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>

diff --git a/sros2/test/policies/permissions/minimal_action/permissions.xml b/sros2/test/policies/permissions/minimal_action/permissions.xml
@@ -3,8 +3,8 @@
     <grant name="/minimal_action/minimal_action_server">
       <subject_name>CN=/minimal_action/minimal_action_server</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>
@@ -60,8 +60,8 @@
     <grant name="/minimal_action/minimal_action_client">
       <subject_name>CN=/minimal_action/minimal_action_client</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>

diff --git a/sros2/test/policies/permissions/sample/permissions.xml b/sros2/test/policies/permissions/sample/permissions.xml
@@ -3,8 +3,8 @@
     <grant name="/talker_listener/talker">
       <subject_name>CN=/talker_listener/talker</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>
@@ -53,8 +53,8 @@
     <grant name="/talker_listener/listener">
       <subject_name>CN=/talker_listener/listener</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>
@@ -103,8 +103,8 @@
     <grant name="/add_two_ints/add_two_ints_server">
       <subject_name>CN=/add_two_ints/add_two_ints_server</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>
@@ -154,8 +154,8 @@
     <grant name="/add_two_ints/add_two_ints_client">
       <subject_name>CN=/add_two_ints/add_two_ints_client</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>
@@ -205,8 +205,8 @@
     <grant name="/minimal_action/minimal_action_server">
       <subject_name>CN=/minimal_action/minimal_action_server</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>
@@ -262,8 +262,8 @@
     <grant name="/minimal_action/minimal_action_client">
       <subject_name>CN=/minimal_action/minimal_action_client</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>
@@ -319,8 +319,8 @@
     <grant name="/sample_policy/admin">
       <subject_name>CN=/sample_policy/admin</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>

diff --git a/sros2/test/policies/permissions/single_context/permissions.xml b/sros2/test/policies/permissions/single_context/permissions.xml
@@ -3,8 +3,8 @@
     <grant name="/single_enclave">
       <subject_name>CN=/single_enclave</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>

diff --git a/sros2/test/policies/permissions/talker_listener/permissions.xml b/sros2/test/policies/permissions/talker_listener/permissions.xml
@@ -3,8 +3,8 @@
     <grant name="/talker_listener/talker">
       <subject_name>CN=/talker_listener/talker</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>
@@ -53,8 +53,8 @@
     <grant name="/talker_listener/listener">
       <subject_name>CN=/talker_listener/listener</subject_name>
       <validity>
-        <not_before>2013-10-26T00:00:00</not_before>
-        <not_after>2023-10-26T22:45:30</not_after>
+        <not_before>2020-05-01T00:00:00</not_before>
+        <not_after>2030-05-01T00:00:00</not_after>
       </validity>
       <allow_rule>
         <domains>

diff --git 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
@@ -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):