From c08b31d906ac879da6bd2c6d7a0616ec4335b68f Mon Sep 17 00:00:00 2001 From: tphan025 Date: Thu, 9 Jul 2026 08:59:26 +0200 Subject: [PATCH 1/3] feat(tls): re-provide tls.ca certificates on refresh --- .gitignore | 3 + sunbeam-python/sunbeam/commands/refresh.py | 41 ++++++ sunbeam-python/sunbeam/features/tls/ca.py | 68 ++++++++-- sunbeam-python/sunbeam/features/tls/common.py | 107 ++++++++++++++++ .../unit/sunbeam/commands/test_refresh.py | 56 +++++++++ .../tests/unit/sunbeam/features/test_tls.py | 119 ++++++++++++++++++ 6 files changed, 387 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 0dd03c9d7..f4c9a03ee 100644 --- a/.gitignore +++ b/.gitignore @@ -162,3 +162,6 @@ cython_debug/ .vscode/ .stestr/ + +# Git worktrees +.worktrees/ diff --git a/sunbeam-python/sunbeam/commands/refresh.py b/sunbeam-python/sunbeam/commands/refresh.py index 4de83e8e4..4a80267c8 100644 --- a/sunbeam-python/sunbeam/commands/refresh.py +++ b/sunbeam-python/sunbeam/commands/refresh.py @@ -26,6 +26,7 @@ from sunbeam.core.openstack import OPENSTACK_MODEL from sunbeam.core.terraform import TerraformInitStep from sunbeam.features.interface.v1.base import is_maas_deployment +from sunbeam.features.tls.ca import CaTlsFeature from sunbeam.steps.horizon import AttachHorizonThemeStep from sunbeam.steps.k8s import DeployK8SApplicationStep from sunbeam.steps.k8s_upgrade import K8SCharmUpgradeStep @@ -73,6 +74,22 @@ def _stored_manifest_risk(client) -> str | None: return risk_counts.most_common(1)[0][0] +def _fix_tls_certs(deployment: Deployment, show_hints: bool) -> None: + """Re-provide stored tls.ca certificates for any outstanding CSRs. + + No-op when tls.ca is not the active TLS provider. + """ + feature = deployment.get_feature_manager().resolve_feature(CaTlsFeature.name) + if not isinstance(feature, CaTlsFeature) or not feature.is_enabled( + deployment.get_client() + ): + LOG.debug("tls.ca is not enabled; skipping TLS certificate re-provision.") + return + + feature.reapply_certificates(deployment, show_hints) + click.echo("TLS certificate re-provision complete.") + + @click.group("refresh", invoke_without_command=True) @click.option( "-c", @@ -205,6 +222,12 @@ def refresh( show_hints, ) + if not upgrade_release: + # Recover deployments affected by the traefik certificate bug by + # re-providing stored tls.ca certificates for any outstanding CSRs. + # No-op unless tls.ca is enabled and there are outstanding requests. + _fix_tls_certs(deployment, show_hints) + click.echo("Refresh complete.") @@ -419,3 +442,21 @@ def refresh_k8s( if message: click.echo(message) click.echo("k8s refresh complete.") + + +@refresh.command("certificates") +@click_option_show_hints +@click.pass_context +def refresh_certificates( + ctx: click.Context, + show_hints: bool = False, +) -> None: + """Re-provide stored tls.ca certificates for outstanding CSRs. + + This is the same as the last step of `sunbeam cluster refresh` + and it is a no-op unless tls.ca is enabled and there are outstanding csrs to be + reprovisioned. + """ + deployment: Deployment = ctx.obj + run_preflight_checks([JujuLoginCheck(deployment.juju_account)], console) + _fix_tls_certs(deployment, show_hints) diff --git a/sunbeam-python/sunbeam/features/tls/ca.py b/sunbeam-python/sunbeam/features/tls/ca.py index 5ec2fd502..e2477fdf9 100644 --- a/sunbeam-python/sunbeam/features/tls/ca.py +++ b/sunbeam-python/sunbeam/features/tls/ca.py @@ -18,14 +18,13 @@ from sunbeam.core.common import ( FORMAT_TABLE, FORMAT_YAML, + ResultType, + get_step_result, read_config, run_plan, str_presenter, ) from sunbeam.core.deployment import Deployment -from sunbeam.core.juju import ( - JujuHelper, -) from sunbeam.core.manifest import ( AddManifestStep, FeatureConfig, @@ -45,6 +44,7 @@ CERTIFICATE_FEATURE_KEY, INGRESS_CHANGE_APPLICATION_TIMEOUT, ConfigureTLSCertificatesStep, + ReapplyTLSCertificatesStep, TlsFeature, TlsFeatureConfig, certificate_questions, @@ -180,6 +180,16 @@ def set_tfvars_on_resize( """Set terraform variables to resize the application.""" return {} + def _local_apps_to_monitor(self, deployment: Deployment) -> list[str]: + """Local applications to monitor after a TLS certificate change.""" + client = deployment.get_client() + apps = ["traefik", "traefik-public"] + if not deployment.external_keystone_model: + apps.append("keystone") + if client.cluster.list_nodes_by_role("storage"): + apps.append("traefik-rgw") + return apps + @click.group() def ca_group(self) -> None: """Manage CA.""" @@ -242,9 +252,7 @@ def configure( if (ca := manifest.get_feature(self.name.split(".")[-1])) and ca.config: preseed = ca.config.model_dump(by_alias=True) model = OPENSTACK_MODEL - apps_to_monitor = ["traefik", "traefik-public", "keystone"] - if client.cluster.list_nodes_by_role("storage"): - apps_to_monitor.append("traefik-rgw") + apps_to_monitor = self._local_apps_to_monitor(deployment) try: config = read_config(client, CERTIFICATE_FEATURE_KEY) @@ -256,7 +264,7 @@ def configure( if ca is None: raise click.ClickException("CA is not configured") - jhelper = JujuHelper(deployment.juju_controller) + jhelper = deployment.get_juju_helper() plan = [ AddManifestStep(client, manifest_path), ConfigureTLSCertificatesStep( @@ -276,6 +284,52 @@ def configure( run_plan(plan, console, show_hints) click.echo("CA certs configured") + def reapply_certificates( + self, deployment: Deployment, show_hints: bool = False + ) -> None: + """Re-provide stored certificates for any outstanding CSRs. + + Used to recover a deployment after a charm refresh (e.g. the traefik + charm crossing revision 308) leaves applications with outstanding + certificate requests. Reuses the certificates already stored in + clusterd; never prompts. No-op when there are no outstanding CSRs or + none match a stored certificate. + """ + client = deployment.get_client() + try: + config = read_config(client, CERTIFICATE_FEATURE_KEY) + except ConfigItemNotFoundException: + config = {} + ca = config.get("ca") + ca_chain = config.get("chain") + + if ca is None: + raise click.ClickException("CA is not configured") + + model = OPENSTACK_MODEL + apps_to_monitor = self._local_apps_to_monitor(deployment) + + jhelper = deployment.get_juju_helper() + rerun_provide_certificates_plan = [ + ReapplyTLSCertificatesStep(client, jhelper, ca, ca_chain) + ] + results = run_plan(rerun_provide_certificates_plan, console, show_hints) + + reapply_result = get_step_result(results, ReapplyTLSCertificatesStep) + if reapply_result.result_type != ResultType.SKIPPED: + run_plan( + [ + WaitForApplicationsStep( + jhelper, + apps_to_monitor, + model, + INGRESS_CHANGE_APPLICATION_TIMEOUT, + ) + ], + console, + show_hints, + ) + def enabled_commands(self) -> dict[str, list[dict]]: """Dict of clickgroup along with commands. diff --git a/sunbeam-python/sunbeam/features/tls/common.py b/sunbeam-python/sunbeam/features/tls/common.py index ad85bc246..b10be7a68 100644 --- a/sunbeam-python/sunbeam/features/tls/common.py +++ b/sunbeam-python/sunbeam/features/tls/common.py @@ -713,3 +713,110 @@ def run(self, context: StepContext) -> Result: ) return Result(ResultType.COMPLETED) + + +class ReapplyTLSCertificatesStep(ConfigureTLSCertificatesStep): + """Re-provide previously signed certificates for outstanding CSRs. + + Recovers a deployment after a charm refresh (e.g. the traefik charm + crossing revision 308) leaves applications with outstanding certificate + requests. It re-runs the ``provide-certificate`` action using the + certificates already stored in clusterd, matched to each outstanding CSR + by its subject. + + Unlike :class:`ConfigureTLSCertificatesStep`, this step is non-interactive: + it never prompts. Outstanding CSRs without a matching stored certificate + (e.g. a genuinely new CSR) are skipped with a warning so that a stale + certificate is never provided for an unknown request. + """ + + def has_prompts(self) -> bool: + """This step never prompts the user.""" + return False + + def is_skip(self, context: StepContext) -> Result: + """Collect outstanding CSRs and match them to stored certificates. + + Populates ``self.process_certs`` from the certificates stored in + clusterd so the inherited ``run()`` re-provides them. + + :return: SKIPPED when there is nothing to re-provide, FAILED on error, + COMPLETED when at least one CSR was matched to a stored cert. + """ + try: + action_result = get_outstanding_certificate_requests( + self.app, self.model, self.jhelper + ) + except (LeaderNotFoundException, ActionFailedException) as e: + LOG.debug("Failed to get CSRs for %s: %s", self.app, e) + return Result(ResultType.FAILED, str(e)) + + if action_result.get("return-code", 0) > 1: + return Result( + ResultType.FAILED, + "Unable to get outstanding certificate requests from CA", + ) + + certs_to_process = json.loads(action_result.get("result", "[]")) + if not certs_to_process: + return Result(ResultType.SKIPPED, "No outstanding certificate requests") + + stored_certs = questions.load_answers(self.client, self._CONFIG).get( + "certificates", {} + ) + if not stored_certs: + return Result(ResultType.SKIPPED, "No stored certificates to re-provide") + + try: + relation_map = self.jhelper.get_relation_map( + self.app, self.interface, self.model + ) + except JujuException as e: + return Result(ResultType.FAILED, f"Unable to get relation map: {e}") + + processed_records: set = set() + for record in certs_to_process: + # Multiple units of the same application may share a CSR/relation_id. + hashable_record = tuple(sorted(record.items())) + if hashable_record in processed_records: + continue + processed_records.add(hashable_record) + + unit_name = record.get("unit_name") + csr = record.get("csr") + app = record.get("application_name") + relation_id = record.get("relation_id") + if relation_id: + app = relation_map.get(f"{self.interface}:{relation_id}") + elif unit_name: + app = unit_name.split("/")[0] + + subject = get_subject_from_csr(csr) + if not subject: + LOG.warning("Skipping invalid CSR for unit %s", unit_name) + continue + + stored_cert = stored_certs.get(subject, {}).get("certificate") + if not stored_cert: + LOG.warning( + "No stored certificate for outstanding CSR (app=%s, subject=%s); " + "skipping. Sign it with `sunbeam tls ca unit_certs`.", + app, + subject, + ) + continue + + self.process_certs[subject] = { + "app": app, + "unit": unit_name, + "relation_id": relation_id, + "csr": csr, + "certificate": stored_cert, + } + + if not self.process_certs: + return Result( + ResultType.SKIPPED, + "No outstanding CSRs matched a stored certificate", + ) + return Result(ResultType.COMPLETED) diff --git a/sunbeam-python/tests/unit/sunbeam/commands/test_refresh.py b/sunbeam-python/tests/unit/sunbeam/commands/test_refresh.py index 9caadf25e..15321f29d 100644 --- a/sunbeam-python/tests/unit/sunbeam/commands/test_refresh.py +++ b/sunbeam-python/tests/unit/sunbeam/commands/test_refresh.py @@ -10,6 +10,7 @@ from sunbeam.clusterd.service import ManifestItemNotFoundException from sunbeam.commands.refresh import _stored_manifest_risk, refresh from sunbeam.core.common import RiskLevel +from sunbeam.features.tls.ca import CaTlsFeature # --------------------------------------------------------------------------- # Helpers @@ -430,3 +431,58 @@ def test_error_when_manifest_and_clear_manifest_both_given(self, mocker, tmp_pat assert result.exit_code != 0 assert "mutually exclusive" in result.output.lower() + + +class TestRefreshFixTlsCerts: + """Tests for the automatic tls.ca certificate re-provision on refresh.""" + + def _invoke(self, args, deployment): + runner = CliRunner() + return runner.invoke(refresh, args, obj=deployment) + + def _deployment_with_feature(self, mocker, feature): + mocker.patch( + "sunbeam.commands.refresh.infer_risk", + return_value=RiskLevel.STABLE, + ) + client = _build_client("stable") + deployment = _build_deployment(client) + deployment.get_feature_manager.return_value.resolve_feature.return_value = ( + feature + ) + return deployment + + def test_reapply_called_when_tls_ca_enabled(self, mocker): + feature = MagicMock(spec=CaTlsFeature) + feature.is_enabled.return_value = True + deployment = self._deployment_with_feature(mocker, feature) + + result = self._invoke([], deployment) + + assert result.exit_code == 0 + feature.reapply_certificates.assert_called_once() + assert "TLS certificate re-provision complete." in result.output + + @pytest.mark.parametrize( + "is_enabled,args", + [ + pytest.param(False, [], id="tls_ca_not_enabled"), + pytest.param(True, ["--upgrade-release"], id="upgrade_release"), + ], + ) + def test_reapply_not_called(self, mocker, is_enabled, args): + feature = MagicMock(spec=CaTlsFeature) + feature.is_enabled.return_value = is_enabled + deployment = self._deployment_with_feature(mocker, feature) + + result = self._invoke(args, deployment) + + assert result.exit_code == 0 + feature.reapply_certificates.assert_not_called() + + def test_skipped_when_tls_ca_feature_missing(self, mocker): + deployment = self._deployment_with_feature(mocker, None) + + result = self._invoke([], deployment) + + assert result.exit_code == 0 diff --git a/sunbeam-python/tests/unit/sunbeam/features/test_tls.py b/sunbeam-python/tests/unit/sunbeam/features/test_tls.py index ce475bcfe..0d2ea4966 100644 --- a/sunbeam-python/tests/unit/sunbeam/features/test_tls.py +++ b/sunbeam-python/tests/unit/sunbeam/features/test_tls.py @@ -546,6 +546,125 @@ def test_run_with_no_ca_chain(self, cclient, jhelper, step_context): assert result.result_type == ResultType.COMPLETED +class TestReapplyTLSCertificatesStep: + def _outstanding(self, jhelper, certs): + jhelper.run_action.return_value = { + "return-code": 0, + "result": json.dumps(certs), + } + + def test_has_no_prompts(self, cclient, jhelper): + step = tls.ReapplyTLSCertificatesStep(cclient, jhelper, "fake-cert") + assert step.has_prompts() is False + + def test_is_skip_when_no_outstanding_requests( + self, cclient, jhelper, step_context, load_answers + ): + self._outstanding(jhelper, []) + step = tls.ReapplyTLSCertificatesStep(cclient, jhelper, "fake-cert") + + result = step.is_skip(step_context) + + assert result.result_type == ResultType.SKIPPED + load_answers.assert_not_called() + + @pytest.mark.parametrize( + "stored,csr_subject", + [ + pytest.param({}, None, id="no_stored_certificates"), + pytest.param( + {"certificates": {"subject-old": {"certificate": "stored-cert"}}}, + "subject-new", + id="subject_mismatch", + ), + ], + ) + def test_is_skip_when_no_cert_can_be_matched( + self, + cclient, + jhelper, + step_context, + load_answers, + get_subject_from_csr, + stored, + csr_subject, + ): + self._outstanding( + jhelper, [{"unit_name": "traefik/0", "csr": "csr", "relation_id": 1}] + ) + load_answers.return_value = stored + get_subject_from_csr.return_value = csr_subject + jhelper.get_relation_map.return_value = {"certificates:1": "traefik"} + step = tls.ReapplyTLSCertificatesStep(cclient, jhelper, "fake-cert") + + result = step.is_skip(step_context) + + assert result.result_type == ResultType.SKIPPED + assert step.process_certs == {} + + def test_is_skip_matches_stored_certificate( + self, cclient, jhelper, step_context, load_answers, get_subject_from_csr + ): + self._outstanding( + jhelper, [{"unit_name": "traefik/0", "csr": "csr", "relation_id": 1}] + ) + get_subject_from_csr.return_value = "subject1" + load_answers.return_value = { + "certificates": {"subject1": {"certificate": "stored-cert"}} + } + jhelper.get_relation_map.return_value = {"certificates:1": "traefik"} + step = tls.ReapplyTLSCertificatesStep(cclient, jhelper, "fake-cert") + + result = step.is_skip(step_context) + + assert result.result_type == ResultType.COMPLETED + assert step.process_certs == { + "subject1": { + "app": "traefik", + "unit": "traefik/0", + "relation_id": 1, + "csr": "csr", + "certificate": "stored-cert", + } + } + + def test_is_skip_when_action_returns_failed_return_code( + self, cclient, jhelper, step_context, load_answers + ): + jhelper.run_action.return_value = {"return-code": 2} + step = tls.ReapplyTLSCertificatesStep(cclient, jhelper, "fake-cert") + + result = step.is_skip(step_context) + + assert result.result_type == ResultType.FAILED + load_answers.assert_not_called() + + def test_run_reprovides_matched_certificate( + self, cclient, jhelper, step_context, load_answers, get_subject_from_csr + ): + self._outstanding( + jhelper, [{"unit_name": "traefik/0", "csr": "csr", "relation_id": 1}] + ) + get_subject_from_csr.return_value = "subject1" + load_answers.return_value = { + "certificates": {"subject1": {"certificate": "stored-cert"}} + } + jhelper.get_relation_map.return_value = {"certificates:1": "traefik"} + step = tls.ReapplyTLSCertificatesStep(cclient, jhelper, "fake-cert") + + assert step.is_skip(step_context).result_type == ResultType.COMPLETED + + jhelper.run_action.return_value = {"return-code": 0} + result = step.run(step_context) + + assert result.result_type == ResultType.COMPLETED + action_name = jhelper.run_action.call_args.args[2] + action_params = jhelper.run_action.call_args.args[3] + assert action_name == "provide-certificate" + assert action_params["certificate"] == "stored-cert" + assert action_params["relation-id"] == 1 + + class FakeUnit: def __init__(self, status, message): class WorkloadStatus: From 7d5d03d5272019cb7ab659d7181fec930b175fb1 Mon Sep 17 00:00:00 2001 From: tphan025 Date: Thu, 16 Jul 2026 19:57:35 +0200 Subject: [PATCH 2/3] check certs for common_name --- .../sunbeam/features/interface/utils.py | 47 +++++++++++++++++++ sunbeam-python/sunbeam/features/tls/common.py | 38 ++++++++++++--- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/sunbeam-python/sunbeam/features/interface/utils.py b/sunbeam-python/sunbeam/features/interface/utils.py index 48e9bc4db..fcace4059 100644 --- a/sunbeam-python/sunbeam/features/interface/utils.py +++ b/sunbeam-python/sunbeam/features/interface/utils.py @@ -3,6 +3,7 @@ import base64 import binascii +import datetime import logging import re import typing @@ -158,6 +159,52 @@ def get_subject_from_csr(csr: str) -> str | None: return None +def get_cn_from_csr(csr: str) -> str | None: + """Return the Common Name from a PEM-encoded CSR string, or None.""" + try: + req = x509.load_pem_x509_csr(bytes(csr, "utf-8")) + cn_attrs = req.subject.get_attributes_for_oid(x509_oid.NameOID.COMMON_NAME) + if not cn_attrs: + return None + return str(cn_attrs[0].value) + except (binascii.Error, TypeError, ValueError) as e: + LOG.debug("Failed to get CN from CSR: %r", e) + return None + + +def get_cn_from_cert(certificate: str) -> str | None: + """Return the Common Name from a base64-encoded PEM certificate, or None.""" + try: + certificate_bytes = base64.b64decode(certificate) + cert = x509.load_pem_x509_certificate(certificate_bytes) + cn_attrs = cert.subject.get_attributes_for_oid(x509_oid.NameOID.COMMON_NAME) + if not cn_attrs: + return None + return str(cn_attrs[0].value) + except (binascii.Error, TypeError, ValueError) as e: + LOG.debug("Failed to get CN from certificate: %r", e) + return None + + +def is_cert_expired(certificate: str) -> bool: + """Return True if the base64-encoded PEM certificate has expired. + + Returns True (treat as expired) if the certificate cannot be decoded. + """ + try: + certificate_bytes = base64.b64decode(certificate) + cert = x509.load_pem_x509_certificate(certificate_bytes) + try: + expiry = cert.not_valid_after_utc + except AttributeError: + # cryptography < 42 returns a naive datetime; assume UTC + expiry = cert.not_valid_after.replace(tzinfo=datetime.timezone.utc) + return expiry < datetime.datetime.now(datetime.timezone.utc) + except (binascii.Error, TypeError, ValueError) as e: + LOG.debug("Failed to check certificate expiry: %r", e) + return True + + def encode_base64_as_string(data: str) -> str | None: try: return base64.b64encode(bytes(data, "utf-8")).decode() diff --git a/sunbeam-python/sunbeam/features/tls/common.py b/sunbeam-python/sunbeam/features/tls/common.py index b10be7a68..8a5c206a1 100644 --- a/sunbeam-python/sunbeam/features/tls/common.py +++ b/sunbeam-python/sunbeam/features/tls/common.py @@ -40,7 +40,10 @@ from sunbeam.features.interface.utils import ( encode_base64_as_string, generate_ca_chain, + get_cn_from_cert, + get_cn_from_csr, get_subject_from_csr, + is_cert_expired, is_certificate_valid, normalize_pem, ) @@ -791,22 +794,43 @@ def is_skip(self, context: StepContext) -> Result: elif unit_name: app = unit_name.split("/")[0] - subject = get_subject_from_csr(csr) - if not subject: - LOG.warning("Skipping invalid CSR for unit %s", unit_name) + cn = get_cn_from_csr(csr) + if not cn: + LOG.warning("Skipping CSR with no CN for unit %s", unit_name) continue - stored_cert = stored_certs.get(subject, {}).get("certificate") + # Find a stored certificate whose CN matches the CSR's CN + # and that has not yet expired. + stored_cert = None + for stored_data in stored_certs.values(): + cert_b64 = stored_data.get("certificate") + if cert_b64 and get_cn_from_cert(cert_b64) == cn: + if is_cert_expired(cert_b64): + LOG.warning( + "Stored certificate for CN %s has expired; skipping.", + cn, + ) + continue + stored_cert = cert_b64 + break + if not stored_cert: LOG.warning( - "No stored certificate for outstanding CSR (app=%s, subject=%s); " + "No stored certificate for outstanding CSR (app=%s, cn=%s); " "skipping. Sign it with `sunbeam tls ca unit_certs`.", app, - subject, + cn, ) continue + + LOG.debug( + "Re-providing stored certificate for outstanding CSR (app=%s, cn=%s, cert=%s)", + app, + cn, + stored_cert, + ) - self.process_certs[subject] = { + self.process_certs[cn] = { "app": app, "unit": unit_name, "relation_id": relation_id, From 7b2e9c61dda879eb2c8f12df98eb34d6f7def065 Mon Sep 17 00:00:00 2001 From: tphan025 Date: Fri, 17 Jul 2026 11:59:42 +0200 Subject: [PATCH 3/3] update condition to provide --- .../sunbeam/features/interface/utils.py | 22 +++++++++++++ sunbeam-python/sunbeam/features/tls/common.py | 32 +++++++++++++------ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/sunbeam-python/sunbeam/features/interface/utils.py b/sunbeam-python/sunbeam/features/interface/utils.py index fcace4059..2afd981f5 100644 --- a/sunbeam-python/sunbeam/features/interface/utils.py +++ b/sunbeam-python/sunbeam/features/interface/utils.py @@ -221,6 +221,28 @@ def decode_base64_as_string(data: str) -> str | None: return None +def cert_and_csr_public_key_match(cert_b64: str, csr: str) -> bool: + """Return True if the public key in the certificate matches the one in the CSR. + + Returns False if either cannot be decoded. + """ + try: + cert = x509.load_pem_x509_certificate(base64.b64decode(cert_b64)) + req = x509.load_pem_x509_csr(bytes(csr, "utf-8")) + cert_pub = cert.public_key() + csr_pub = req.public_key() + if hasattr(cert_pub, "public_numbers") and hasattr(csr_pub, "public_numbers"): + return cert_pub.public_numbers() == csr_pub.public_numbers() + if hasattr(cert_pub, "public_bytes_raw") and hasattr( + csr_pub, "public_bytes_raw" + ): + return cert_pub.public_bytes_raw() == csr_pub.public_bytes_raw() + return False + except (binascii.Error, TypeError, ValueError) as e: + LOG.debug("Failed to compare public keys: %r", e) + return False + + def cert_and_key_match(certificate: bytes, key: bytes) -> bool: """Checks if the supplied cert is derived from the supplied key.""" crt = x509.load_pem_x509_certificate(certificate, backends.default_backend()) diff --git a/sunbeam-python/sunbeam/features/tls/common.py b/sunbeam-python/sunbeam/features/tls/common.py index 8a5c206a1..4eee474a8 100644 --- a/sunbeam-python/sunbeam/features/tls/common.py +++ b/sunbeam-python/sunbeam/features/tls/common.py @@ -38,6 +38,7 @@ ) from sunbeam.core.openstack import OPENSTACK_MODEL from sunbeam.features.interface.utils import ( + cert_and_csr_public_key_match, encode_base64_as_string, generate_ca_chain, get_cn_from_cert, @@ -799,20 +800,31 @@ def is_skip(self, context: StepContext) -> Result: LOG.warning("Skipping CSR with no CN for unit %s", unit_name) continue - # Find a stored certificate whose CN matches the CSR's CN + # Find a stored certificate whose CN and public key match the CSR # and that has not yet expired. stored_cert = None for stored_data in stored_certs.values(): cert_b64 = stored_data.get("certificate") - if cert_b64 and get_cn_from_cert(cert_b64) == cn: - if is_cert_expired(cert_b64): - LOG.warning( - "Stored certificate for CN %s has expired; skipping.", - cn, - ) - continue - stored_cert = cert_b64 - break + if not cert_b64: + continue + if get_cn_from_cert(cert_b64) != cn: + continue + if not cert_and_csr_public_key_match(cert_b64, csr): + LOG.warning( + "Stored certificate CN matches but public key differs " + "for CSR (app=%s, cn=%s); skipping.", + app, + cn, + ) + continue + if is_cert_expired(cert_b64): + LOG.warning( + "Stored certificate for CN %s has expired; skipping.", + cn, + ) + continue + stored_cert = cert_b64 + break if not stored_cert: LOG.warning(