Skip to content

Commit

Permalink
use timezone aware datetime, remove -1 day hack in validity
Browse files Browse the repository at this point in the history
Signed-off-by: Mikael Arguedas <[email protected]>
  • Loading branch information
mikaelarguedas committed May 7, 2024
1 parent 2754240 commit 3dd75da
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 7 deletions.
8 changes: 6 additions & 2 deletions sros2/sros2/_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import datetime
import os
import pathlib
from zoneinfo import ZoneInfo

from cryptography import x509
from cryptography.hazmat.backends import default_backend as cryptography_backend
Expand All @@ -28,6 +29,9 @@
_DOMAIN_ID_ENV = 'ROS_DOMAIN_ID'
_KEYSTORE_DIR_ENV = 'ROS_SECURITY_KEYSTORE'

def convert_naive_to_utc(naive_datetime: datetime.datetime) -> datetime.datetime:
return naive_datetime.replace(tzinfo=ZoneInfo('UTC'))


def create_symlink(*, src: pathlib.Path, dst: pathlib.Path):
if dst.exists():
Expand Down Expand Up @@ -80,7 +84,7 @@ def build_key_and_cert(subject_name, *, ca=False, ca_key=None, issuer_name=''):
else:
extension = x509.BasicConstraints(ca=False, path_length=None)

utcnow = datetime.datetime.utcnow()
utcnow = datetime.datetime.now(datetime.UTC)
builder = x509.CertificateBuilder(
).issuer_name(
issuer_name
Expand All @@ -90,7 +94,7 @@ def build_key_and_cert(subject_name, *, ca=False, ca_key=None, issuer_name=''):
# Using a day earlier here to prevent Connext (5.3.1) from complaining
# when extracting it from the permissions file and thinking it's in the future
# https://github.com/ros2/ci/pull/436#issuecomment-624874296
utcnow - datetime.timedelta(days=1)
utcnow
).not_valid_after(
# TODO: This should not be hard-coded
utcnow + datetime.timedelta(days=3650)
Expand Down
6 changes: 4 additions & 2 deletions sros2/sros2/keystore/_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@ def create_permission_file(path: pathlib.Path, domain_id, policy_element) -> Non

cert_path = path.parent.joinpath('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())
# TODO replace not_valid_before/not_valid_after functions by not_valid_before_utc/not_valid_after_utc
# once cryptography 42 is supported on all target platforms
kwargs['not_valid_before'] = etree.XSLT.strparam(_utilities.convert_naive_to_utc(cert_content.not_valid_before).isoformat())
kwargs['not_valid_after'] = etree.XSLT.strparam(_utilities.convert_naive_to_utc(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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,15 @@ def test_cert_pem(enclave_keys_dir):
assert isinstance(cert.signature_hash_algorithm, hashes.SHA256)

# Verify the cert is valid for the expected timespan
utcnow = datetime.datetime.utcnow()
utcnow = datetime.datetime.now(datetime.UTC)

# Using a day earlier here to prevent Connext (5.3.1) from complaining
# when extracting it from the permissions file and thinking it's in the future
# https://github.com/ros2/ci/pull/436#issuecomment-624874296
assert _datetimes_are_close(cert.not_valid_before, utcnow - datetime.timedelta(days=1))
assert _datetimes_are_close(cert.not_valid_after, utcnow + datetime.timedelta(days=3650))
# TODO replace not_valid_before/not_valid_after functions by not_valid_before_utc/not_valid_after_utc
# once cryptography 42 is supported on all target platforms
assert _datetimes_are_close(_utilities.convert_naive_to_utc(cert.not_valid_before), utcnow)
assert _datetimes_are_close(_utilities.convert_naive_to_utc(cert.not_valid_after), utcnow + datetime.timedelta(days=3650))

# Verify that the cert ensures this key cannot be used to sign others as a CA
assert len(cert.extensions) == 1
Expand Down

0 comments on commit 3dd75da

Please sign in to comment.