From fcfc555317058f0dae1f5fc73861c79c5415e9f9 Mon Sep 17 00:00:00 2001 From: Noctua Date: Thu, 2 May 2024 22:01:10 +0200 Subject: [PATCH] chore: update charm libraries (#598) --- .../observability_libs/v1/cert_handler.py | 426 ++++++++++++------ .../v3/tls_certificates.py | 165 +++++-- 2 files changed, 417 insertions(+), 174 deletions(-) diff --git a/lib/charms/observability_libs/v1/cert_handler.py b/lib/charms/observability_libs/v1/cert_handler.py index 79458e09..ab369353 100644 --- a/lib/charms/observability_libs/v1/cert_handler.py +++ b/lib/charms/observability_libs/v1/cert_handler.py @@ -31,10 +31,12 @@ Since this library uses [Juju Secrets](https://juju.is/docs/juju/secret) it requires Juju >= 3.0.3. """ +import abc import ipaddress +import json import socket from itertools import filterfalse -from typing import List, Optional, Union +from typing import Dict, List, Optional, Union try: from charms.tls_certificates_interface.v3.tls_certificates import ( # type: ignore @@ -42,13 +44,14 @@ CertificateAvailableEvent, CertificateExpiringEvent, CertificateInvalidatedEvent, + ProviderCertificate, TLSCertificatesRequiresV3, generate_csr, generate_private_key, ) except ImportError as e: raise ImportError( - "failed to import charms.tls_certificates_interface.v2.tls_certificates; " + "failed to import charms.tls_certificates_interface.v3.tls_certificates; " "Either the library itself is missing (please get it through charmcraft fetch-lib) " "or one of its dependencies is unmet." ) from e @@ -58,14 +61,15 @@ from ops.charm import CharmBase, RelationBrokenEvent from ops.framework import EventBase, EventSource, Object, ObjectEvents from ops.jujuversion import JujuVersion -from ops.model import SecretNotFoundError +from ops.model import Relation, Secret, SecretNotFoundError logger = logging.getLogger(__name__) - LIBID = "b5cd5cd580f3428fa5f59a8876dcbe6a" LIBAPI = 1 -LIBPATCH = 5 +LIBPATCH = 7 + +VAULT_SECRET_LABEL = "cert-handler-private-vault" def is_ip_address(value: str) -> bool: @@ -87,6 +91,178 @@ class CertHandlerEvents(ObjectEvents): cert_changed = EventSource(CertChanged) +class _VaultBackend(abc.ABC): + """Base class for a single secret manager. + + Assumptions: + - A single secret (label) is managed by a single instance. + - Secret is per-unit (not per-app, i.e. may differ from unit to unit). + """ + + def store(self, contents: Dict[str, str], clear: bool = False): ... + + def get_value(self, key: str) -> Optional[str]: ... + + def retrieve(self) -> Dict[str, str]: ... + + def clear(self): ... + + +class _RelationVaultBackend(_VaultBackend): + """Relation backend for Vault. + + Use it to store data in a relation databag. + Assumes that a single relation exists and its data is readable. + If not, it will raise RuntimeErrors as soon as you try to read/write. + It will store the data, in plaintext (json-dumped) nested under a configurable + key in the **unit databag** of this relation. + + Typically, you'll use this with peer relations. + + Note: it is assumed that this object has exclusive access to the data, even though in practice it does not. + Modifying relation data yourself would go unnoticed and disrupt consistency. + """ + + _NEST_UNDER = "lib.charms.observability_libs.v1.cert_handler::vault" + # This key needs to be relation-unique. If someone ever creates multiple Vault(_RelationVaultBackend) + # instances backed by the same (peer) relation, they'll need to set different _NEST_UNDERs + # for each _RelationVaultBackend instance or they'll be fighting over it. + + def __init__(self, charm: CharmBase, relation_name: str): + self.charm = charm + self.relation_name = relation_name + + def _check_ready(self): + relation = self.charm.model.get_relation(self.relation_name) + if not relation or not relation.data: + # if something goes wrong here, the peer-backed vault is not ready to operate + # it can be because you are trying to use it too soon, i.e. before the (peer) + # relation has been created (or has joined). + raise RuntimeError("Relation backend not ready.") + + @property + def _relation(self) -> Optional[Relation]: + self._check_ready() + return self.charm.model.get_relation(self.relation_name) + + @property + def _databag(self): + self._check_ready() + # _check_ready verifies that there is a relation + return self._relation.data[self.charm.unit] # type: ignore + + def _read(self) -> Dict[str, str]: + value = self._databag.get(self._NEST_UNDER) + if value: + return json.loads(value) + return {} + + def _write(self, value: Dict[str, str]): + if not all(isinstance(x, str) for x in value.values()): + # the caller has to take care of encoding + raise TypeError("You can only store strings in Vault.") + + self._databag[self._NEST_UNDER] = json.dumps(value) + + def store(self, contents: Dict[str, str], clear: bool = False): + """Create a new revision by updating the previous one with ``contents``.""" + current = self._read() + + if clear: + current.clear() + + current.update(contents) + self._write(current) + + def get_value(self, key: str) -> Optional[str]: + """Like retrieve, but single-value.""" + return self._read().get(key) + + def retrieve(self): + """Return the full vault content.""" + return self._read() + + def clear(self): + del self._databag[self._NEST_UNDER] + + +class _SecretVaultBackend(_VaultBackend): + """Relation backend for Vault. + + Use it to store data in a Juju secret. + Assumes that Juju supports secrets. + If not, it will raise some exception as soon as you try to read/write. + + Note: it is assumed that this object has exclusive access to the data, even though in practice it does not. + Modifying secret's data yourself would go unnoticed and disrupt consistency. + """ + + _uninitialized_key = "uninitialized-secret-key" + + def __init__(self, charm: CharmBase, secret_label: str): + self.charm = charm + self.secret_label = secret_label # needs to be charm-unique. + + @property + def _secret(self) -> Secret: + try: + # we are owners, so we don't need to grant it to ourselves + return self.charm.model.get_secret(label=self.secret_label) + except SecretNotFoundError: + # we need to set SOME contents when we're creating the secret, so we do it. + return self.charm.unit.add_secret( + {self._uninitialized_key: "42"}, label=self.secret_label + ) + + def store(self, contents: Dict[str, str], clear: bool = False): + """Create a new revision by updating the previous one with ``contents``.""" + secret = self._secret + current = secret.get_content(refresh=True) + + if clear: + current.clear() + elif current.get(self._uninitialized_key): + # is this the first revision? clean up the mock contents we created instants ago. + del current[self._uninitialized_key] + + current.update(contents) + secret.set_content(current) + + def get_value(self, key: str) -> Optional[str]: + """Like retrieve, but single-value.""" + return self._secret.get_content(refresh=True).get(key) + + def retrieve(self): + """Return the full vault content.""" + return self._secret.get_content(refresh=True) + + def clear(self): + self._secret.remove_all_revisions() + + +class Vault: + """Simple application secret wrapper for local usage.""" + + def __init__(self, backend: _VaultBackend): + self._backend = backend + + def store(self, contents: Dict[str, str], clear: bool = False): + """Store these contents in the vault overriding whatever is there.""" + self._backend.store(contents, clear=clear) + + def get_value(self, key: str): + """Like retrieve, but single-value.""" + return self._backend.get_value(key) + + def retrieve(self) -> Dict[str, str]: + """Return the full vault content.""" + return self._backend.retrieve() + + def clear(self): + """Clear the vault.""" + self._backend.clear() + + class CertHandler(Object): """A wrapper for the requirer side of the TLS Certificates charm library.""" @@ -114,9 +290,8 @@ def __init__( sans: DNS names. If none are given, use FQDN. """ super().__init__(charm, key) - self._check_juju_supports_secrets() - self.charm = charm + # We need to sanitize the unit name, otherwise route53 complains: # "urn:ietf:params:acme:error:malformed" :: Domain name contains an invalid character self.cert_subject = charm.unit.name.replace("/", "-") if not cert_subject else cert_subject @@ -126,6 +301,17 @@ def __init__( self.sans_ip = list(filter(is_ip_address, sans)) self.sans_dns = list(filterfalse(is_ip_address, sans)) + if self._check_juju_supports_secrets(): + vault_backend = _SecretVaultBackend(charm, secret_label=VAULT_SECRET_LABEL) + + # TODO: gracefully handle situations where the + # secret is gone because the admin has removed it manually + # self.framework.observe(self.charm.on.secret_remove, self._rotate_csr) + + else: + vault_backend = _RelationVaultBackend(charm, relation_name="peers") + self.vault = Vault(vault_backend) + self.certificates_relation_name = certificates_relation_name self.certificates = TLSCertificatesRequiresV3(self.charm, self.certificates_relation_name) @@ -157,6 +343,34 @@ def __init__( self.charm.on[self.certificates_relation_name].relation_broken, # pyright: ignore self._on_certificates_relation_broken, ) + self.framework.observe( + self.charm.on.upgrade_charm, # pyright: ignore + self._on_upgrade_charm, + ) + + def _on_upgrade_charm(self, _): + self._migrate_vault() + + def _migrate_vault(self): + peer_backend = _RelationVaultBackend(self.charm, relation_name="peers") + + if self._check_juju_supports_secrets(): + # we are on recent juju + if self.vault.retrieve(): + # we already were on recent juju: nothing to migrate + return + + # we used to be on old juju: our secret stuff is in peer data + if peer_backend.retrieve(): + # move over to secret-backed storage + self.vault.store(peer_backend.retrieve()) + + # clear the peer storage + peer_backend.clear() + return + + # if we are downgrading, i.e. from juju with secrets to juju without, + # we have lost all that was in the secrets backend. @property def enabled(self) -> bool: @@ -185,30 +399,17 @@ def enabled(self) -> bool: return True def _on_certificates_relation_joined(self, _) -> None: - self._generate_privkey() + # this will only generate a csr if we don't have one already self._generate_csr() - def _generate_privkey(self): - # Generate priv key unless done already - # TODO figure out how to go about key rotation. - - if not (relation := self.charm.model.get_relation(self.certificates_relation_name)): - return - - if not self.private_key: - private_key = generate_private_key() - secret = self.charm.unit.add_secret({"private-key": private_key.decode()}) - secret.grant(relation) - relation.data[self.charm.unit]["private-key-secret-id"] = secret.id # pyright: ignore - def _on_config_changed(self, _): - relation = self.charm.model.get_relation(self.certificates_relation_name) - - if not relation: - return + # this will only generate a csr if we don't have one already + self._generate_csr() - self._generate_privkey() - self._generate_csr(renew=True) + @property + def relation(self): + """The "certificates" relation.""" + return self.charm.model.get_relation(self.certificates_relation_name) def _generate_csr( self, overwrite: bool = False, renew: bool = False, clear_cert: bool = False @@ -222,7 +423,7 @@ def _generate_csr( This method intentionally does not emit any events, leave it for caller's responsibility. """ # if we are in a relation-broken hook, we might not have a relation to publish the csr to. - if not self.charm.model.get_relation(self.certificates_relation_name): + if not self.relation: logger.warning( f"No {self.certificates_relation_name!r} relation found. " f"Cannot generate csr." ) @@ -231,12 +432,6 @@ def _generate_csr( # In case we already have a csr, do not overwrite it by default. if overwrite or renew or not self._csr: private_key = self.private_key - if private_key is None: - # FIXME: raise this in a less nested scope by - # generating privkey and csr in the same method. - raise RuntimeError( - "private key unset. call _generate_privkey() before you call this method." - ) csr = generate_csr( private_key=private_key.encode(), subject=self.cert_subject, @@ -258,119 +453,73 @@ def _generate_csr( ) self.certificates.request_certificate_creation(certificate_signing_request=csr) - # Note: CSR is being replaced with a new one, so until we get the new cert, we'd have - # a mismatch between the CSR and the cert. - # For some reason the csr contains a trailing '\n'. TODO figure out why - self._csr = csr.decode().strip() - if clear_cert: - try: - secret = self.model.get_secret(label="ca-certificate-chain") - secret.remove_all_revisions() - except SecretNotFoundError: - logger.debug("Secret with label: 'ca-certificate-chain' not found") + self.vault.clear() def _on_certificate_available(self, event: CertificateAvailableEvent) -> None: - """Get the certificate from the event and store it in a peer relation. - - Note: assuming "limit: 1" in metadata - """ - event_csr = ( - event.certificate_signing_request.strip() - if event.certificate_signing_request - else None - ) - if event_csr == self._csr: - content = { - "ca-cert": event.ca, - "server-cert": event.certificate, - "chain": event.chain_as_pem(), - "csr": event_csr, - } - try: - secret = self.model.get_secret(label="ca-certificate-chain") - except SecretNotFoundError: - if not ( - relation := self.charm.model.get_relation(self.certificates_relation_name) - ): - logger.error("Relation %s not found", self.certificates_relation_name) - return - - secret = self.charm.unit.add_secret(content, label="ca-certificate-chain") - secret.grant(relation) - relation.data[self.charm.unit]["secret-id"] = secret.id # pyright: ignore - self.on.cert_changed.emit() # pyright: ignore - - def _retrieve_secret_id(self, secret_id_name: str) -> Optional[str]: - if not (relation := self.charm.model.get_relation(self.certificates_relation_name)): - return None - - if not (secret_id := relation.data[self.charm.unit].get(secret_id_name)): - return None - - return secret_id - - def _retrieve_from_secret(self, value: str, secret_id_name: str) -> Optional[str]: - if not (secret_id := self._retrieve_secret_id(secret_id_name)): - return None - - if not (secret := self.model.get_secret(id=secret_id)): - return None - - content = secret.get_content() - return content.get(value) + """Emit cert-changed.""" + self.on.cert_changed.emit() # pyright: ignore @property - def private_key(self) -> Optional[str]: - """Private key.""" - return self._retrieve_from_secret("private-key", "private-key-secret-id") + def private_key(self) -> str: + """Private key. - @property - def private_key_secret_id(self) -> Optional[str]: - """ID of the Juju Secret for the Private key.""" - return self._retrieve_secret_id("private-key-secret-id") + BEWARE: if the vault misbehaves, the backing secret is removed, the peer relation dies + or whatever, we might be calling generate_private_key() again and cause a desync + with the CSR because it's going to be signed with an outdated key we have no way of retrieving. + The caller needs to ensure that if the vault backend gets reset, then so does the csr. + + TODO: we could consider adding a way to verify if the csr was signed by our privkey, + and do that on collect_unit_status as a consistency check + """ + private_key = self.vault.get_value("private-key") + if private_key is None: + private_key = generate_private_key().decode() + self.vault.store({"private-key": private_key}) + return private_key @property def _csr(self) -> Optional[str]: - return self._retrieve_from_secret("csr", "csr-secret-id") + csrs = self.certificates.get_requirer_csrs() + if not csrs: + return None - @_csr.setter - def _csr(self, value: str): - if not (relation := self.charm.model.get_relation(self.certificates_relation_name)): - return + # in principle we only ever need one cert. + # we might want to complicate this a bit once we get into cert rotations: during the rotation, we may need to + # keep the old one around for a little while. If there's multiple certs, at the moment we're + # ignoring all but the last one. + if len(csrs) > 1: + logger.warning( + "Multiple CSRs found in `certificates` relation. " + "cert_handler is not ready to expect it." + ) - if not (secret_id := relation.data[self.charm.unit].get("csr-secret-id", None)): - secret = self.charm.unit.add_secret({"csr": value}) - secret.grant(relation) - relation.data[self.charm.unit]["csr-secret-id"] = secret.id # pyright: ignore - return + return csrs[-1].csr - secret = self.model.get_secret(id=secret_id) - secret.set_content({"csr": value}) + def get_cert(self) -> Optional[ProviderCertificate]: + """Get the certificate from relation data.""" + all_certs = self.certificates.get_provider_certificates() + # search for the cert matching our csr. + matching_cert = [c for c in all_certs if c.csr == self._csr] + return matching_cert[0] if matching_cert else None @property def ca_cert(self) -> Optional[str]: """CA Certificate.""" - return self._retrieve_from_secret("ca-cert", "secret-id") - - @property - def ca_server_cert_secret_id(self) -> Optional[str]: - """CA server cert secret id.""" - return self._retrieve_secret_id("secret-id") + cert = self.get_cert() + return cert.ca if cert else None @property def server_cert(self) -> Optional[str]: """Server Certificate.""" - return self._retrieve_from_secret("server-cert", "secret-id") - - @property - def _chain(self) -> Optional[str]: - return self._retrieve_from_secret("chain", "secret-id") + cert = self.get_cert() + return cert.certificate if cert else None @property def chain(self) -> Optional[str]: - """Return the ca chain.""" - return self._chain + """Return the ca chain bundled as a single PEM string.""" + cert = self.get_cert() + return cert.chain_as_pem() if cert else None def _on_certificate_expiring( self, event: Union[CertificateExpiringEvent, CertificateInvalidatedEvent] @@ -378,6 +527,7 @@ def _on_certificate_expiring( """Generate a new CSR and request certificate renewal.""" if event.certificate == self.server_cert: self._generate_csr(renew=True) + # FIXME why are we not emitting cert_changed here? def _certificate_revoked(self, event) -> None: """Remove the certificate and generate a new CSR.""" @@ -388,13 +538,11 @@ def _certificate_revoked(self, event) -> None: def _on_certificate_invalidated(self, event: CertificateInvalidatedEvent) -> None: """Deal with certificate revocation and expiration.""" - if event.certificate != self.server_cert: - return - - # if event.reason in ("revoked", "expired"): - # Currently, the reason does not matter to us because the action is the same. - self._generate_csr(overwrite=True, clear_cert=True) - self.on.cert_changed.emit() # pyright: ignore + if event.certificate == self.server_cert: + # if event.reason in ("revoked", "expired"): + # Currently, the reason does not matter to us because the action is the same. + self._generate_csr(overwrite=True, clear_cert=True) + self.on.cert_changed.emit() # pyright: ignore def _on_all_certificates_invalidated(self, _: AllCertificatesInvalidatedEvent) -> None: # Do what you want with this information, probably remove all certificates @@ -403,18 +551,14 @@ def _on_all_certificates_invalidated(self, _: AllCertificatesInvalidatedEvent) - self.on.cert_changed.emit() # pyright: ignore def _on_certificates_relation_broken(self, _: RelationBrokenEvent) -> None: - """Clear the certificates data when removing the relation.""" - try: - secret = self.model.get_secret(label="csr-secret-id") - secret.remove_all_revisions() - except SecretNotFoundError: - logger.debug("Secret 'csr-scret-id' not found") + """Clear all secrets data when removing the relation.""" + self.vault.clear() self.on.cert_changed.emit() # pyright: ignore - def _check_juju_supports_secrets(self) -> None: + def _check_juju_supports_secrets(self) -> bool: version = JujuVersion.from_environ() - if not JujuVersion(version=str(version)).has_secrets: msg = f"Juju version {version} does not supports Secrets. Juju >= 3.0.3 is needed" - logger.error(msg) - raise RuntimeError(msg) + logger.debug(msg) + return False + return True diff --git a/lib/charms/tls_certificates_interface/v3/tls_certificates.py b/lib/charms/tls_certificates_interface/v3/tls_certificates.py index cbdd80d1..6fa26397 100644 --- a/lib/charms/tls_certificates_interface/v3/tls_certificates.py +++ b/lib/charms/tls_certificates_interface/v3/tls_certificates.py @@ -111,6 +111,7 @@ def _on_certificate_request(self, event: CertificateCreationRequestEvent) -> Non ca=ca_certificate, chain=[ca_certificate, certificate], relation_id=event.relation_id, + recommended_expiry_notification_time=720, ) def _on_certificate_revocation_request(self, event: CertificateRevocationRequestEvent) -> None: @@ -316,7 +317,7 @@ def _on_all_certificates_invalidated(self, event: AllCertificatesInvalidatedEven # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 10 +LIBPATCH = 13 PYDEPS = ["cryptography", "jsonschema"] @@ -453,11 +454,35 @@ class ProviderCertificate: ca: str chain: List[str] revoked: bool + expiry_time: datetime + expiry_notification_time: Optional[datetime] = None def chain_as_pem(self) -> str: """Return full certificate chain as a PEM string.""" return "\n\n".join(reversed(self.chain)) + def to_json(self) -> str: + """Return the object as a JSON string. + + Returns: + str: JSON representation of the object + """ + return json.dumps( + { + "relation_id": self.relation_id, + "application_name": self.application_name, + "csr": self.csr, + "certificate": self.certificate, + "ca": self.ca, + "chain": self.chain, + "revoked": self.revoked, + "expiry_time": self.expiry_time.isoformat(), + "expiry_notification_time": self.expiry_notification_time.isoformat() + if self.expiry_notification_time + else None, + } + ) + class CertificateAvailableEvent(EventBase): """Charm Event triggered when a TLS certificate is available.""" @@ -682,21 +707,49 @@ def _get_closest_future_time( ) -def _get_certificate_expiry_time(certificate: str) -> Optional[datetime]: - """Extract expiry time from a certificate string. +def calculate_expiry_notification_time( + validity_start_time: datetime, + expiry_time: datetime, + provider_recommended_notification_time: Optional[int], + requirer_recommended_notification_time: Optional[int], +) -> datetime: + """Calculate a reasonable time to notify the user about the certificate expiry. + + It takes into account the time recommended by the provider and by the requirer. + Time recommended by the provider is preferred, + then time recommended by the requirer, + then dynamically calculated time. Args: - certificate (str): x509 certificate as a string + validity_start_time: Certificate validity time + expiry_time: Certificate expiry time + provider_recommended_notification_time: + Time in hours prior to expiry to notify the user. + Recommended by the provider. + requirer_recommended_notification_time: + Time in hours prior to expiry to notify the user. + Recommended by the requirer. Returns: - Optional[datetime]: Expiry datetime or None + datetime: Time to notify the user about the certificate expiry. """ - try: - certificate_object = x509.load_pem_x509_certificate(data=certificate.encode()) - return certificate_object.not_valid_after_utc - except ValueError: - logger.warning("Could not load certificate.") - return None + if provider_recommended_notification_time is not None: + provider_recommended_notification_time = abs(provider_recommended_notification_time) + provider_recommendation_time_delta = ( + expiry_time - timedelta(hours=provider_recommended_notification_time) + ) + if validity_start_time < provider_recommendation_time_delta: + return provider_recommendation_time_delta + + if requirer_recommended_notification_time is not None: + requirer_recommended_notification_time = abs(requirer_recommended_notification_time) + requirer_recommendation_time_delta = ( + expiry_time - timedelta(hours=requirer_recommended_notification_time) + ) + if validity_start_time < requirer_recommendation_time_delta: + return requirer_recommendation_time_delta + calculated_hours = (expiry_time - validity_start_time).total_seconds() / (3600 * 3) + return expiry_time - timedelta(hours=calculated_hours) def generate_ca( @@ -965,6 +1018,8 @@ def generate_csr( # noqa: C901 organization: Optional[str] = None, email_address: Optional[str] = None, country_name: Optional[str] = None, + state_or_province_name: Optional[str] = None, + locality_name: Optional[str] = None, private_key_password: Optional[bytes] = None, sans: Optional[List[str]] = None, sans_oid: Optional[List[str]] = None, @@ -983,6 +1038,8 @@ def generate_csr( # noqa: C901 organization (str): Name of organization. email_address (str): Email address. country_name (str): Country Name. + state_or_province_name (str): State or Province Name. + locality_name (str): Locality Name. private_key_password (bytes): Private key password sans (list): Use sans_dns - this will be deprecated in a future release List of DNS subject alternative names (keeping it for now for backward compatibility) @@ -1008,6 +1065,12 @@ def generate_csr( # noqa: C901 subject_name.append(x509.NameAttribute(x509.NameOID.EMAIL_ADDRESS, email_address)) if country_name: subject_name.append(x509.NameAttribute(x509.NameOID.COUNTRY_NAME, country_name)) + if state_or_province_name: + subject_name.append( + x509.NameAttribute(x509.NameOID.STATE_OR_PROVINCE_NAME, state_or_province_name) + ) + if locality_name: + subject_name.append(x509.NameAttribute(x509.NameOID.LOCALITY_NAME, locality_name)) csr = x509.CertificateSigningRequestBuilder(subject_name=x509.Name(subject_name)) _sans: List[x509.GeneralName] = [] @@ -1135,6 +1198,7 @@ def _add_certificate( certificate_signing_request: str, ca: str, chain: List[str], + recommended_expiry_notification_time: Optional[int] = None, ) -> None: """Add certificate to relation data. @@ -1144,6 +1208,8 @@ def _add_certificate( certificate_signing_request (str): Certificate Signing Request ca (str): CA Certificate chain (list): CA Chain + recommended_expiry_notification_time (int): + Time in hours before the certificate expires to notify the user. Returns: None @@ -1161,6 +1227,7 @@ def _add_certificate( "certificate_signing_request": certificate_signing_request, "ca": ca, "chain": chain, + "recommended_expiry_notification_time": recommended_expiry_notification_time, } provider_relation_data = self._load_app_relation_data(relation) provider_certificates = provider_relation_data.get("certificates", []) @@ -1227,6 +1294,7 @@ def set_relation_certificate( ca: str, chain: List[str], relation_id: int, + recommended_expiry_notification_time: Optional[int] = None, ) -> None: """Add certificates to relation data. @@ -1236,6 +1304,8 @@ def set_relation_certificate( ca (str): CA Certificate chain (list): CA Chain relation_id (int): Juju relation ID + recommended_expiry_notification_time (int): + Recommended time in hours before the certificate expires to notify the user. Returns: None @@ -1257,6 +1327,7 @@ def set_relation_certificate( certificate_signing_request=certificate_signing_request.strip(), ca=ca.strip(), chain=[cert.strip() for cert in chain], + recommended_expiry_notification_time=recommended_expiry_notification_time, ) def remove_certificate(self, certificate: str) -> None: @@ -1310,6 +1381,13 @@ def get_provider_certificates( provider_relation_data = self._load_app_relation_data(relation) provider_certificates = provider_relation_data.get("certificates", []) for certificate in provider_certificates: + try: + certificate_object = x509.load_pem_x509_certificate( + data=certificate["certificate"].encode() + ) + except ValueError as e: + logger.error("Could not load certificate - Skipping: %s", e) + continue provider_certificate = ProviderCertificate( relation_id=relation.id, application_name=relation.app.name, @@ -1318,6 +1396,10 @@ def get_provider_certificates( ca=certificate["ca"], chain=certificate["chain"], revoked=certificate.get("revoked", False), + expiry_time=certificate_object.not_valid_after_utc, + expiry_notification_time=certificate.get( + "recommended_expiry_notification_time" + ), ) certificates.append(provider_certificate) return certificates @@ -1475,15 +1557,17 @@ def __init__( self, charm: CharmBase, relationship_name: str, - expiry_notification_time: int = 168, + expiry_notification_time: Optional[int] = None, ): """Generate/use private key and observes relation changed event. Args: charm: Charm object relationship_name: Juju relation name - expiry_notification_time (int): Time difference between now and expiry (in hours). - Used to trigger the CertificateExpiring event. Default: 7 days. + expiry_notification_time (int): Number of hours prior to certificate expiry. + Used to trigger the CertificateExpiring event. + This value is used as a recommendation only, + The actual value is calculated taking into account the provider's recommendation. """ super().__init__(charm, relationship_name) if not JujuVersion.from_environ().has_secrets: @@ -1544,9 +1628,25 @@ def get_provider_certificates(self) -> List[ProviderCertificate]: if not certificate: logger.warning("No certificate found in relation data - Skipping") continue + try: + certificate_object = x509.load_pem_x509_certificate(data=certificate.encode()) + except ValueError as e: + logger.error("Could not load certificate - Skipping: %s", e) + continue ca = provider_certificate_dict.get("ca") chain = provider_certificate_dict.get("chain", []) csr = provider_certificate_dict.get("certificate_signing_request") + recommended_expiry_notification_time = provider_certificate_dict.get( + "recommended_expiry_notification_time" + ) + expiry_time = certificate_object.not_valid_after_utc + validity_start_time = certificate_object.not_valid_before_utc + expiry_notification_time = calculate_expiry_notification_time( + validity_start_time=validity_start_time, + expiry_time=expiry_time, + provider_recommended_notification_time=recommended_expiry_notification_time, + requirer_recommended_notification_time=self.expiry_notification_time, + ) if not csr: logger.warning("No CSR found in relation data - Skipping") continue @@ -1559,6 +1659,8 @@ def get_provider_certificates(self) -> List[ProviderCertificate]: ca=ca, chain=chain, revoked=revoked, + expiry_time=expiry_time, + expiry_notification_time=expiry_notification_time, ) provider_certificates.append(provider_certificate) return provider_certificates @@ -1708,13 +1810,9 @@ def get_expiring_certificates(self) -> List[ProviderCertificate]: expiring_certificates: List[ProviderCertificate] = [] for requirer_csr in self.get_certificate_signing_requests(fulfilled_only=True): if cert := self._find_certificate_in_relation_data(requirer_csr.csr): - expiry_time = _get_certificate_expiry_time(cert.certificate) - if not expiry_time: + if not cert.expiry_time or not cert.expiry_notification_time: continue - expiry_notification_time = expiry_time - timedelta( - hours=self.expiry_notification_time - ) - if datetime.now(timezone.utc) > expiry_notification_time: + if datetime.now(timezone.utc) > cert.expiry_notification_time: expiring_certificates.append(cert) return expiring_certificates @@ -1790,13 +1888,14 @@ def _on_relation_changed(self, event: RelationChangedEvent) -> None: secret = self.model.get_secret(label=f"{LIBID}-{certificate.csr}") secret.set_content({"certificate": certificate.certificate}) secret.set_info( - expire=self._get_next_secret_expiry_time(certificate.certificate), + expire=self._get_next_secret_expiry_time(certificate), ) except SecretNotFoundError: + logger.debug("Adding secret with label %s", f"{LIBID}-{certificate.csr}") secret = self.charm.unit.add_secret( {"certificate": certificate.certificate}, label=f"{LIBID}-{certificate.csr}", - expire=self._get_next_secret_expiry_time(certificate.certificate), + expire=self._get_next_secret_expiry_time(certificate), ) self.on.certificate_available.emit( certificate_signing_request=certificate.csr, @@ -1805,7 +1904,7 @@ def _on_relation_changed(self, event: RelationChangedEvent) -> None: chain=certificate.chain, ) - def _get_next_secret_expiry_time(self, certificate: str) -> Optional[datetime]: + def _get_next_secret_expiry_time(self, certificate: ProviderCertificate) -> Optional[datetime]: """Return the expiry time or expiry notification time. Extracts the expiry time from the provided certificate, calculates the @@ -1813,17 +1912,18 @@ def _get_next_secret_expiry_time(self, certificate: str) -> Optional[datetime]: the future. Args: - certificate: x509 certificate + certificate: ProviderCertificate object Returns: Optional[datetime]: None if the certificate expiry time cannot be read, next expiry time otherwise. """ - expiry_time = _get_certificate_expiry_time(certificate) - if not expiry_time: + if not certificate.expiry_time or not certificate.expiry_notification_time: return None - expiry_notification_time = expiry_time - timedelta(hours=self.expiry_notification_time) - return _get_closest_future_time(expiry_notification_time, expiry_time) + return _get_closest_future_time( + certificate.expiry_notification_time, + certificate.expiry_time, + ) def _on_relation_broken(self, event: RelationBrokenEvent) -> None: """Handle Relation Broken Event. @@ -1864,20 +1964,19 @@ def _on_secret_expired(self, event: SecretExpiredEvent) -> None: event.secret.remove_all_revisions() return - expiry_time = _get_certificate_expiry_time(provider_certificate.certificate) - if not expiry_time: + if not provider_certificate.expiry_time: # A secret expired but matching certificate is invalid. Cleaning up event.secret.remove_all_revisions() return - if datetime.now(timezone.utc) < expiry_time: + if datetime.now(timezone.utc) < provider_certificate.expiry_time: logger.warning("Certificate almost expired") self.on.certificate_expiring.emit( certificate=provider_certificate.certificate, - expiry=expiry_time.isoformat(), + expiry=provider_certificate.expiry_time.isoformat(), ) event.secret.set_info( - expire=_get_certificate_expiry_time(provider_certificate.certificate), + expire=provider_certificate.expiry_time, ) else: logger.warning("Certificate is expired")