From 48bec879c67a94725ea5d8c5e3a4edec6cc57551 Mon Sep 17 00:00:00 2001 From: Sam Batschelet Date: Tue, 18 Aug 2026 17:26:15 -0400 Subject: [PATCH] fix(operator): stop advertised cert data removing a site from routing The gateway probe verified the live leaf against the configured pins, then compared the SWIM-advertised copy of that certificate to the same pins and demoted the site on mismatch. The live handshake is the trust authority, so a stale or unparseable gossiped copy could remove an authenticated peer from routing, and grid_network.rs requires exactly Active for eligibility. Record the mismatch instead of acting on it. The site stays Active and keeps its reason, so the condition is visible without changing eligibility. Since the reason now comes from the success path, map it to a Normal event: a Warning on a site that is verified and routing reads as actionable when it is not. Extract advertised_leaf_der so the parse is testable and so an unparseable gossiped PEM is ignored rather than short-circuiting the probe before the handshake that authenticates the peer. Cover the wrapper for every probe-eligible starting phase, matching what the probe tests already do, and correct the tls_probe wording that still described the advertised certificate as authorization material. Reported by @nerdalert. Signed-off-by: Sam Batschelet --- docs/architecture/crds.md | 2 +- docs/architecture/operations.md | 23 +++-- operator/src/controller/grid_site.rs | 127 ++++++++++++++++++++++-- operator/src/resources/gateway_probe.rs | 34 +++++-- operator/src/resources/tls_probe.rs | 7 +- xtask/src/env/mod.rs | 9 +- 6 files changed, 160 insertions(+), 42 deletions(-) diff --git a/docs/architecture/crds.md b/docs/architecture/crds.md index 3edf55f..aa68a4a 100644 --- a/docs/architecture/crds.md +++ b/docs/architecture/crds.md @@ -322,7 +322,7 @@ A discovered SWIM peer is not automatically authorized for routing. | `IdentityMismatch` | Connecting | Server SAN does not match configured `serverName` | | `CertificateExpired` / `CertificateNotYetValid` | Connecting | Server certificate is outside its validity period | | `PinMismatch` | Connecting | Canonical fingerprint does not match a configured pin | -| `AdvertisedCertMismatch` | Connecting | SWIM-advertised certificate does not match either configured rotation pin | +| `AdvertisedCertMismatch` | Active | SWIM-advertised certificate does not match a configured pin; recorded only, since the live leaf verified | | `TrustMaterialMissing` | Connecting | CA, client certificate, key, server name, or pin policy is absent | | `TrustMaterialInvalid` | Connecting | Trust material is malformed, oversized, or uses the deprecated fingerprint format | diff --git a/docs/architecture/operations.md b/docs/architecture/operations.md index d6b8127..7c04536 100644 --- a/docs/architecture/operations.md +++ b/docs/architecture/operations.md @@ -523,9 +523,8 @@ The trust bootstrap for a remote site progresses through these steps: 5. **Identity-aware gateway probe passes** — the `GridSite` controller performs a bounded mTLS handshake. It verifies the CA chain, DNS SAN, client - authentication, canonical pin, and agreement with the SWIM-advertised leaf - certificate. Success promotes the site to `Active` with reason - `TlsVerified`. + authentication, and the canonical pin against the live leaf certificate. + Success promotes the site to `Active` with reason `TlsVerified`. ```yaml spec: @@ -594,7 +593,7 @@ The `GridSite` controller verifies gateway reachability and identity against |-----------|---------------| | `SWIMReachable` | SWIM membership reports the peer Alive | | `GatewayAddressKnown` | `spec.egress.address` is non-empty | -| `TlsVerified` | Mutual TLS handshake, chain, SAN, pin, and advertised leaf all verify | +| `TlsVerified` | Mutual TLS handshake, chain, SAN, and live-leaf pin all verify | | `IdentityVerificationRequired` | Plaintext endpoint accepts TCP, but remains ineligible because its identity is not verified | Request-time authorization remains enforced by the provider gateway after the @@ -870,10 +869,14 @@ arbitrary or ambiguous advertised strings do not become routable endpoints. **Probe behavior:** In Mutual mode, the `GridSite` controller performs a bounded mTLS connection to `spec.egress.address`. It verifies the configured CA, -`serverName`, canonical live-certificate pin, and that any SWIM-advertised -certificate matches one of the configured rotation pins. A successful probe -reports `reason: TlsVerified`. Connection failures move an Active site to -`Unreachable`; identity or trust failures move it to `Connecting`. +`serverName`, and the canonical live-certificate pin. A successful probe reports +`reason: TlsVerified`. Connection failures move an Active site to `Unreachable`; +identity or trust failures move it to `Connecting`. + +A SWIM-advertised certificate that does not match a configured pin is recorded +as `reason: AdvertisedCertMismatch` and does not change the phase: it arrives +over gossip and is not trust material. Trust comes from the handshake above, +where a pin mismatch on the live leaf is `PinMismatch` and still demotes. Explicit `Plaintext` mode performs only a bounded TCP connection for diagnostics. It never promotes a site to `Active` and is never selected as a @@ -943,8 +946,8 @@ separate steps. configured CA/server name. - `PinMismatch`: the live leaf certificate does not match either configured canonical pin. - - `AdvertisedCertMismatch`: wait for certificate gossip to converge or - investigate an unexpected live gateway identity. + - `AdvertisedCertMismatch`: the gossiped certificate copy does not match a + configured pin. Diagnostic, the site stays Active. - `HandshakeTimeout` or `TlsProtocolError`: the TCP endpoint answered but did not complete the expected TLS protocol. diff --git a/operator/src/controller/grid_site.rs b/operator/src/controller/grid_site.rs index e09d41f..c8a0e8e 100644 --- a/operator/src/controller/grid_site.rs +++ b/operator/src/controller/grid_site.rs @@ -238,6 +238,26 @@ async fn evaluate_gateway(site: &GridSite, client: &Client, network: &GridNetwor } } +/// SWIM-advertised leaf DER, or `None` when absent or unparseable. +/// +/// Gossiped, so unparseable is ignored: an `Err` would skip the authenticating handshake. +fn advertised_leaf_der(site: &GridSite) -> Option> { + site.status + .as_ref() + .and_then(|s| s.public_cert_pem.as_ref()) + .and_then(|pem| match first_cert_der_from_pem(pem) { + Ok(der) => Some(der), + Err(e) => { + tracing::warn!( + error = %e, + site = site.metadata.name.as_deref().unwrap_or_default(), + "ignoring unparseable advertised certificate" + ); + None + }, + }) +} + /// Build a `ProbeConfig` by loading trust material from Kubernetes /// Secrets referenced by the `GridNetwork`. /// @@ -302,19 +322,14 @@ async fn build_probe_config_from_secrets( let pins = resolve_pins(site)?; - let advertised_leaf_der = site - .status - .as_ref() - .and_then(|s| s.public_cert_pem.as_ref()) - .map(|pem| first_cert_der_from_pem(pem).map_err(|_err| O::TrustMaterialInvalid)) - .transpose()?; + let advertised = advertised_leaf_der(site); Ok(crate::resources::tls_probe::ProbeConfig { address: addr.to_owned(), tls_config, server_name, pins, - advertised_leaf_der, + advertised_leaf_der: advertised, }) } @@ -525,7 +540,10 @@ async fn update_status( /// [`Warning`]: EventType::Warning fn event_type_for_reason(reason: &str) -> EventType { match reason { - "TlsVerified" | "AwaitingDiscovery" | "GatewayAddressKnown" | "Left" => EventType::Normal, + // AdvertisedCertMismatch is a success path, so Warning would be false alarm. + "TlsVerified" | "AwaitingDiscovery" | "GatewayAddressKnown" | "Left" | "AdvertisedCertMismatch" => { + EventType::Normal + }, _ => EventType::Warning, } } @@ -977,15 +995,96 @@ mod tests { assert_eq!(reason, "PinMismatch"); } + /// A site whose only relevant property is its advertised certificate. + fn site_with_advertised(pem: Option<&str>) -> GridSite { + GridSite { + status: Some(GridSiteStatus { + public_cert_pem: pem.map(ToOwned::to_owned), + ..Default::default() + }), + ..site_no_egress(None) + } + } + + /// Absent advertised material yields no DER, and no error. + #[test] + fn advertised_leaf_absent_is_none() { + assert!(advertised_leaf_der(&site_no_egress(None)).is_none(), "no status"); + assert!( + advertised_leaf_der(&site_with_advertised(None)).is_none(), + "status, no PEM" + ); + } + + /// A real advertised certificate parses to the expected DER. + #[test] + fn advertised_leaf_valid_is_parsed() { + let ca = certs::generate_ca("t").unwrap_or_else(|_| std::process::abort()); + let leaf = certs::generate_site_cert(&ca, "peer").unwrap_or_else(|_| std::process::abort()); + let want = first_cert_der_from_pem(&leaf.cert_pem).unwrap_or_else(|_| std::process::abort()); + assert_eq!( + advertised_leaf_der(&site_with_advertised(Some(&leaf.cert_pem))), + Some(want) + ); + } + + /// A chain PEM yields the leaf, not an intermediate. + #[test] + fn advertised_leaf_of_chain_is_the_leaf() { + let ca = certs::generate_ca("t").unwrap_or_else(|_| std::process::abort()); + let leaf = certs::generate_site_cert(&ca, "peer").unwrap_or_else(|_| std::process::abort()); + let chain = format!("{}{}", leaf.cert_pem, ca.cert_pem); + let want = first_cert_der_from_pem(&leaf.cert_pem).unwrap_or_else(|_| std::process::abort()); + assert_eq!(advertised_leaf_der(&site_with_advertised(Some(&chain))), Some(want)); + } + + /// An unparseable advertised PEM is ignored, not surfaced as an error. + #[test] + fn unparseable_advertised_leaf_is_ignored() { + let bad = "-----BEGIN CERTIFICATE-----\nMIIBIjANBgkqhkiG9\n-----END CERTIFICATE-----"; + assert!( + first_cert_der_from_pem(bad).is_err(), + "fixture must be the unparseable case this guards" + ); + assert!( + advertised_leaf_der(&site_with_advertised(Some(bad))).is_none(), + "unparseable advertised material must be ignored, never surfaced as an error" + ); + } + #[test] - fn advertised_cert_mismatch_demotes_active_to_connecting() { + fn advertised_cert_mismatch_promotes_connecting_to_active() { + let site = site_with_egress(Some(GridSitePhase::Connecting), "10.0.0.1:8443"); + let (phase, reason, _msg) = site_phase_next( + &GridSitePhase::Connecting, + &site, + Some(&GatewayProbeOutcome::AdvertisedCertificateMismatch), + ); + assert_eq!(phase, GridSitePhase::Active); + assert_eq!(reason, "AdvertisedCertMismatch"); + } + + #[test] + fn advertised_cert_mismatch_recovers_unreachable_to_active() { + let site = site_with_egress(Some(GridSitePhase::Unreachable), "10.0.0.1:8443"); + let (phase, reason, _msg) = site_phase_next( + &GridSitePhase::Unreachable, + &site, + Some(&GatewayProbeOutcome::AdvertisedCertificateMismatch), + ); + assert_eq!(phase, GridSitePhase::Active); + assert_eq!(reason, "AdvertisedCertMismatch"); + } + + #[test] + fn advertised_cert_mismatch_keeps_active() { let site = site_with_egress(Some(GridSitePhase::Active), "10.0.0.1:8443"); let (phase, reason, _msg) = site_phase_next( &GridSitePhase::Active, &site, Some(&GatewayProbeOutcome::AdvertisedCertificateMismatch), ); - assert_eq!(phase, GridSitePhase::Connecting); + assert_eq!(phase, GridSitePhase::Active); assert_eq!(reason, "AdvertisedCertMismatch"); } @@ -1374,6 +1473,14 @@ mod tests { assert!(matches!(event_type_for_reason("Left"), EventType::Normal)); } + #[test] + fn event_type_advertised_cert_mismatch_is_normal() { + assert!(matches!( + event_type_for_reason("AdvertisedCertMismatch"), + EventType::Normal + )); + } + #[test] fn event_type_pin_mismatch_is_warning() { assert!(matches!(event_type_for_reason("PinMismatch"), EventType::Warning)); diff --git a/operator/src/resources/gateway_probe.rs b/operator/src/resources/gateway_probe.rs index 29de717..a6522e7 100644 --- a/operator/src/resources/gateway_probe.rs +++ b/operator/src/resources/gateway_probe.rs @@ -160,9 +160,10 @@ pub(crate) fn probe_transition(current_phase: &GridSitePhase, outcome: &GatewayP "PinMismatch", "server cert fingerprint does not match any configured pin", ), - O::AdvertisedCertificateMismatch => trust_failure( + // Recorded, not acted on: the live leaf already matched a pin above. + O::AdvertisedCertificateMismatch => success( "AdvertisedCertMismatch", - "SWIM-advertised certificate does not match any configured pin", + "SWIM-advertised certificate does not match any configured pin; live leaf verified", ), } } @@ -470,32 +471,45 @@ mod tests { } #[test] - fn advertised_cert_mismatch_demotes_active_to_connecting() { + fn advertised_cert_mismatch_keeps_active() { let t = probe_transition( &GridSitePhase::Active, &GatewayProbeOutcome::AdvertisedCertificateMismatch, ); assert_eq!( t.phase, - GridSitePhase::Connecting, - "advertised cert mismatch must demote Active to Connecting" + GridSitePhase::Active, + "the live leaf already matched a pin; the advertised copy must not demote" ); assert_eq!( t.reason, "AdvertisedCertMismatch", - "reason must be AdvertisedCertMismatch" + "the mismatch is still recorded in the reason" + ); + } + + #[test] + fn advertised_cert_mismatch_recovers_from_unreachable() { + let t = probe_transition( + &GridSitePhase::Unreachable, + &GatewayProbeOutcome::AdvertisedCertificateMismatch, + ); + assert_eq!( + t.phase, + GridSitePhase::Active, + "the peer answered and verified, so it is no longer unreachable" ); } #[test] - fn advertised_cert_mismatch_stays_connecting() { + fn advertised_cert_mismatch_does_not_block_active() { let t = probe_transition( &GridSitePhase::Connecting, &GatewayProbeOutcome::AdvertisedCertificateMismatch, ); assert_eq!( t.phase, - GridSitePhase::Connecting, - "advertised cert mismatch must keep Connecting" + GridSitePhase::Active, + "reached only after chain, SAN, and live-leaf pin verified" ); assert_eq!( t.reason, "AdvertisedCertMismatch", @@ -574,7 +588,6 @@ mod tests { GatewayProbeOutcome::CertificateExpired, GatewayProbeOutcome::CertificateNotYetValid, GatewayProbeOutcome::PinMismatch, - GatewayProbeOutcome::AdvertisedCertificateMismatch, ]; for outcome in &trust_failures { let t = probe_transition(&GridSitePhase::Active, outcome); @@ -598,7 +611,6 @@ mod tests { GatewayProbeOutcome::CertificateExpired, GatewayProbeOutcome::CertificateNotYetValid, GatewayProbeOutcome::PinMismatch, - GatewayProbeOutcome::AdvertisedCertificateMismatch, ]; for outcome in &trust_failures { let t = probe_transition(&GridSitePhase::Connecting, outcome); diff --git a/operator/src/resources/tls_probe.rs b/operator/src/resources/tls_probe.rs index e54039c..6644ff6 100644 --- a/operator/src/resources/tls_probe.rs +++ b/operator/src/resources/tls_probe.rs @@ -64,8 +64,8 @@ pub(crate) struct ProbeConfig { /// Canonical DER fingerprint pins (1–2 entries). pub pins: Vec, - /// Optional SWIM-advertised leaf cert DER for authorization against the - /// configured rotation pins. + /// Optional SWIM-advertised leaf cert DER, compared with the configured + /// rotation pins for diagnostics only. pub advertised_leaf_der: Option>, } @@ -182,7 +182,8 @@ pub(crate) fn build_tls_config( /// 2. TLS handshake under [`PROBE_DEADLINE`] (total, including connect). /// 3. Extract peer leaf certificate DER. /// 4. Validate canonical fingerprint pin. -/// 5. If present, verify the SWIM-advertised leaf cert matches an authorized rotation pin. +/// 5. If present, compare the SWIM-advertised leaf with the pins and record a mismatch without failing the verified +/// connection. /// /// Returns a `GatewayProbeOutcome` — never panics, never leaks /// private material. diff --git a/xtask/src/env/mod.rs b/xtask/src/env/mod.rs index 99064e3..c3e0f18 100644 --- a/xtask/src/env/mod.rs +++ b/xtask/src/env/mod.rs @@ -6233,13 +6233,8 @@ fn env_verify_gridsite_rotation(config: &Path, site: Option<&str>) -> Result<(), ROTATION_POLL_TIMEOUT, )?; eprintln!(" [PASS] step 6b: TlsVerified — cert-B matches fp-B in dual-pin"); - // NOTE: AdvertisedCertificateMismatch (SWIM-advertised cert vs configured - // pins) requires modifying the remote operator's TLS broadcast secret - // independently of the probe server cert. This pathway has unit test - // coverage (advertised_cert_mismatch_demotes_active_to_connecting, - // advertised_cert_mismatch_stays_connecting) but no xtask verifier - // intentionally creates that runtime state. The rotation verifier - // focuses on the live TLS handshake lifecycle. + // No step for AdvertisedCertificateMismatch: it needs the remote TLS broadcast + // secret patched apart from the probe cert, and no longer changes phase. // ── Step 7: Patch [fp-B] only → still Active/TlsVerified ──────────── eprintln!("verify-gridsite-rotation: [7] single pin [fp-B]");