From 37e50b000f0900f55510d34075d756d21cb2bf9b Mon Sep 17 00:00:00 2001 From: jeanluc Date: Wed, 10 Jan 2024 20:26:49 +0100 Subject: [PATCH] Migrate to TZ-aware datetime objects in x509_v2 --- changelog/65837.fixed.md | 1 + salt/modules/x509_v2.py | 42 +++++++++++++++++++-------- salt/states/x509_v2.py | 25 +++++++++++----- salt/utils/x509.py | 35 +++++++++++++--------- tests/pytests/unit/utils/test_x509.py | 10 +++---- 5 files changed, 73 insertions(+), 40 deletions(-) create mode 100644 changelog/65837.fixed.md diff --git a/changelog/65837.fixed.md b/changelog/65837.fixed.md new file mode 100644 index 00000000000..72f4a30fbda --- /dev/null +++ b/changelog/65837.fixed.md @@ -0,0 +1 @@ +Corrected x509_v2 CRL creation `last_update` and `next_update` values when system timezone is not UTC diff --git a/salt/modules/x509_v2.py b/salt/modules/x509_v2.py index 72e8f466bb7..9b5617f5048 100644 --- a/salt/modules/x509_v2.py +++ b/salt/modules/x509_v2.py @@ -135,12 +135,12 @@ Note that when a ``ca_server`` is involved, both peers must use the updated modu import base64 import copy -import datetime import glob import logging import os.path import re import sys +from datetime import datetime, timedelta, timezone try: import cryptography.x509 as cx509 @@ -1376,10 +1376,12 @@ def expires(certificate, days=0): Defaults to ``0``, which checks for the current time. """ cert = x509util.load_cert(certificate) - # dates are encoded in UTC/GMT, they are returned as a naive datetime object - return cert.not_valid_after <= datetime.datetime.utcnow() + datetime.timedelta( - days=days - ) + try: + not_after = cert.not_valid_after_utc + except AttributeError: + # naive datetime object, release <42 (it's always UTC) + not_after = cert.not_valid_after.replace(tzinfo=timezone.utc) + return not_after <= datetime.now(tz=timezone.utc) + timedelta(days=days) def expired(certificate): @@ -1659,6 +1661,13 @@ def read_certificate(certificate): cert = x509util.load_cert(certificate) key_type = x509util.get_key_type(cert.public_key(), as_string=True) + try: + not_before = cert.not_valid_before_utc + not_after = cert.not_valid_after_utc + except AttributeError: + # naive datetime object, release <42 (it's always UTC) + not_before = cert.not_valid_before.replace(tzinfo=timezone.utc) + not_after = cert.not_valid_after.replace(tzinfo=timezone.utc) ret = { "version": cert.version.value + 1, # 0-indexed "key_size": cert.public_key().key_size if key_type in ["ec", "rsa"] else None, @@ -1674,8 +1683,8 @@ def read_certificate(certificate): "issuer": _parse_dn(cert.issuer), "issuer_hash": x509util.pretty_hex(_get_name_hash(cert.issuer)), "issuer_str": cert.issuer.rfc4514_string(), - "not_before": cert.not_valid_before.strftime(x509util.TIME_FMT), - "not_after": cert.not_valid_after.strftime(x509util.TIME_FMT), + "not_before": not_before.strftime(x509util.TIME_FMT), + "not_after": not_after.strftime(x509util.TIME_FMT), "public_key": get_public_key(cert), "extensions": _parse_extensions(cert.extensions), } @@ -1741,10 +1750,16 @@ def read_crl(crl): The certificate revocation list to read. """ crl = x509util.load_crl(crl) + try: + last_update = crl.last_update_utc + next_update = crl.next_update_utc + except AttributeError: + last_update = crl.last_update.replace(tzinfo=timezone.utc) + next_update = crl.next_update.replace(tzinfo=timezone.utc) ret = { "issuer": _parse_dn(crl.issuer), - "last_update": crl.last_update.strftime(x509util.TIME_FMT), - "next_update": crl.next_update.strftime(x509util.TIME_FMT), + "last_update": last_update.strftime(x509util.TIME_FMT), + "next_update": next_update.strftime(x509util.TIME_FMT), "revoked_certificates": {}, "extensions": _parse_extensions(crl.extensions), } @@ -1764,12 +1779,15 @@ def read_crl(crl): ret["signature_algorithm"] = crl.signature_algorithm_oid.dotted_string for revoked in crl: + try: + revocation_date = revoked.revocation_date_utc + except AttributeError: + # naive datetime object, release <42 (it's always UTC) + revocation_date = revoked.revocation_date.replace(tzinfo=timezone.utc) ret["revoked_certificates"].update( { x509util.dec2hex(revoked.serial_number).replace(":", ""): { - "revocation_date": revoked.revocation_date.strftime( - x509util.TIME_FMT - ), + "revocation_date": revocation_date.strftime(x509util.TIME_FMT), "extensions": _parse_crl_entry_extensions(revoked.extensions), } } diff --git a/salt/states/x509_v2.py b/salt/states/x509_v2.py index 7047b3612c9..af1cb05e75b 100644 --- a/salt/states/x509_v2.py +++ b/salt/states/x509_v2.py @@ -183,9 +183,9 @@ according to the www policy. import base64 import copy -import datetime import logging import os.path +from datetime import datetime, timedelta, timezone import salt.utils.files from salt.exceptions import CommandExecutionError, SaltInvocationError @@ -487,11 +487,16 @@ def certificate_managed( else None ): changes["pkcs12_friendlyname"] = pkcs12_friendlyname + try: + curr_not_after = current.not_valid_after_utc + except AttributeError: + # naive datetime object, release <42 (it's always UTC) + curr_not_after = current.not_valid_after.replace( + tzinfo=timezone.utc + ) - if ( - current.not_valid_after - < datetime.datetime.utcnow() - + datetime.timedelta(days=days_remaining) + if curr_not_after < datetime.now(tz=timezone.utc) + timedelta( + days=days_remaining ): changes["expiration"] = True @@ -896,10 +901,14 @@ def crl_managed( if encoding != current_encoding: changes["encoding"] = encoding + try: + curr_next_update = current.next_update_utc + except AttributeError: + # naive datetime object, release <42 (it's always UTC) + curr_next_update = current.next_update.replace(tzinfo=timezone.utc) if days_remaining and ( - current.next_update - < datetime.datetime.utcnow() - + datetime.timedelta(days=days_remaining) + curr_next_update + < datetime.now(tz=timezone.utc) + timedelta(days=days_remaining) ): changes["expiration"] = True diff --git a/salt/utils/x509.py b/salt/utils/x509.py index ad8bb30fb04..2d5c26fcbeb 100644 --- a/salt/utils/x509.py +++ b/salt/utils/x509.py @@ -1,10 +1,10 @@ import base64 import copy -import datetime import ipaddress import logging import os.path import re +from datetime import datetime, timedelta, timezone from enum import Enum from urllib.parse import urlparse, urlunparse @@ -313,14 +313,14 @@ def build_crt( ) not_before = ( - datetime.datetime.strptime(not_before, TIME_FMT) + datetime.strptime(not_before, TIME_FMT).replace(tzinfo=timezone.utc) if not_before - else datetime.datetime.utcnow() + else datetime.now(tz=timezone.utc) ) not_after = ( - datetime.datetime.strptime(not_after, TIME_FMT) + datetime.strptime(not_after, TIME_FMT).replace(tzinfo=timezone.utc) if not_after - else datetime.datetime.utcnow() + datetime.timedelta(days=days_valid) + else datetime.now(tz=timezone.utc) + timedelta(days=days_valid) ) builder = builder.not_valid_before(not_before).not_valid_after(not_after) @@ -422,32 +422,38 @@ def build_crl( builder = cx509.CertificateRevocationListBuilder() if signing_cert: builder = builder.issuer_name(signing_cert.subject) - builder = builder.last_update(datetime.datetime.today()) + builder = builder.last_update(datetime.now(tz=timezone.utc)) builder = builder.next_update( - datetime.datetime.today() + datetime.timedelta(days=days_valid) + datetime.now(tz=timezone.utc) + timedelta(days=days_valid) ) for rev in revoked: serial_number = not_after = revocation_date = None if "not_after" in rev: - not_after = datetime.datetime.strptime(rev["not_after"], TIME_FMT) + not_after = datetime.strptime(rev["not_after"], TIME_FMT).replace( + tzinfo=timezone.utc + ) if "serial_number" in rev: serial_number = rev["serial_number"] if "certificate" in rev: rev_cert = load_cert(rev["certificate"]) serial_number = rev_cert.serial_number - not_after = rev_cert.not_valid_after + try: + not_after = rev_cert.not_valid_after_utc + except AttributeError: + # naive datetime object, release <42 (it's always UTC) + not_after = rev_cert.not_valid_after.replace(tzinfo=timezone.utc) if not serial_number: raise SaltInvocationError("Need serial_number or certificate") serial_number = _get_serial_number(serial_number) if not_after and not include_expired: - if datetime.datetime.utcnow() > not_after: + if datetime.now(tz=timezone.utc) > not_after: continue if "revocation_date" in rev: - revocation_date = datetime.datetime.strptime( + revocation_date = datetime.strptime( rev["revocation_date"], TIME_FMT - ) + ).replace(tzinfo=timezone.utc) else: - revocation_date = datetime.datetime.utcnow() + revocation_date = datetime.now(tz=timezone.utc) revoked_cert = cx509.RevokedCertificateBuilder( serial_number=serial_number, revocation_date=revocation_date @@ -1624,8 +1630,9 @@ def _create_invalidity_date(val, **kwargs): if critical: val = val.split(" ", maxsplit=1)[1] try: + # InvalidityDate deals in naive datetime objects only currently return ( - cx509.InvalidityDate(datetime.datetime.strptime(val, TIME_FMT)), + cx509.InvalidityDate(datetime.strptime(val, TIME_FMT)), critical, ) except ValueError as err: diff --git a/tests/pytests/unit/utils/test_x509.py b/tests/pytests/unit/utils/test_x509.py index d91c1d61ab6..38c1ba9de25 100644 --- a/tests/pytests/unit/utils/test_x509.py +++ b/tests/pytests/unit/utils/test_x509.py @@ -1,5 +1,5 @@ -import datetime import ipaddress +from datetime import datetime import pytest @@ -1019,12 +1019,12 @@ class TestCreateExtension: [ ( "critical, 2022-10-11 13:37:42", - datetime.datetime.strptime("2022-10-11 13:37:42", "%Y-%m-%d %H:%M:%S"), + datetime.strptime("2022-10-11 13:37:42", "%Y-%m-%d %H:%M:%S"), True, ), ( "2022-10-11 13:37:42", - datetime.datetime.strptime("2022-10-11 13:37:42", "%Y-%m-%d %H:%M:%S"), + datetime.strptime("2022-10-11 13:37:42", "%Y-%m-%d %H:%M:%S"), False, ), ], @@ -1875,9 +1875,7 @@ def test_get_dn(inpt, expected): cx509.Extension( cx509.InvalidityDate.oid, value=cx509.InvalidityDate( - datetime.datetime.strptime( - "2022-10-11 13:37:42", "%Y-%m-%d %H:%M:%S" - ) + datetime.strptime("2022-10-11 13:37:42", "%Y-%m-%d %H:%M:%S") ), critical=False, ),