fix(colony)!: verify each control frame once

[sephynox]

Summary

Inbound colony control verified a signature and then resolved the signer again.
The second lookup is gone, so the plane comes from the certificate the check already proved.

Related Issues

None

Changes Made

• A control frame signature is checked once.
• A signer present in peer trust is a peer.
• Hive control from that signer is refused.

Testing

• cargo test-all: passes

Breaking Changes

• Use CertificateTrust::verify_frame for frame signatures.
• Import TrustVerification from crypto::x509::store.
• Drop frame_signer_cert and peer_signer_fingerprint.

---

Note

High Risk
Changes authentication, trust-plane classification, and certificate rotation behavior across colony control, gossip, and hive security gates—security-critical paths with breaking API moves.

Overview
Colony gateway control handling now verifies each inbound frame’s signature once and threads the result through gossip, peer ads, and hive registration via VerifiedControlFrame / VerifiedSignerId, replacing separate verify_*_origin checks and repeated signer lookups (frame_signer_cert, peer_signer_fingerprint, hive verify_frame_signature).

Trust plane membership is keyed by public key, not certificate object identity: TrustPlanes and hive/peer gates use CertificateTrust::trusts_public_key, so a key enrolled in peer_trust stays a peer (and cannot act on the hive plane) even when the presented cert differs from the enrolled one—e.g. rotation with distinct hive vs peer cert objects for the same key.

The X.509 layer centralizes frame checks on CertificateTrust::verify_frame, moves TrustVerification to crypto::x509::store (verified arm returns the resolved cert), adds find_by_signer_identifier, and fixes SKID/signer-id derivation to a single SHA3-256 protocol rule (Skid::of_public_key, non-generic CertificateTrustBuilder / compute_signer_identifier). Call sites and tests are updated accordingly; frame_signer_cert and peer_signer_fingerprint are removed from the public cluster API.

^{Reviewed by Cursor Bugbot for commit 67e629b. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 4678e76. Configure here.}

[cursor]

Stale comment

[cursor]

Stale comment

[cursor]

Stale comment

Config/Template Injection Review — review_module: config_template_reviewer

Scope: config objects, template objects, and code that constructs/mutates/serializes them.

No config/template injection issues found.

Checked:

