fix(tee): fail closed on empty TCB-status allowlist in TEE admission

crates/context/src/handlers/admit_tee_node.rs

@@ -136,9 +136,19 @@ impl Handler<AdmitTeeNodeRequest> for ContextManager {
         if !policy.allowed_mrtd.iter().any(|a| a == &mrtd) {
             return ActorResponse::reply(Err(eyre::eyre!("MRTD not in policy allowlist")));
         }
-        if !policy.allowed_tcb_statuses.is_empty()
-            && !policy.allowed_tcb_statuses.iter().any(|a| a == &tcb_status)
-        {
+        // Fail-closed TCB-status gate (audit #356 / #17). An empty
+        // `allowed_tcb_statuses` no longer skips the check: it enforces against
+        // the secure default `{UpToDate}`. `Revoked` is rejected
+        // unconditionally. Mock (`is_mock`) bypasses the allowlist only when the
+        // policy sets `accept_mock` — defense-in-depth alongside the explicit
+        // `is_mock && !accept_mock` rejection above. Shared with the op-apply
+        // path (`governance_store::membership::tcb_status_allowed`).
+        if !calimero_governance_store::tcb_status_allowed(
+            &policy.allowed_tcb_statuses,
+            &tcb_status,
+            is_mock,
+            policy.accept_mock,
+        ) {
             return ActorResponse::reply(Err(eyre::eyre!("TCB status not in policy allowlist")));
         }
         if !policy.allowed_rtmr0.is_empty() && !policy.allowed_rtmr0.iter().any(|a| a == &rtmr0) {

crates/governance-store/src/lib.rs

@@ -94,7 +94,10 @@ pub use self::local_state::{
     track_member_context_join,
 };
 pub use self::membership::MembershipRepository;
-pub use self::membership::{GroupMembershipView, MembershipPath, MembershipPolicy};
+pub use self::membership::{
+    tcb_status_allowed, GroupMembershipView, MembershipPath, MembershipPolicy,
+    DEFAULT_ALLOWED_TCB_STATUS, TCB_STATUS_MOCK, TCB_STATUS_REVOKED,
+};
 pub use self::meta::MetaRepository;
 
 pub use self::metadata::MetadataRepository;

crates/governance-store/src/membership/mod.rs

@@ -19,6 +19,9 @@ mod tests;
 
 pub use self::core::{MembershipPath, MembershipRepository};
 pub use self::policy::MembershipPolicy;
-pub use self::policy_rules::TeeAttestationClaims;
+pub use self::policy_rules::{
+    tcb_status_allowed, TeeAttestationClaims, DEFAULT_ALLOWED_TCB_STATUS, TCB_STATUS_MOCK,
+    TCB_STATUS_REVOKED,
+};
 pub(crate) use self::status::role_from_invited_role;
 pub use self::view::GroupMembershipView;

crates/governance-store/src/membership/policy.rs

@@ -129,6 +129,7 @@ impl<'a> MembershipPolicy<'a> {
             allowed_rtmr2: policy.allowed_rtmr2.clone(),
             allowed_rtmr3: policy.allowed_rtmr3.clone(),
             allowed_tcb_statuses: policy.allowed_tcb_statuses.clone(),
+            accept_mock: policy.accept_mock,
         };
         if let Err(err) = validate_tee_attestation_allowlists(&normalized_policy, fields) {
             let reason = match err.reason() {

crates/governance-store/src/membership/policy_rules.rs

@@ -1,3 +1,57 @@
+/// Secure fail-closed default enforced when a policy's `allowed_tcb_statuses`
+/// is empty. An empty allowlist must NOT skip the TCB-status check (that was a
+/// fail-open hole — audit findings #356 / #17): instead it enforces against
+/// this single status. Must be the exact PascalCase value dcap-qvl emits
+/// (`crates/tee-attestation`) and matches the mero-tee KMS key-delivery gate,
+/// keeping "admitted ⟹ can get key" consistent. Do not broaden.
+pub const DEFAULT_ALLOWED_TCB_STATUS: &str = "UpToDate";
+
+/// dcap-qvl TCB status that is rejected unconditionally, regardless of policy
+/// (defense-in-depth). Real verification already bails on `Revoked`, but the
+/// subgroup-reuse admission path reuses a STORED tcb_status string without
+/// re-running verify — this guards that path.
+pub const TCB_STATUS_REVOKED: &str = "Revoked";
+
+/// Mock-attestation TCB status (set by `calimero_tee_attestation` mock verify).
+/// Real dcap-qvl never emits this value, so it uniquely identifies the mock
+/// path. Mock admission is gated upstream by `accept_mock`, so the TCB
+/// allowlist must not apply to it.
+pub const TCB_STATUS_MOCK: &str = "Mock";
+
+/// Shared TCB-status gate used by every `allowed_tcb_statuses` enforcement site.
+///
+/// Rules (in order):
+/// 1. `Revoked` (case-insensitive) → always rejected, even for the stored-status
+///    subgroup-reuse path that does not re-run dcap-qvl verify.
+/// 2. Mock path → allowed **only when the group policy sets `accept_mock`**. The
+///    mock signal is either the explicit runtime `is_mock` flag (admit_tee_node)
+///    or the reserved `"Mock"` status that carries it on the op-apply /
+///    subgroup-reuse path (which has `is_mock = false`). Gating both on
+///    `accept_mock` means a stored `"Mock"` status replayed onto a real fleet
+///    (`accept_mock = false`) is rejected instead of being a permanent bypass
+///    token — it no longer relies solely on the upstream admission gate
+///    (audit follow-up to #356 / #17).
+/// 3. Empty allowlist → fail-closed: enforce against the secure default
+///    [`DEFAULT_ALLOWED_TCB_STATUS`] instead of skipping the check.
+/// 4. Non-empty allowlist → honored exactly (case-sensitive, as today).
+pub fn tcb_status_allowed(
+    allowed_tcb_statuses: &[String],
+    tcb_status: &str,
+    is_mock: bool,
+    accept_mock: bool,
+) -> bool {
+    if tcb_status.eq_ignore_ascii_case(TCB_STATUS_REVOKED) {
+        return false;
+    }
+    if (is_mock || tcb_status == TCB_STATUS_MOCK) && accept_mock {
+        return true;
+    }
+    if allowed_tcb_statuses.is_empty() {
+        return tcb_status == DEFAULT_ALLOWED_TCB_STATUS;
+    }
+    allowed_tcb_statuses.iter().any(|a| a == tcb_status)
+}
+
 pub const TEE_REJECT_MRTD: &str = "mrtd_not_allowed";
 pub const TEE_REJECT_TCB_STATUS: &str = "tcb_status_not_allowed";
 pub const TEE_REJECT_RTMR0: &str = "rtmr0_not_allowed";
@@ -67,6 +121,10 @@ pub struct TeeAllowlistPolicy {
     pub allowed_rtmr2: Vec<String>,
     pub allowed_rtmr3: Vec<String>,
     pub allowed_tcb_statuses: Vec<String>,
+    /// Whether this group accepts mock attestations. Gates the mock TCB bypass
+    /// on the op-apply path so a stored `"Mock"` status only passes on a fleet
+    /// that actually opted into mock. See [`tcb_status_allowed`].
+    pub accept_mock: bool,
 }
 
 pub struct TeeAttestationClaims<'a> {
@@ -88,12 +146,18 @@ pub fn validate_tee_attestation_allowlists(
         });
     }
 
-    if !policy.allowed_tcb_statuses.is_empty()
-        && !policy
-            .allowed_tcb_statuses
-            .iter()
-            .any(|a| a == fields.tcb_status)
-    {
+    // Fail-closed TCB-status gate (shared with `admit_tee_node`). This is the
+    // op-apply path: it runs on every node replicating the op and has no
+    // explicit `is_mock` flag, so mock is detected via the reserved "Mock"
+    // status inside `tcb_status_allowed`. That mock bypass is in turn gated on
+    // the group's stored `accept_mock`, so a replayed `"Mock"` status cannot
+    // bypass the gate on a fleet that did not opt into mock.
+    if !tcb_status_allowed(
+        &policy.allowed_tcb_statuses,
+        fields.tcb_status,
+        false,
+        policy.accept_mock,
+    ) {
         return Err(MembershipPolicyValidationError {
             reason: MembershipPolicyRejection::TcbStatusNotAllowed,
         });

crates/governance-store/src/membership/tests.rs

@@ -250,6 +250,7 @@ fn membership_policy_rules_report_rejection_reasons() {
         allowed_rtmr2: vec![],
         allowed_rtmr3: vec![],
         allowed_tcb_statuses: vec!["ok".to_owned()],
+        accept_mock: false,
     };
     let claims = TeeAttestationClaims {
         mrtd: "m-ok",
@@ -284,6 +285,160 @@ fn membership_policy_rules_report_rejection_reasons() {
     assert_eq!(err.reason(), MembershipPolicyRejection::TcbStatusNotAllowed);
 }
 
+#[test]
+fn tcb_status_gate_fail_closed_on_empty_allowlist() {
+    use super::policy_rules::{tcb_status_allowed, DEFAULT_ALLOWED_TCB_STATUS};
+
+    // Audit #356 / #17: an empty allowlist must NOT skip the TCB check; it
+    // enforces against the secure default {"UpToDate"}.
+    assert_eq!(DEFAULT_ALLOWED_TCB_STATUS, "UpToDate");
+
+    // Real UpToDate is admitted under an empty (default) policy.
+    assert!(tcb_status_allowed(&[], "UpToDate", false, false));
+
+    // Real OutOfDate is rejected under an empty policy — the regression this
+    // fix proves. Previously this was admitted (check skipped).
+    assert!(!tcb_status_allowed(&[], "OutOfDate", false, false));
+    assert!(!tcb_status_allowed(&[], "SWHardeningNeeded", false, false));
+}
+
+#[test]
+fn tcb_status_gate_honors_non_empty_allowlist() {
+    use super::policy_rules::tcb_status_allowed;
+
+    let allow = vec!["SWHardeningNeeded".to_owned()];
+    // Explicit allowlist still honored: an entry it lists is admitted...
+    assert!(tcb_status_allowed(
+        &allow,
+        "SWHardeningNeeded",
+        false,
+        false
+    ));
+    // ...and a status it does not list is rejected (including the default).
+    assert!(!tcb_status_allowed(&allow, "UpToDate", false, false));
+    assert!(!tcb_status_allowed(&allow, "OutOfDate", false, false));
+}
+
+#[test]
+fn tcb_status_gate_rejects_revoked_unconditionally() {
+    use super::policy_rules::tcb_status_allowed;
+
+    // Revoked is rejected even if somehow present in the allowlist, and
+    // case-insensitively (guards the stored-status subgroup-reuse path).
+    assert!(!tcb_status_allowed(&[], "Revoked", false, false));
+    assert!(!tcb_status_allowed(
+        &["Revoked".to_owned()],
+        "Revoked",
+        false,
+        false
+    ));
+    assert!(!tcb_status_allowed(&[], "revoked", false, false));
+    assert!(!tcb_status_allowed(&[], "REVOKED", false, false));
+    // Even an explicit mock flag on a mock-accepting policy does not rescue a
+    // Revoked status — Revoked is rejected before the mock branch.
+    assert!(!tcb_status_allowed(&[], "Revoked", true, true));
+}
+
+#[test]
+fn tcb_status_gate_preserves_mock_path() {
+    use super::policy_rules::tcb_status_allowed;
+
+    // Mock must still be admitted on a mock-accepting policy: via the explicit
+    // is_mock flag under an empty policy (admit_tee_node path)...
+    assert!(tcb_status_allowed(&[], "Mock", true, true));
+    // ...and via the reserved "Mock" status with is_mock=false (the op-apply /
+    // subgroup-reuse path, which carries no is_mock flag).
+    assert!(tcb_status_allowed(&[], "Mock", false, true));
+    // Mock bypasses even a non-empty allowlist that does not list it.
+    assert!(tcb_status_allowed(
+        &["UpToDate".to_owned()],
+        "Mock",
+        false,
+        true
+    ));
+
+    // The is_mock flag bypasses the TCB check entirely — even a real-looking
+    // non-"Mock" status like OutOfDate is admitted when is_mock=true on a
+    // mock-accepting policy (the TCB allowlist does not apply to the mock path).
+    assert!(tcb_status_allowed(&[], "OutOfDate", true, true));
+}
+
+#[test]
+fn tcb_status_gate_mock_requires_accept_mock() {
+    use super::policy_rules::tcb_status_allowed;
+
+    // Audit follow-up: the mock bypass is gated on the group's stored
+    // `accept_mock`. A stored "Mock" status replayed onto a real fleet
+    // (accept_mock=false) on the op-apply path must NOT bypass the gate — it
+    // falls through to the empty-allowlist secure default and is rejected,
+    // rather than acting as a permanent bypass token.
+    assert!(!tcb_status_allowed(&[], "Mock", false, false));
+    assert!(!tcb_status_allowed(
+        &["UpToDate".to_owned()],
+        "Mock",
+        false,
+        false
+    ));
+    // The explicit is_mock flag is likewise inert when the policy rejects mock
+    // (defense-in-depth behind admit_tee_node's upstream `is_mock && !accept_mock`
+    // rejection). With a non-allowlisted status it is rejected.
+    assert!(!tcb_status_allowed(&[], "OutOfDate", true, false));
+}
+
+#[test]
+fn validate_allowlists_empty_tcb_enforces_secure_default() {
+    use super::policy_rules::{
+        validate_tee_attestation_allowlists, MembershipPolicyRejection, TeeAllowlistPolicy,
+        TeeAttestationClaims,
+    };
+
+    // Empty allowlists everywhere except the secure-default TCB enforcement.
+    // `accept_mock` defaults false here (a real fleet); the mock case below
+    // flips it on.
+    let policy = TeeAllowlistPolicy {
+        allowed_mrtd: vec!["m-ok".to_owned()],
+        allowed_rtmr0: vec![],
+        allowed_rtmr1: vec![],
+        allowed_rtmr2: vec![],
+        allowed_rtmr3: vec![],
+        allowed_tcb_statuses: vec![],
+        accept_mock: false,
+    };
+    let up_to_date = TeeAttestationClaims {
+        mrtd: "m-ok",
+        rtmr0: "x",
+        rtmr1: "x",
+        rtmr2: "x",
+        rtmr3: "x",
+        tcb_status: "UpToDate",
+    };
+    assert!(validate_tee_attestation_allowlists(&policy, &up_to_date).is_ok());
+
+    let out_of_date = TeeAttestationClaims {
+        tcb_status: "OutOfDate",
+        ..up_to_date
+    };
+    let err = validate_tee_attestation_allowlists(&policy, &out_of_date).unwrap_err();
+    assert_eq!(err.reason(), MembershipPolicyRejection::TcbStatusNotAllowed);
+
+    // A stored "Mock" status on the op-apply path is rejected on a real fleet
+    // (accept_mock=false): it falls through to the secure default rather than
+    // bypassing the gate.
+    let mock = TeeAttestationClaims {
+        tcb_status: "Mock",
+        ..up_to_date
+    };
+    let err = validate_tee_attestation_allowlists(&policy, &mock).unwrap_err();
+    assert_eq!(err.reason(), MembershipPolicyRejection::TcbStatusNotAllowed);
+
+    // ...but on a mock-accepting fleet the same stored "Mock" passes through.
+    let mock_policy = TeeAllowlistPolicy {
+        accept_mock: true,
+        ..policy
+    };
+    assert!(validate_tee_attestation_allowlists(&mock_policy, &mock).is_ok());
+}
+
 #[test]
 fn count_members_and_admins() {
     let store = test_store();