From bc49fab0fd69a696454d805d838f8e6cdcdbbddf Mon Sep 17 00:00:00 2001 From: saltiyazan Date: Tue, 24 Sep 2024 15:15:49 +0200 Subject: [PATCH] feat: Implement CA certificate renewal (#242) --- charmcraft.yaml | 63 ++++- .../v4/tls_certificates.py | 30 ++- src/charm.py | 245 +++++++++++++++--- src/constants.py | 7 + tests/integration/test_integration.py | 91 ++++++- tests/unit/test_charm_collect_status.py | 26 +- tests/unit/test_charm_configure.py | 137 +++++++--- tests/unit/test_charm_get_ca_certificate.py | 5 +- .../test_charm_get_issued_certificates.py | 7 +- 9 files changed, 505 insertions(+), 106 deletions(-) create mode 100644 src/constants.py diff --git a/charmcraft.yaml b/charmcraft.yaml index 31518e8..c1d8cb1 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -76,33 +76,70 @@ config: ca-common-name: type: string default: self-signed-certificates-operator - description: Common name to be used by the Certificate Authority. + description: > + Common name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-organization: type: string - description: Organization name to be used by the Certificate Authority. + description: > + Organization name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-organizational-unit: type: string - description: Organizational unit to be used by the Certificate Authority. + description: > + Organizational unit to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-email-address: type: string - description: Email address to be used by the Certificate Authority. + description: > + Email address to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-country-name: type: string - description: Country name to be used by the Certificate Authority. + description: > + Country name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-state-or-province-name: type: string - description: State or province name to be used by the Certificate Authority. + description: > + State or province name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-locality-name: type: string - description: Locality name to be used by the Certificate Authority. + description: > + Locality name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. root-ca-validity: - type: int - default: 365 - description: RootCA certificate validity (in days). + type: string + default: 365d + description: > + RootCA certificate validity. + The given value must be followed by one of: "m" for minutes, "h" for hours, "d" for days and "w" for weeks. + For example, "1m" for 1 minute, "10w" for 10 weeks. + If no units are given, the unit will be assumed as days. + Defaults to 365 days. + This value should be equal to or longer than twice the certificate-validity. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. certificate-validity: - type: int - default: 365 - description: Certificate validity (in days). + type: string + default: 90d + description: > + Signed certificate validity. + The given value must be followed by one of: "m" for minutes, "h" for hours, "d" for days and "w" for weeks. + For example, "1m" for 1 minute, "10w" for 10 weeks. + If no units are given, the unit will be assumed as days. + Defaults to 90 days. + This value should be equal to or shorter than half the root-ca-validity. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. actions: get-ca-certificate: diff --git a/lib/charms/tls_certificates_interface/v4/tls_certificates.py b/lib/charms/tls_certificates_interface/v4/tls_certificates.py index 967ee03..1e8ccc6 100644 --- a/lib/charms/tls_certificates_interface/v4/tls_certificates.py +++ b/lib/charms/tls_certificates_interface/v4/tls_certificates.py @@ -185,7 +185,7 @@ def _get_config_locality_name(self) -> Optional[str]: # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 6 +LIBPATCH = 7 PYDEPS = ["cryptography", "pydantic"] @@ -862,7 +862,7 @@ def generate_csr( # noqa: C901 def generate_ca( private_key: PrivateKey, - validity: int, + validity: timedelta, common_name: str, sans_dns: Optional[FrozenSet[str]] = frozenset(), sans_ip: Optional[FrozenSet[str]] = frozenset(), @@ -878,7 +878,7 @@ def generate_ca( Args: private_key (PrivateKey): Private key - validity (int): Certificate validity time (in days) + validity (timedelta): Certificate validity time common_name (str): Common Name that can be an IP or a Full Qualified Domain Name (FQDN). sans_dns (FrozenSet[str]): DNS Subject Alternative Names sans_ip (FrozenSet[str]): IP Subject Alternative Names @@ -945,7 +945,7 @@ def generate_ca( .public_key(private_key_object.public_key()) .serial_number(x509.random_serial_number()) .not_valid_before(datetime.now(timezone.utc)) - .not_valid_after(datetime.now(timezone.utc) + timedelta(days=validity)) + .not_valid_after(datetime.now(timezone.utc) + validity) .add_extension(x509.SubjectAlternativeName(set(_sans)), critical=False) .add_extension(x509.SubjectKeyIdentifier(digest=subject_identifier), critical=False) .add_extension( @@ -971,7 +971,7 @@ def generate_certificate( csr: CertificateSigningRequest, ca: Certificate, ca_private_key: PrivateKey, - validity: int, + validity: timedelta, is_ca: bool = False, ) -> Certificate: """Generate a TLS certificate based on a CSR. @@ -980,7 +980,7 @@ def generate_certificate( csr (CertificateSigningRequest): CSR ca (Certificate): CA Certificate ca_private_key (PrivateKey): CA private key - validity (int): Certificate validity (in days) + validity (timedelta): Certificate validity time is_ca (bool): Whether the certificate is a CA certificate Returns: @@ -999,7 +999,7 @@ def generate_certificate( .public_key(csr_object.public_key()) .serial_number(x509.random_serial_number()) .not_valid_before(datetime.now(timezone.utc)) - .not_valid_after(datetime.now(timezone.utc) + timedelta(days=validity)) + .not_valid_after(datetime.now(timezone.utc) + validity) ) extensions = _get_certificate_request_extensions( authority_key_identifier=ca_pem.extensions.get_extension_for_class( @@ -1793,6 +1793,22 @@ def get_provider_certificates( certificates.append(certificate.to_provider_certificate(relation_id=relation.id)) return certificates + def get_unsolicited_certificates( + self, relation_id: Optional[int] = None + ) -> List[ProviderCertificate]: + """Return provider certificates for which no certificate requests exists. + + Those certificates should be revoked. + """ + unsolicited_certificates: List[ProviderCertificate] = [] + provider_certificates = self.get_provider_certificates(relation_id=relation_id) + requirer_csrs = self.get_certificate_requests(relation_id=relation_id) + list_of_csrs = [csr.certificate_signing_request for csr in requirer_csrs] + for certificate in provider_certificates: + if certificate.certificate_signing_request not in list_of_csrs: + unsolicited_certificates.append(certificate) + return unsolicited_certificates + def get_outstanding_certificate_requests( self, relation_id: Optional[int] = None ) -> List[RequirerCSR]: diff --git a/src/charm.py b/src/charm.py index 9a7fe5c..0966493 100755 --- a/src/charm.py +++ b/src/charm.py @@ -4,8 +4,9 @@ """Self Signed X.509 Certificates.""" -import datetime import logging +import typing +from datetime import datetime, timedelta from typing import Optional, cast from charms.certificate_transfer_interface.v0.certificate_transfer import ( @@ -28,13 +29,14 @@ from ops.main import main from ops.model import ActiveStatus, BlockedStatus, SecretNotFoundError -logger = logging.getLogger(__name__) - +from constants import ( + CA_CERT_PATH, + CA_CERTIFICATES_SECRET_LABEL, + EXPIRING_CA_CERTIFICATES_SECRET_LABEL, + SEND_CA_CERT_REL_NAME, +) -CA_CERTIFICATES_SECRET_LABEL = "ca-certificates" -SEND_CA_CERT_REL_NAME = "send-ca-cert" # Must match metadata -CA_CERT_STORAGE = "certs" -CA_CERT_PATH = "/tmp/ca-cert.pem" +logger = logging.getLogger(__name__) @trace_charm( @@ -55,11 +57,15 @@ def __init__(self, *args): ) self.framework.observe(self.on.collect_unit_status, self._on_collect_unit_status) - self.framework.observe(self.on.update_status, self._configure) - self.framework.observe(self.on.config_changed, self._configure) - self.framework.observe(self.on.secret_expired, self._configure) - self.framework.observe(self.on.secret_changed, self._configure) - self.framework.observe(self.on.certificates_relation_changed, self._configure) + configure_events = [ + self.on.update_status, + self.on.config_changed, + self.on.secret_changed, + self.on.certificates_relation_changed, + self.on.secret_expired, + ] + for event in configure_events: + self.framework.observe(event, self._configure) self.framework.observe(self.on.get_ca_certificate_action, self._on_get_ca_certificate) self.framework.observe( self.on.get_issued_certificates_action, self._on_get_issued_certificates @@ -83,14 +89,55 @@ def _on_collect_unit_status(self, event: CollectStatusEvent): return event.add_status(ActiveStatus()) + def _is_ca_cert_active(self) -> bool: + """Return whether the CA certificate is active by checking the secret expiry.""" + secret = self.model.get_secret(label=CA_CERTIFICATES_SECRET_LABEL) + secret_info = secret.get_info() + if not secret_info.expires: + return False + if secret_info.expires.tzinfo is None: + return secret_info.expires > datetime.now() + return secret_info.expires > datetime.now(secret_info.expires.tzinfo) + @property - def _config_root_ca_certificate_validity(self) -> int: - """Return Root CA certificate validity (in days). + def _config_root_ca_certificate_validity(self) -> timedelta | None: + """Return Root CA certificate validity from the charm config as a timedelta object.""" + try: + validity = self._parse_config_time_string( + str(self.model.config.get("root-ca-validity", "")) + ) + except ValueError: + logger.warning('config option "certificate-validity" is invalid.', exc_info=True) + return None + return validity - Returns: - int: Certificate validity (in days) + @property + def _config_certificate_validity(self) -> timedelta | None: + """Returns certificate validity from the charm config as a timedelta object.""" + try: + validity = self._parse_config_time_string( + str(self.model.config.get("certificate-validity", "")) + ) + except ValueError: + logger.warning('config option "certificate-validity" is invalid.', exc_info=True) + return None + return validity + + @property + def _ca_certificate_renewal_threshold(self) -> timedelta | None: + """Return CA certificate renewal threshold. + + Which is the time difference between the validity of the root certificate + and issued certificates. + For example if the CA is valid for 365 days, + and the issued certificates are valid for 90 days, + the renewal threshold will be 275 days. + This is important so the CA does not expire during the issued certificate validity. """ - return int(self.model.config.get("root-ca-validity")) # type: ignore[arg-type] + if not self._config_root_ca_certificate_validity or not self._config_certificate_validity: + logger.warning("No root CA certificate validity or certificate validity set") + return None + return self._config_root_ca_certificate_validity - self._config_certificate_validity def _on_get_issued_certificates(self, event: ActionEvent) -> None: """Handle get-issued-certificates action. @@ -110,14 +157,30 @@ def _on_get_issued_certificates(self, event: ActionEvent) -> None: results = {"certificates": [certificate.to_json() for certificate in certificates]} event.set_results(results) - @property - def _config_certificate_validity(self) -> int: - """Returns certificate validity (in days). + def _parse_config_time_string(self, time_str: str) -> timedelta: + """Parse a given time string. + It must be a number followed by either an + m for minutes, h for hours, d for days or y for years. + + Args: + time_str: the input string. Ex: "15m", "365d", "10w" + or "10" and will be converted to days Returns: - int: Certificate validity (in days) + timedelta object representing the given string """ - return int(self.model.config.get("certificate-validity")) # type: ignore[arg-type] + if time_str.isnumeric(): + return timedelta(days=int(time_str)) + value, unit = int(time_str[:-1]), time_str[-1] + if unit == "m": + return timedelta(minutes=value) + elif unit == "h": + return timedelta(hours=value) + elif unit == "d": + return timedelta(days=value) + elif unit == "w": + return timedelta(weeks=value) + raise ValueError(f"unsupported time string format: {time_str}") @property def _config_ca_common_name(self) -> Optional[str]: @@ -167,8 +230,14 @@ def _generate_root_certificate(self) -> None: If the secret is already created, we simply update its content, else we create a new secret. """ - if not self._config_ca_common_name: - raise ValueError("CA common name should not be empty") + if ( + not self._config_ca_common_name + or not self._config_root_ca_certificate_validity + or not self._config_certificate_validity + or not self._ca_certificate_renewal_threshold + ): + logger.warning("Missing configuration for root CA certificate") + return private_key = generate_private_key() ca_certificate = generate_ca( private_key=private_key, @@ -186,18 +255,14 @@ def _generate_root_certificate(self) -> None: "private-key": str(private_key), "ca-certificate": str(ca_certificate), } - if self._root_certificate_is_stored: - secret = self.model.get_secret(label=CA_CERTIFICATES_SECRET_LABEL) - secret.set_content(content=secret_content) - else: - self.app.add_secret( - content=secret_content, - label=CA_CERTIFICATES_SECRET_LABEL, - expire=datetime.timedelta(days=self._config_root_ca_certificate_validity), - ) + self._set_juju_secret( + label=CA_CERTIFICATES_SECRET_LABEL, + content=secret_content, + expire=self._ca_certificate_renewal_threshold, + ) logger.info("Root certificates generated and stored.") - def _configure(self, event: EventBase) -> None: + def _configure(self, _: EventBase) -> None: """Validate configuration and generates root certificate. It will revoke the certificates signed by the previous root certificate. @@ -210,13 +275,76 @@ def _configure(self, event: EventBase) -> None: if self._invalid_configs(): return if not self._root_certificate_is_stored or not self._root_certificate_matches_config(): - self._generate_root_certificate() self.tls_certificates.revoke_all_certificates() + self._clean_up_juju_secret(EXPIRING_CA_CERTIFICATES_SECRET_LABEL) + self._generate_root_certificate() logger.info("Revoked all previously issued certificates.") return + if not self._is_ca_cert_active(): + logger.info("Renewing CA certificate") + self._renew_root_certificate() + return self._send_ca_cert() self._process_outstanding_certificate_requests() + def _renew_root_certificate(self): + """Generate a new active root CA certificate. + + If there is a CA certificate that is about to expire, + move it to the expiring-ca-certificate secret. + Generate a new active CA certificate + """ + if not self.unit.is_leader(): + return + self._move_active_ca_cert_to_expiring() + self._generate_root_certificate() + + def _move_active_ca_cert_to_expiring(self): + """Make current active CA certificate expiring. + + CA certificate is moved to the secret holding expiring CA certificates. + The validity of the expiring CA can't be shorter than + the validity of the issued certificates. + """ + if ( + not self._config_ca_common_name + or not self._config_root_ca_certificate_validity + or not self._config_certificate_validity + or not self._ca_certificate_renewal_threshold + ): + logger.warning("Missing configuration for expiring CA certificate") + return + try: + current_active_ca_cert_secret = self.model.get_secret( + label=CA_CERTIFICATES_SECRET_LABEL + ) + current_active_ca_cert_secret_content = current_active_ca_cert_secret.get_content( + refresh=True + ) + except SecretNotFoundError: + logger.warning("No active CA certificate found to move to expiring") + return + self._set_juju_secret( + label=EXPIRING_CA_CERTIFICATES_SECRET_LABEL, + content=current_active_ca_cert_secret_content, + expire=self._config_certificate_validity, + ) + + def _set_juju_secret(self, label: str, content: dict[str, str], expire: timedelta) -> None: + """Create or update a juju secret.""" + try: + secret = self.model.get_secret(label=label) + date_time_expire = datetime.now() + expire + # TODO, Workaround for https://github.com/canonical/operator/issues/1288 + secret._backend.secret_set( + typing.cast(str, secret.get_info().id), + content=content, + expire=date_time_expire, + label=label, + ) + except SecretNotFoundError: + self.app.add_secret(content=content, label=label, expire=expire) + def _root_certificate_matches_config(self) -> bool: """Return whether the stored root certificate matches with the config.""" if not self._config_ca_common_name: @@ -224,7 +352,32 @@ def _root_certificate_matches_config(self) -> bool: ca_certificate_secret = self.model.get_secret(label=CA_CERTIFICATES_SECRET_LABEL) ca_certificate_secret_content = ca_certificate_secret.get_content(refresh=True) ca = ca_certificate_secret_content["ca-certificate"] - return self._config_ca_common_name == Certificate.from_string(ca).common_name + certificate = Certificate.from_string(ca) + configured_root_ca_validity = ( + certificate.expiry_time - certificate.validity_start_time + if certificate.validity_start_time and certificate.expiry_time + else timedelta(days=0) + ) + + return ( + self._config_ca_common_name == certificate.common_name + and self._config_ca_organization == certificate.organization + and self._config_ca_organizational_unit == certificate.organizational_unit + and self._config_ca_email_address == certificate.email_address + and self._config_ca_country_name == certificate.country_name + and self._config_ca_state_or_province_name == certificate.state_or_province_name + and self._config_ca_locality_name == certificate.locality_name + and self._config_root_ca_certificate_validity == configured_root_ca_validity + ) + + def _clean_up_juju_secret(self, label: str): + """Remove the secret with the given label.""" + try: + expiring_ca_secret = self.model.get_secret(label=label) + expiring_ca_secret.remove_all_revisions() + except SecretNotFoundError: + return + return def _process_outstanding_certificate_requests(self) -> None: """Process outstanding certificate requests.""" @@ -244,10 +397,16 @@ def _invalid_configs(self) -> list[str]: invalid_configs = [] if not self._config_ca_common_name: invalid_configs.append("ca-common-name") - if not self._config_root_ca_certificate_validity: - invalid_configs.append("root-ca-validity") - if not self._config_certificate_validity: + if ( + not self._config_certificate_validity + or not self._config_root_ca_certificate_validity + or not self._config_root_ca_certificate_validity + >= 2 * self._config_certificate_validity + or self._config_root_ca_certificate_validity == timedelta(days=0) + or self._config_certificate_validity == timedelta(days=0) + ): invalid_configs.append("certificate-validity") + invalid_configs.append("root-ca-validity") return invalid_configs def _generate_self_signed_certificate( @@ -260,6 +419,14 @@ def _generate_self_signed_certificate( is_ca (bool): Whether the certificate is a CA relation_id (int): Relation id """ + if ( + not self._config_ca_common_name + or not self._config_root_ca_certificate_validity + or not self._config_certificate_validity + or not self._ca_certificate_renewal_threshold + ): + logger.warning("Missing configuration for self-signed certificate") + return ca_certificate_secret = self.model.get_secret(label=CA_CERTIFICATES_SECRET_LABEL) ca_certificate_secret_content = ca_certificate_secret.get_content(refresh=True) ca_certificate = Certificate.from_string(ca_certificate_secret_content["ca-certificate"]) diff --git a/src/constants.py b/src/constants.py new file mode 100644 index 0000000..8115bf7 --- /dev/null +++ b/src/constants.py @@ -0,0 +1,7 @@ +"""Constants.""" + +CA_CERTIFICATES_SECRET_LABEL = "active-ca-certificates" +EXPIRING_CA_CERTIFICATES_SECRET_LABEL = "expiring-ca-certificates" +CA_CERT_PATH = "/tmp/ca-cert.pem" +TLS_LIB_PATH = "charms.tls_certificates_interface.v4.tls_certificates" +SEND_CA_CERT_REL_NAME = "send-ca-cert" # Must match metadata diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 043e1a4..88dccbd 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -3,6 +3,7 @@ # See LICENSE file for licensing details. +import json import logging import platform import time @@ -23,25 +24,37 @@ CA_COMMON_NAME = "example.com" ARCH = "arm64" if platform.machine() == "aarch64" else "amd64" +REQUIRER_CHARM_REVISION_ARM = 103 +REQUIRER_CHARM_REVISION_AMD = 104 -async def wait_for_requirer_ca_certificate(ops_test: OpsTest, ca_common_name: str) -> None: - """Wait for the certificate to be provided to the `tls-requirer-requirer/0` unit.""" +async def wait_for_requirer_certificates(ops_test: OpsTest, ca_common_name: str) -> Dict[str, str]: + """Wait for the certificate to be provided to the `tls-requirer-requirer/0` unit. + + Checks that CA certificate common name is the one expected. + Returns the certificate output from the get-certificate action if successful. + Otherwise, times out and raises a TimeoutError. + """ t0 = time.time() timeout = 300 while time.time() - t0 < timeout: logger.info("Waiting for CA certificate with common name %s", ca_common_name) time.sleep(5) action_output = await run_get_certificate_action(ops_test) - ca_certificate = action_output.get("ca-certificate", "") - if not ca_certificate: + try: + certificates = json.loads(action_output.get("certificates", ""))[0] + except json.JSONDecodeError: + continue + ca_certificate = certificates.get("ca-certificate", "") + certificate = certificates.get("certificate", "") + if not ca_certificate or not certificate: continue existing_ca_common_name = get_common_name_from_certificate(ca_certificate.encode()) if existing_ca_common_name != ca_common_name: logger.info("Existing CA Common Name: %s", existing_ca_common_name) continue logger.info("Certificate with CA common name %s provided", ca_common_name) - return + return certificates raise TimeoutError("Timed out waiting for certificate") @@ -58,12 +71,23 @@ async def deploy(ops_test: OpsTest, request): application_name=APP_NAME, series="jammy", trust=True, - config={"ca-common-name": CA_COMMON_NAME}, + config={ + "ca-common-name": CA_COMMON_NAME, + "root-ca-validity": "200", + "certificate-validity": "100", + "ca-email-address": "test@example.com", + "ca-country-name": "US", + "ca-state-or-province-name": "California", + "ca-locality-name": "San Francisco", + "ca-organization": "Example Org", + "ca-organizational-unit": "Example Unit", + }, constraints={"arch": ARCH}, ) await ops_test.model.deploy( TLS_REQUIRER_CHARM_NAME, application_name=TLS_REQUIRER_CHARM_NAME, + revision=REQUIRER_CHARM_REVISION_ARM if ARCH == "arm64" else REQUIRER_CHARM_REVISION_AMD, channel="stable", constraints={"arch": ARCH}, ) @@ -95,7 +119,7 @@ async def test_given_tls_requirer_is_deployed_when_integrated_then_certificate_i status="active", timeout=1000, ) - await wait_for_requirer_ca_certificate(ops_test=ops_test, ca_common_name=CA_COMMON_NAME) + await wait_for_requirer_certificates(ops_test=ops_test, ca_common_name=CA_COMMON_NAME) async def test_given_tls_requirer_is_integrated_when_ca_common_name_config_changed_then_new_certificate_is_provided( # noqa: E501 @@ -113,7 +137,58 @@ async def test_given_tls_requirer_is_integrated_when_ca_common_name_config_chang timeout=1000, ) - await wait_for_requirer_ca_certificate(ops_test=ops_test, ca_common_name=new_common_name) + await wait_for_requirer_certificates(ops_test=ops_test, ca_common_name=new_common_name) + + +async def test_given_tls_requirer_is_integrated_when_certificates_expires_then_new_certificate_is_provided( # noqa: E501 + ops_test: OpsTest, + deploy, +): + new_common_name = "newexample.org" + assert ops_test.model + application = ops_test.model.applications[APP_NAME] + assert application + await application.set_config( + { + "root-ca-validity": "3m", + "certificate-validity": "1m", + } + ) + await ops_test.model.wait_for_idle( + apps=[APP_NAME, TLS_REQUIRER_CHARM_NAME], + status="active", + timeout=1000, + ) + + action_output = await wait_for_requirer_certificates( + ops_test=ops_test, ca_common_name=new_common_name + ) + new_common_name_certificate = action_output.get("certificate", "") + new_common_name_ca = action_output.get("ca-certificate", "") + + assert new_common_name_certificate + + # Wait for the certificate to expire + time.sleep(60) + + action_output = await wait_for_requirer_certificates( + ops_test=ops_test, ca_common_name=new_common_name + ) + renewed_certificate = action_output.get("certificate", "") + assert renewed_certificate + assert renewed_certificate != new_common_name_certificate + assert action_output.get("ca-certificate", "") == new_common_name_ca + + # Wait for the CA certificate to expire + time.sleep(120) + action_output = await wait_for_requirer_certificates( + ops_test=ops_test, ca_common_name=new_common_name + ) + new_certificate_with_new_ca = action_output.get("certificate", "") + new_ca = action_output.get("ca-certificate", "") + assert new_certificate_with_new_ca + assert new_certificate_with_new_ca != renewed_certificate + assert new_ca != new_common_name_ca async def test_given_charm_scaled_then_charm_does_not_crash( diff --git a/tests/unit/test_charm_collect_status.py b/tests/unit/test_charm_collect_status.py index 0abc754..90126ab 100644 --- a/tests/unit/test_charm_collect_status.py +++ b/tests/unit/test_charm_collect_status.py @@ -21,7 +21,7 @@ def test_given_invalid_config_when_collect_unit_status_then_status_is_blocked(se state_in = scenario.State( config={ "ca-common-name": "", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, ) @@ -36,7 +36,7 @@ def test_given_invalid_validity_config_when_collect_unit_status_then_status_is_b state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 0, + "certificate-validity": "0", }, leader=True, ) @@ -44,7 +44,25 @@ def test_given_invalid_validity_config_when_collect_unit_status_then_status_is_b state_out = self.ctx.run(self.ctx.on.collect_unit_status(), state=state_in) assert state_out.unit_status == BlockedStatus( - "The following configuration values are not valid: ['certificate-validity']" + "The following configuration values are not valid: ['certificate-validity', 'root-ca-validity']" # noqa: E501 + ) + + def test_given_root_ca_validity_config_not_twice_the_certificate_validity_when_collect_unit_status_then_status_is_blocked( # noqa: E501 + self, + ): + state_in = scenario.State( + config={ + "ca-common-name": "pizza.example.com", + "certificate-validity": "100", + "root-ca-validity": "0", + }, + leader=True, + ) + + state_out = self.ctx.run(self.ctx.on.collect_unit_status(), state=state_in) + + assert state_out.unit_status == BlockedStatus( + "The following configuration values are not valid: ['certificate-validity', 'root-ca-validity']" # noqa: E501 ) @patch("charm.generate_private_key") @@ -59,7 +77,7 @@ def test_given_valid_config_when_collect_unit_status_then_status_is_active( state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, ) diff --git a/tests/unit/test_charm_configure.py b/tests/unit/test_charm_configure.py index 0b0d6bb..8586959 100644 --- a/tests/unit/test_charm_configure.py +++ b/tests/unit/test_charm_configure.py @@ -15,9 +15,12 @@ generate_private_key, ) -from charm import CA_CERTIFICATES_SECRET_LABEL, SelfSignedCertificatesCharm - -TLS_LIB_PATH = "charms.tls_certificates_interface.v4.tls_certificates" +from charm import SelfSignedCertificatesCharm +from constants import ( + CA_CERTIFICATES_SECRET_LABEL, + EXPIRING_CA_CERTIFICATES_SECRET_LABEL, + TLS_LIB_PATH, +) class TestCharmConfigure: @@ -47,8 +50,8 @@ def test_given_valid_config_when_config_changed_then_ca_certificate_is_pushed_to state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, - "root-ca-validity": 200, + "certificate-validity": "100", + "root-ca-validity": "200", }, leader=True, ) @@ -69,11 +72,13 @@ def test_given_valid_config_when_config_changed_then_ca_certificate_is_stored_in patch_generate_ca.return_value = ca_certificate_string patch_generate_private_key.return_value = private_key_string + certificate_validity = 100 + root_ca_validity = 200 state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, - "root-ca-validity": 200, + "certificate-validity": str(certificate_validity), + "root-ca-validity": str(root_ca_validity), }, leader=True, ) @@ -85,7 +90,7 @@ def test_given_valid_config_when_config_changed_then_ca_certificate_is_stored_in assert content["private-key"] == private_key_string ca_certificates_secret_expiry = ca_certificates_secret.expire assert ca_certificates_secret_expiry - expected_delta = timedelta(days=200) + expected_delta = timedelta(days=root_ca_validity - certificate_validity) actual_delta = ca_certificates_secret_expiry - datetime.now() tolerance = timedelta(seconds=1) assert ( @@ -106,7 +111,7 @@ def test_given_root_certificate_not_stored_when_config_changed_then_existing_cer state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, secrets=frozenset(), @@ -118,7 +123,7 @@ def test_given_root_certificate_not_stored_when_config_changed_then_existing_cer @patch("charm.generate_private_key") @patch("charm.generate_ca") - def test_given_new_common_name_when_config_changed_then_new_root_ca_is_stored( + def test_given_new_root_ca_config_when_config_changed_then_new_root_ca_is_replaced( self, patch_generate_ca, patch_generate_private_key, @@ -127,12 +132,12 @@ def test_given_new_common_name_when_config_changed_then_new_root_ca_is_stored( initial_ca_certificate = generate_ca( private_key=ca_private_key, common_name="initial.example.com", - validity=100, + validity=timedelta(days=100), ) new_ca = generate_ca( private_key=ca_private_key, common_name="new.example.com", - validity=100, + validity=timedelta(days=100), ) patch_generate_ca.return_value = new_ca patch_generate_private_key.return_value = ca_private_key @@ -141,7 +146,7 @@ def test_given_new_common_name_when_config_changed_then_new_root_ca_is_stored( "ca-certificate": str(initial_ca_certificate), "private-key": str(ca_private_key), }, - label="ca-certificates", + label=CA_CERTIFICATES_SECRET_LABEL, owner="app", expire=datetime.now() + timedelta(days=100), ) @@ -151,7 +156,8 @@ def test_given_new_common_name_when_config_changed_then_new_root_ca_is_stored( "ca-email-address": "abc@example.com", "ca-country-name": "CA", "ca-locality-name": "Montreal", - "certificate-validity": 100, + "certificate-validity": "100", + "root-ca-validity": "200", }, leader=True, secrets={ca_certificate_secret}, @@ -159,7 +165,7 @@ def test_given_new_common_name_when_config_changed_then_new_root_ca_is_stored( state_out = self.ctx.run(self.ctx.on.config_changed(), state=state_in) - ca_certificates_secret = state_out.get_secret(label="ca-certificates") + ca_certificates_secret = state_out.get_secret(label=CA_CERTIFICATES_SECRET_LABEL) secret_content = ca_certificates_secret.latest_content assert secret_content is not None assert secret_content["ca-certificate"] == str(new_ca) @@ -172,7 +178,69 @@ def test_given_new_common_name_when_config_changed_then_new_root_ca_is_stored( country_name="CA", state_or_province_name=None, locality_name="Montreal", - validity=365, + validity=timedelta(days=200), + ) + + @patch("charm.generate_private_key") + @patch("charm.generate_ca") + def test_given_root_ca_about_to_expire_then_root_ca_is_marked_expiring_and_new_one_is_generated( # noqa: E501 + self, + patch_generate_ca, + patch_generate_private_key, + ): + initial_ca_private_key = generate_private_key() + new_ca_private_key = generate_private_key() + initial_ca_certificate = generate_ca( + private_key=initial_ca_private_key, + common_name="example.com", + validity=timedelta(minutes=2), + ) + new_ca_certificate = generate_ca( + private_key=new_ca_private_key, + common_name="example.com", + validity=timedelta(minutes=2), + ) + patch_generate_ca.return_value = new_ca_certificate + patch_generate_private_key.return_value = new_ca_private_key + ca_certificate_secret = scenario.Secret( + { + "ca-certificate": str(initial_ca_certificate), + "private-key": str(initial_ca_private_key), + }, + label=CA_CERTIFICATES_SECRET_LABEL, + owner="app", + expire=datetime.now() + timedelta(milliseconds=1), + ) + state_in = scenario.State( + config={ + "ca-common-name": "example.com", + "root-ca-validity": "2m", + "certificate-validity": "1m", + }, + leader=True, + secrets={ca_certificate_secret}, + ) + + state_out = self.ctx.run(self.ctx.on.config_changed(), state=state_in) + + ca_certificates_secret = state_out.get_secret(label=CA_CERTIFICATES_SECRET_LABEL) + + secret_content = ca_certificates_secret.latest_content + expiring_ca_certificates_secret = state_out.get_secret( + label=EXPIRING_CA_CERTIFICATES_SECRET_LABEL + ) + expiring_secret_content = expiring_ca_certificates_secret.latest_content + assert expiring_secret_content is not None + assert secret_content is not None + assert secret_content["ca-certificate"] == str(new_ca_certificate) + assert secret_content["private-key"] == str(new_ca_private_key) + assert expiring_secret_content["ca-certificate"] == str(initial_ca_certificate) + assert expiring_secret_content["private-key"] == str(initial_ca_private_key) + assert expiring_ca_certificates_secret.expire + tolerance = timedelta(seconds=1) + assert ( + abs(expiring_ca_certificates_secret.expire - (datetime.now() + timedelta(minutes=1))) + <= tolerance ) @patch(f"{TLS_LIB_PATH}.TLSCertificatesProvidesV4.set_relation_certificate") @@ -189,14 +257,14 @@ def test_given_outstanding_certificate_requests_when_config_changed_then_certifi provider_ca = generate_ca( private_key=provider_private_key, common_name="example.com", - validity=100, + validity=timedelta(days=200), ) requirer_csr = generate_csr(private_key=requirer_private_key, common_name="example.com") certificate = generate_certificate( csr=requirer_csr, ca=provider_ca, ca_private_key=provider_private_key, - validity=100, + validity=timedelta(days=100), ) tls_relation = scenario.Relation( endpoint="certificates", @@ -207,7 +275,7 @@ def test_given_outstanding_certificate_requests_when_config_changed_then_certifi "ca-certificate": str(provider_ca), "private-key": str(provider_private_key), }, - label="ca-certificates", + label=CA_CERTIFICATES_SECRET_LABEL, owner="app", expire=datetime.now() + timedelta(days=100), ) @@ -222,7 +290,8 @@ def test_given_outstanding_certificate_requests_when_config_changed_then_certifi state_in = scenario.State( config={ "ca-common-name": "example.com", - "certificate-validity": 100, + "certificate-validity": "100", + "root-ca-validity": "200", }, leader=True, relations={tls_relation}, @@ -260,14 +329,14 @@ def test_given_valid_config_and_unit_is_leader_when_secret_expired_then_new_ca_c "ca-certificate": "whatever initial CA certificate", "private-key": private_key_string, }, - label="ca-certificates", + label=CA_CERTIFICATES_SECRET_LABEL, owner="app", expire=datetime.now(), ) state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, secrets={ca_certificates_secret}, @@ -277,7 +346,9 @@ def test_given_valid_config_and_unit_is_leader_when_secret_expired_then_new_ca_c self.ctx.on.secret_expired(secret=ca_certificates_secret, revision=1), state=state_in ) - ca_certificates_secret = state_out.get_secret(label="ca-certificates").latest_content + ca_certificates_secret = state_out.get_secret( + label=CA_CERTIFICATES_SECRET_LABEL + ).latest_content assert ca_certificates_secret is not None assert ca_certificates_secret["ca-certificate"] == ca_certificate_string @@ -295,12 +366,12 @@ def test_given_initial_config_when_config_changed_then_stored_ca_common_name_use initial_ca_certificate = generate_ca( private_key=initial_ca_private_key, common_name="common-name-initial.example.com", - validity=100, + validity=timedelta(days=100), ) new_ca_certificate = generate_ca( private_key=new_ca_private_key, common_name="common-name-new.example.com", - validity=100, + validity=timedelta(days=100), ) patch_generate_ca.return_value = new_ca_certificate patch_generate_private_key.return_value = new_ca_private_key @@ -310,14 +381,14 @@ def test_given_initial_config_when_config_changed_then_stored_ca_common_name_use "ca-certificate": str(initial_ca_certificate), "private-key": str(initial_ca_private_key), }, - label="ca-certificates", + label=CA_CERTIFICATES_SECRET_LABEL, owner="app", expire=datetime.now() + timedelta(days=100), ) state_in = scenario.State( config={ "ca-common-name": "common-name-new.example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, secrets={ca_certificates_secret}, @@ -325,7 +396,9 @@ def test_given_initial_config_when_config_changed_then_stored_ca_common_name_use state_out = self.ctx.run(self.ctx.on.config_changed(), state=state_in) - ca_certificates_secret = state_out.get_secret(label="ca-certificates").latest_content + ca_certificates_secret = state_out.get_secret( + label=CA_CERTIFICATES_SECRET_LABEL + ).latest_content assert ca_certificates_secret is not None assert ca_certificates_secret["ca-certificate"] == str(new_ca_certificate) assert ca_certificates_secret["private-key"] == str(new_ca_private_key) @@ -343,15 +416,16 @@ def test_given_certificate_transfer_relations_when_configure_then_ca_cert_is_adv provider_ca = generate_ca( private_key=provider_private_key, common_name="example.com", - validity=100, + validity=timedelta(days=200), ) secret = scenario.Secret( { "ca-certificate": str(provider_ca), "private-key": str(provider_private_key), }, - label="ca-certificates", + label=CA_CERTIFICATES_SECRET_LABEL, owner="app", + expire=datetime.now() + timedelta(days=100), ) state_in = scenario.State( relations={traefik_relation, another_relation}, @@ -359,7 +433,8 @@ def test_given_certificate_transfer_relations_when_configure_then_ca_cert_is_adv leader=True, config={ "ca-common-name": "example.com", - "certificate-validity": 100, + "certificate-validity": "100", + "root-ca-validity": "200", }, ) diff --git a/tests/unit/test_charm_get_ca_certificate.py b/tests/unit/test_charm_get_ca_certificate.py index dbfb390..cccdb2d 100644 --- a/tests/unit/test_charm_get_ca_certificate.py +++ b/tests/unit/test_charm_get_ca_certificate.py @@ -2,6 +2,8 @@ # See LICENSE file for licensing details. +from datetime import datetime, timedelta + import pytest import scenario @@ -23,8 +25,9 @@ def test_given_ca_cert_generated_when_get_ca_certificate_action_then_returns_ca_ { "ca-certificate": ca_certificate, }, - label="ca-certificates", + label="active-ca-certificates", owner="app", + expire=datetime.now() + timedelta(days=100), ) state_in = scenario.State( leader=True, diff --git a/tests/unit/test_charm_get_issued_certificates.py b/tests/unit/test_charm_get_issued_certificates.py index 47160ab..54b61f4 100644 --- a/tests/unit/test_charm_get_issued_certificates.py +++ b/tests/unit/test_charm_get_issued_certificates.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import json +from datetime import timedelta from unittest.mock import patch import pytest @@ -45,7 +46,7 @@ def test_given_certificates_issued_when_get_issued_certificates_action_then_acti ca_certificate = generate_ca( private_key=ca_private_key, common_name="example.com", - validity=100, + validity=timedelta(days=100), ) requirer_private_key = generate_private_key() csr = generate_csr(private_key=requirer_private_key, common_name="example.com") @@ -53,7 +54,7 @@ def test_given_certificates_issued_when_get_issued_certificates_action_then_acti csr=csr, ca=ca_certificate, ca_private_key=ca_private_key, - validity=100, + validity=timedelta(days=100), ) chain = [ca_certificate, certificate] revoked = False @@ -70,7 +71,7 @@ def test_given_certificates_issued_when_get_issued_certificates_action_then_acti state_in = scenario.State( config={ "ca-common-name": "example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, )