• ClusterTlsConfig — no new fields; only doc-comment wording updated
• VerifiedControlFrame<'a> — short-lived, lifetime-scoped struct; not serialized or persisted into any config blob
• TrustVerification::Verified(&'a Certificate) — borrowed reference for the duration of frame handling only; not written to config
• Error paths — return TransitStatus enum variants; no file paths, stack traces, source code, or prompts flow into config
• trusts_public_key default impl — SPKI/SID key-identity lookup; no user-controlled code data enters config

{"findings": []}

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

Stale comment

{"findings": []}

"review_module": "agent_tooling_trust_boundary" — No agent tooling, LLM, MCP, prompt, or AI-related code is present in this diff. All changes are X.509 certificate trust store refactoring and cluster frame verification logic. No findings for this module.

The previously open thread about trusts_public_key defaulting to fingerprint-based identity has been fully addressed: the default implementation now computes a SignerIdentifier from the SPKI DER via compute_signer_identifier_from_der and resolves through find_by_signer_identifier, providing true key-identity semantics rather than certificate-fingerprint comparison.

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

Stale comment

Filesystem / Workspace Boundary review (filesystem_workspace_boundary)

{"findings": []}

All changed files are pure Rust cluster-authentication code (X.509 certificate trust, cluster frame verification, gossip/trust-plane refactoring). No filesystem path operations, workspace path handling, file extraction, or agent/MCP-controlled filesystem inputs are present anywhere in the diff. No filesystem/workspace boundary findings.

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

Stale comment

{"findings": [], "review_module": "privacy_guard"}

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

Stale comment

[cursor]

Stale comment

🔒 Agentic Security Review — platform_pattern_reviewer

Prior thread verdicts

Thread	Concern	Verdict
PRRT_kwDOQA8CD86kGEP-	TrustPlanes::classify used fingerprint identity (is_trusted) instead of key identity	Fully addressed — classify now calls trusts_public_key throughout; test peer_wins_when_same_key_has_distinct_certificates confirms rotated-cert key identity
PRRT_kwDOQA8CD86kGrFD	Default trusts_public_key silently fell back to is_trusted (cert fingerprint)	Partially addressed — the is_trusted fallback is gone; the default now calls compute_signer_identifier_from_der::<Sha3_256>. A new latent issue remains (see finding below)

---

Findings

{
  "findings": [
    {
      "review_module": "platform_pattern_reviewer",
      "severity": "medium",
      "location": "tightbeam/src/crypto/x509/store.rs:162-171",
      "description": "CertificateTrustStore does not override trusts_public_key. The default implementation hardcodes Sha3_256 for SKID computation, but CertificateTrustBuilder<D> indexes the skid_index using D::digest. When D != Sha3_256 the default trusts_public_key computes a Sha3_256-based SKID, finds no matching entry in the index, and silently returns false — breaking the peer-wins invariant in TrustPlanes::classify.",
      "impact": "A peer node whose key is enrolled in peer_trust under a CertificateTrustBuilder<D> (D ≠ Sha3_256) passes verify_hive because trusts_public_key returns false, classify returns Party::FirstParty, and the party == required guard in verify_plane succeeds. The node gains hive-plane control access (registration, spawn results) without being a legitimate hive member.",
      "verified": true,
      "attack_path": "Operator deploys peer_trust via CertificateTrustBuilder<D> where D ≠ Sha3_256 → skid_index keys are D-hashed → trusts_public_key queries with Sha3_256-hashed SKID → find_by_signer_identifier returns None → trusts_public_key false → TrustPlanes::classify skips peer check → Party::FirstParty → verify_hive passes → peer acts on hive plane.",
      "evidence": [
        "tightbeam/src/crypto/x509/store.rs:162-171 (default trusts_public_key, hardcoded Sha3_256)",
        "tightbeam/src/crypto/x509/store.rs:681 (add_certificate uses D::digest for skid_index)",
        "tightbeam/src/crypto/x509/store.rs:594-630 (CertificateTrustStore impl has no trusts_public_key override)"
      ],
      "remediation": "Either (a) remove the D type parameter from CertificateTrustBuilder and hardcode Sha3_256 — all production and test code already uses CertificateTrustBuilder<Sha3_256> — so the builder and the default always agree; or (b) override trusts_public_key in CertificateTrustStore to compute the SKID with the same algorithm used by add_certificate (requires storing a PhantomData<D> in CertificateTrustStore so the hash algorithm is recoverable at lookup time)."
    }
  ]
}

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

Stale comment

🔒 Agentic Security Review

Previous threads resolved: The earlier finding about trusts_public_key falling back to certificate-fingerprint identity is fully addressed in the current HEAD — the default implementation now uses find_by_signer_identifier (SKID-based key-identity lookup) rather than is_trusted (fingerprint). The peer-wins check correctly uses key identity across rotated certificates.

One new finding below (MEDIUM). All other focus areas checked clean:

• verify_hive/verify_peer correctly gate every handler; no code path processes a frame without verification.
• VerifiedControlFrame party classification result is never discarded before the required check.
• AdmittedPeerAd::admit now requires Party::Peer before accessing the signer cert — no regression.
• verify_frame inherits the same algorithm-OID/policy chain as the removed verify_frame_signature; policy rejects unknown OIDs.
• weaken_invalid_relay uses the pre-verified relay_id, not a re-resolved frame lookup — correct.
• frame_colony_urn, frame_signer_cert, peer_signer_fingerprint, verify_frame_signature, verify_hive_origin, verify_peer_origin have no remaining call-sites in the branch.
• No unsafe blocks introduced; no production-path panics (unwrap/expect are test-only).

{
  "findings": [
    {
      "review_module": "security_reviewer",
      "severity": "medium",
      "location": "tightbeam/src/crypto/x509/store.rs:162-171",
      "description": "The default `trusts_public_key` implementation on `CertificateTrust` hardcodes `Sha3_256` when computing the SKID used for lookup, but `CertificateTrustBuilder<D>` is generic and builds `skid_index` using `D::digest()`. For any store constructed with `D != Sha3_256`, the SKID bytes stored in the index differ from the Sha3_256 bytes the default generates, causing `find_by_signer_identifier` to always return `None` and `trusts_public_key` to always return `false`. This silently disables the peer-wins key-identity invariant this PR specifically introduces.",
      "impact": "A peer enrolled in `peer_trust` (built with `D != Sha3_256`) whose key also appears under a different certificate in `hive_trust` can send a hive-plane frame that passes `verify_hive` signature verification. The subsequent `TrustPlanes::classify` call invokes `peer_trust.trusts_public_key(cert)`, which returns `false` due to the SKID hash mismatch, so the peer is classified as `Party::FirstParty` rather than `Party::Peer`. The peer-wins check in `verify_plane` then passes, granting hive-plane access to an entity that should be confined to the peer plane.",
      "verified": true,
      "attack_path": "1. Deployment builds `peer_trust` via `CertificateTrustBuilder::<D>` where `D != Sha3_256` (any `Digest` impl — this is unconstrained public API). 2. Peer with key K is enrolled in that store (skid_index key = `D::digest(SPKI_K)[:20]`). 3. The same key K also appears in `hive_trust` under a distinct cert A. 4. Peer sends a hive-plane frame signed with K. 5. `verify_hive` → `hive_trust.verify_frame` succeeds (resolves cert A, verifies sig). 6. `TrustPlanes::classify(cert_A)` → `peer_trust.trusts_public_key(cert_A)` → default computes `Sha3_256(SPKI_K)[:20]`, queries `skid_index` which holds `D::digest(SPKI_K)[:20]` — bytes differ — returns `None` → `false`. 7. Classify falls through to `hive_trust.trusts_public_key(cert_A)` → `true` → `Party::FirstParty`. 8. `party == required (FirstParty)` check passes → `VerifiedControlFrame { party: FirstParty }` returned. 9. Hive-plane access granted to a peer.",
      "evidence": [
        "tightbeam/src/crypto/x509/store.rs:162-171 (default trusts_public_key hardcodes Sha3_256)",
        "tightbeam/src/crypto/x509/store.rs:682 (skid_index built with D::digest, not Sha3_256)",
        "tightbeam/src/colony/cluster/runtime/verify.rs:143-147 (verify_plane calls classify which calls trusts_public_key)"
      ],
      "remediation": "Either (a) constrain `CertificateTrustBuilder<D>` to `D = Sha3_256` (remove the generic parameter; all in-tree uses already pass `Sha3_256`), or (b) make `CertificateTrustStore` record which digest was used and expose it so `trusts_public_key` can generate a matching SKID, or (c) add a `#[doc(important)]` note to `CertificateTrustBuilder<D>` warning that `D` must equal `Sha3_256` for the default `trusts_public_key` to work, paired with a compile-time assertion or trait bound requiring `D: SameAs<Sha3_256>` where that check is feasible. Option (a) is simplest and lowest risk."
    }
  ]
}

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

Stale comment

Platform Pattern Review

Module: platform_pattern_reviewer

Findings

1 MEDIUM finding. No critical or high findings.

Resolved prior thread: The previous concern about trusts_public_key defaulting to is_trusted(cert) (fingerprint identity rather than key identity) is fully addressed in this HEAD. The default now correctly derives a SubjectKeyIdentifier-form SignerIdentifier by hashing the SPKI with SHA3-256 and delegates to find_by_signer_identifier — correctly catching the rotated-certificate peer-wins scenario the prior review flagged.

Patterns checked with no issues found:

• TLS bypass (danger_accept_invalid_certs, rejectUnauthorized): not present in changed code.
• Algorithm OID in verify_frame: signer_info.signature_algorithm.oid is attacker-controlled, but Secp256k1Policy.verify_signature explicitly rejects any OID other than SIGNER_ECDSA_WITH_SHA3_256 — no algorithm-confusion bypass is possible through the default policy.
• Certificate chain validation in verify_frame: no regression; certificates are validated at store-construction time by the builder, consistent with the previous verify_frame_signature function.
• SKID collision handling: same-SKID + same-fingerprint (same cert) is allowed; different-fingerprint is rejected. Correct.
• Timing side-channels: no new constant-time-sensitive comparisons introduced.

---

{
  "findings": [
    {
      "review_module": "platform_pattern_reviewer",
      "severity": "medium",
      "location": "tightbeam/src/crypto/x509/store.rs:162-170",
      "description": "The default `trusts_public_key` implementation hardcodes `Sha3_256` for the SKID lookup, but `CertificateTrustBuilder<D>` is generic over any `D: Digest` and populates `CertificateTrustStore::skid_index` using `D::digest(spki)[:20]`. Because `CertificateTrustStore` does not override `trusts_public_key`, it relies on the default, which queries the index with `Sha3_256(spki)[:20]`. When a store is built with `D ≠ Sha3_256`, every `trusts_public_key` call silently returns `false`, breaking the peer-wins invariant without any compile-time or runtime error.",
      "impact": "If an operator (or external consumer of the public `CertificateTrustBuilder<D>` API) constructs `peer_trust` with a non-`Sha3_256` digest builder, and a peer's key appears under distinct certificate objects in both `peer_trust` and `hive_trust`, `TrustPlanes::classify` will mis-classify the peer as `Party::FirstParty`. `verify_hive` returns a `VerifiedControlFrame` with `Party::FirstParty`, granting the peer hive-plane control access it should not have.",
      "verified": true,
      "attack_path": "1. Operator builds `peer_trust` with `CertificateTrustBuilder::<SomeOtherDigest>::from(policy)`. 2. Peer key K is enrolled in `peer_trust` (index keyed by `OtherDigest(SPKI)[:20]`) and also appears under a distinct cert in `hive_trust`. 3. Peer sends a hive-plane frame signed with K. 4. `verify_hive` calls `hive_trust.verify_frame` → resolves cert_A, signature verified. 5. `TrustPlanes::classify(cert_A)` calls `peer_trust.trusts_public_key(cert_A)` → default impl computes `Sha3_256(SPKI)[:20]`, looks up in SKID index keyed by `OtherDigest` → no match → returns `false`. 6. Falls through to `hive_trust.trusts_public_key(cert_A)` → `true` → `Party::FirstParty`. 7. `party == required` → `VerifiedControlFrame` returned. Peer has hive-plane access.",
      "evidence": [
        "tightbeam/src/crypto/x509/store.rs:162-170 (default trusts_public_key uses Sha3_256)",
        "tightbeam/src/crypto/x509/store.rs:681-683 (add_certificate uses D::digest)"
      ],
      "remediation": "Either (a) add a `CertificateTrustStore`-specific override of `trusts_public_key` that is generic over the same digest used at build time — requiring the type to carry `D` — or (b) seal the builder so only `Sha3_256` is accepted (e.g., `pub type CertificateTrustBuilder = CertificateTrustBuilderInner<Sha3_256>`), dropping the generic parameter from the public API. At minimum, document in the trait that `find_by_signer_identifier` must accept a `SubjectKeyIdentifier` produced by `Sha3_256(SPKI)[:20]`, and add a test that exercises a store built with a non-standard digest to make the mismatch visible."
    }
  ]
}

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

Stale comment

🔒 Agentic Security Review

Reviewed commit 1566c170e2d8cf925b5845fc884998c98c09daf6. One medium finding.

---

Prior threads resolved in this commit set

Thread	Concern	Status
PRRT_kwDOQA8CD86kGEP-	signer_is_peer fingerprint vs. key identity in verify_plane	Fully addressed — TrustPlanes::classify now calls trusts_public_key everywhere; the rotated-cert test (hive_origin_refuses_peer_key_under_rotated_certificate) demonstrates the fix.
PRRT_kwDOQA8CD86kFMJx	Origin publish requiring peer self-trust	Fully addressed — verify_peer on the self-signed rumor is now ok()-wrapped; a hive-only gateway produces None for rumor_signer and skips the local peer-ad apply without failing the flood.
PRRT_kwDOQA8CD86kGrFD	Default trusts_public_key fell back to is_trusted (fingerprint)	Partially addressed — the fingerprint fallback is gone; the default now delegates through find_by_signer_identifier. Residual risk: the default hardcodes Sha3_256 for SKID computation while CertificateTrustBuilder<D> is generic over D (see finding below).

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

Stale comment

🔒 Agentic Security Review — 2 findings (both MEDIUM)

Two latent API design concerns in the new CertificateTrust public surface. No active exploits against current in-tree code; both issues affect external deployers and future callers.

#	Severity	Location	Summary
1	MEDIUM	store.rs:166	trusts_public_key default hardcodes Sha3_256; builder is generic over D
2	MEDIUM	store.rs:133	TrustVerification and verify_frame are missing #[must_use]

All other areas checked clean across all review modules: gossip/peer handlers are correctly gated by verify_hive/verify_peer, party classifications are never silently discarded, relay scoring uses the pre-verified identity, all removed symbols (frame_signer_cert, peer_signer_fingerprint, frame_colony_urn) have no remaining call sites, and no privacy, config-injection, agent-tooling, or filesystem boundary issues were found.

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

🔒 Agentic Security Review (run 2 of 2)

{"findings": []}

All focus-area checks passed against HEAD commit 67e629b:

1. SKID consistency — CertificateTrustBuilder::add_certificate uses Skid::of_public_key (SHA3-256 truncated to 20 bytes). trusts_public_key calls compute_signer_identifier_from_der which also uses Skid::of_public_key. Fully consistent; one protocol constant (SkidDigest = Sha3_256) governs both paths.

2. find_by_signer_identifier round-trip — compute_signer_identifier_from_der produces an OctetString from exactly 20 bytes (skid.as_bytes()). Skid::parse(skid.0.as_bytes()) recovers those same 20 bytes. The round-trip is exact and Skid::parse always succeeds for store-generated identifiers.

3. Gossip handler / VerifiedControlFrame propagation — Every gossip entry point (relay_gossip, publish, request_reconcile) calls verify_peer or verify_hive before acting on any signer identity. The relay path performs two independent verify_peer calls (one for the outer relay frame, one for the inner rumor), each returning a VerifiedControlFrame that cannot be forged or transposed.

4. Hive gate — ClusterSecurityGate::admit was never responsible for the peer-wins check; that responsibility is correctly placed at ClusterConfig::verify_hive → verify_plane → TrustPlanes::classify. The gate change from verify_frame_signature to trust_store.verify_frame is behavior-preserving and correct.

5. TOCTOU — hive_trust and peer_trust are Arc<dyn CertificateTrust> sealed at construction. No mutation path exists between verify_frame returning signer_cert and TrustPlanes::classify consuming it; the ordering is a safe same-thread sequence.

6. Error-path information leakage — TrustVerification::Invalid returns no key material. SkidCollision exposes only the 20-byte hex SKID, which is derived from the public key and is public information by design.

Previous findings resolved in this HEAD: the CertificateTrustBuilder<D> generic-parameter SKID-mismatch (all five threads) is eliminated by making the builder non-generic; the missing #[must_use] on TrustVerification, verify_frame, and trusts_public_key is addressed with matching attributes in this commit.

  

_{Sent by Cursor Security Agent: Security Reviewer}