diff --git a/docs/adr/0002-storage-acceptance-is-not-cryptographic-endorsement.md b/docs/adr/0002-storage-acceptance-is-not-cryptographic-endorsement.md new file mode 100644 index 00000000..3d00fb81 --- /dev/null +++ b/docs/adr/0002-storage-acceptance-is-not-cryptographic-endorsement.md @@ -0,0 +1,133 @@ + + +# ADR 0002 — Storage acceptance is not cryptographic endorsement + +Status: accepted +Date: 2026-08-07 + +## Context + +DSM storage nodes are non-authoritative infrastructure: they persist and serve +replicated bytes (device registrations, identity bindings, receipts) so that +clients can retrieve them. They are indexers, not authorities. The client holds +the trust roots — a peer's SPHINCS+ signing key (the **AK**) is established +out-of-band through the QR/BLE pairing, and the client verifies cryptographic +evidence itself. + +A device identity binding ties a peer's ML-KEM (Kyber-768) public key to its +`(device_id, genesis_hash)` under the peer's AK: + +```text +digest = H(KYBER_IDENTITY_BINDING_TAG ‖ device_id ‖ genesis_hash ‖ kyber_pubkey) +binding = SPHINCS+_sign(AK_secret, digest) +verify = SPHINCS+_verify(AK_public, digest, binding) // AK_public is the trust root +``` + +A brief detour (PR #625, first iteration) had the storage node verify this +binding *before persistence*. Two problems made that the wrong place for it: + +1. **It cannot establish trust.** The node can only verify the binding against + the AK the registrant itself supplied in the request. That proves the + submission is *internally consistent* — the registrant controls the AK it + claims and signed the Kyber key under it — but the registrant chooses its own + AK. A device can still bind any Kyber key to its own self-chosen identity. A + peer with no contact record has no trusted AK to compare against, so the + node's acceptance tells it nothing about identity. +2. **It quietly moves authority into the storage layer.** If clients treat "the + node stored and served it" as endorsement, the storage layer becomes a + de-facto certificate authority — the opposite of DSM's model. + +## Decision + +**A client may accept or cache peer Kyber (ML-KEM) material only when it can +establish a cryptographic chain from that material to an independently +authenticated peer AK, or to an equivalently authenticated pairing transcript. +Storage-node acceptance is never part of that trust chain.** + +The test is a *proof chain*, not a function call. "Does this path call +`verify_kyber_identity_binding()`?" is the wrong question; the right question is +**"is the Kyber key bound — by a signature the client verified against the pinned +peer AK — to the identity it is being cached under?"** A path satisfies the +invariant in exactly one of two ways, and every consumer must land in one of +these or be **rejected**: + +- **Authenticated by detached binding.** The client holds the pinned AK + out-of-band and verifies a detached `kyber_binding_sig` over + `H(TAG ‖ device_id ‖ genesis ‖ kyber_pk)` against that pinned AK before + caching. (Node-served material is *always* this case — untrusted bytes, verified + against the pinned AK.) +- **Authenticated by signed pairing transcript.** The Kyber public key is inside + the exact bytes of a message the client already signature-verified against the + pinned peer AK. If the verified transcript covers the Kyber key, a second + detached check is redundant, not security-critical. If it does **not** cover the + Kyber key, the transcript does not authenticate it and the path needs a detached + binding — or is rejected. + +Concretely: + +- **Storage node — dumb, non-authoritative.** It enforces *structural* admission + only: field presence, exact lengths (`device_id` 32, `genesis_hash` 32, + `kyber_public_key` 1184), size ceilings, database invariants, and + (out of scope here) rate/quota/authenticated-write limits. It stores the + `kyber_binding_sig` as opaque bytes and serves it. It makes **no** + identity-trust decision and runs **no** signature verification on registration + content. "The node stored and served it" is never a link in the trust chain. + +- **The pinned AK is itself a trust root and is never re-rooted from untrusted + transport.** A node/peer-supplied AK that differs from the pinned AK is a + substitution attempt — reject it; never overwrite the pinned AK from wire bytes + (node quorum *or* BLE). A genuinely rotated AK is a new identity re-established + through the explicit pairing flow. + +- **Unknown peer ⇒ no chain ⇒ fail closed.** No independently authenticated AK + means no authenticated Kyber identity. Outside the explicit pairing flow this + fails closed. **No implicit TOFU.** + +## Complete path matrix + +Every client path that consumes or caches peer Kyber material, classified by how +(if at all) it chains to the pinned AK. Every row lands in exactly one of three +statuses — **authenticated by detached binding**, **authenticated by signed +pairing transcript**, or **rejected**. There is deliberately no fourth +"trusted because storage returned it" state. Cited against +`dsm_client/deterministic_state_machine/` on branch `fix/kyber-binding-verification`. + +| # | Client path | Kyber source | Chain to pinned AK | Status | Enforced today? | +|---|---|---|---|---|---| +| 1 | Send-path repair from quorum (`app_router_impl.rs` `repair_contact_decision`) | node quorum | require returned AK == pinned AK, then verify detached `kyber_binding_sig` vs pinned AK before refresh; AK never overwritten | **Authenticated by detached binding** | ✅ (this PR — repair-path AK preservation) | +| 2 | Storage-route hydrate (`storage_routes.rs` :186–197) | node quorum | verify detached binding vs pinned `contact.public_key`; empty AK → fail closed; bind only if absent | **Authenticated by detached binding** | ✅ | +| 3 | Send-side encapsulation (`app_router_impl.rs` :1519–1546 → `receipts.rs` :149) | local cache | reads only the cache; every node-served writer of that cache is row 1/2; fail-closed on empty | **Authenticated (transitive)** | ✅ | +| 4 | QR contact-add (`contact_sdk.rs` :643; `resolve_counterparty_via_transport`) | — (QR carries no Kyber) | QR transcript authenticates the **signing AK only**; Kyber slot stored empty and later filled by row 1/2 | **Authenticated pairing transcript (AK); Kyber deferred** | ✅ | +| 5 | Unknown peer (no contact record) | node/wire | no pinned AK ⇒ no chain; online send requires an existing contact and aborts otherwise | **Rejected (fail closed, no TOFU)** | ✅ | +| 6 | BLE prepare-**request** → AK write (`bilateral_ble_handler.rs`, `bind_contact_public_key_if_absent`) | BLE wire, unsigned | first-write-wins: establish the AK only when absent; a *differing* wire AK is reported as a substitution and rejected — the pinned AK is preserved | **Rejected (must not re-root pinned AK)** | ✅ (this PR — first-write-wins) | +| 7 | BLE prepare-**response** → AK write (`bilateral_ble_handler.rs`, `bind_contact_public_key_if_absent`) | BLE wire, unsigned | same first-write-wins treatment | **Rejected (must not re-root pinned AK)** | ✅ (this PR — first-write-wins) | +| 8 | BLE prepare-**request** → Kyber bind (`bilateral_ble_handler.rs` :2338) | BLE wire | `handle_prepare_request` runs **no** signature verify before the bind; `BilateralPrepareRequest` carries no envelope signature and no `kyber_binding_sig` — the Kyber key is in **no** verified-against-pinned-AK message | **Rejected (gap)** | ❌ → **BLE P0 follow-up** | +| 9 | BLE prepare-**response** → Kyber bind (`bilateral_ble_handler.rs` :3114) | BLE wire | bind runs **before** the σ_B verify (:3242); σ_B covers only `"DSM/bilateral-sign\0" ‖ commitment_hash` (excludes the Kyber key) and verifies against the just-overwritten AK (tautology) | **Rejected (gap)** | ❌ → **BLE P0 follow-up** | + +**Decisive question for the BLE rows** (8/9): *is the Kyber public key inside the +exact bytes that were successfully verified against the pinned peer AK before +`bind_contact_kyber_key_if_absent` runs?* Verified against source: **no** — no +signature covers the Kyber field on either BLE path. These are therefore real +gaps, not redundant checks. Closing them is a wire-format change (bind the Kyber +material into the signed BLE transcript, or add a detached `kyber_binding_sig` to +the prepare messages) that must be validated on two phones — tracked as the +**release-blocking BLE P0 follow-up**, deliberately kept out of #625 so the +evidence boundary (client-only, no proto) stays clean. + +## Consequences + +- **Resource abuse ≠ identity authentication.** Without node-side signature + verification, a client can push cryptographic garbage into storage. That is a + storage-abuse concern, addressed with structural validation, size caps, rate + limits, quotas, and authenticated write permissions — **not** by moving + identity authority into the node. + +- **No legacy/TOFU path.** A binding served for a peer the client has never + paired with is untrusted and unusable (row 5); the peer must be established + in person (QR/BLE) first. DSM beta has no trust-on-first-use fallback. + +- The storage node's own comment already states the model: *"the node is a dumb + indexer: it enforces length/presence only; the cryptographic identity binding + is verified client-side against the peer's AK."* This ADR makes that binding + invariant explicit and repo-wide, and the matrix above makes each path's status + auditable. diff --git a/dsm_client/deterministic_state_machine/dsm_sdk/src/bluetooth/bilateral_ble_handler.rs b/dsm_client/deterministic_state_machine/dsm_sdk/src/bluetooth/bilateral_ble_handler.rs index 8b48413e..1a8308fa 100644 --- a/dsm_client/deterministic_state_machine/dsm_sdk/src/bluetooth/bilateral_ble_handler.rs +++ b/dsm_client/deterministic_state_machine/dsm_sdk/src/bluetooth/bilateral_ble_handler.rs @@ -2296,33 +2296,55 @@ impl BilateralBleHandler { prepare_request.sender_signing_public_key.is_empty() ); if !prepare_request.sender_signing_public_key.is_empty() { - log::info!( - "[BilateralBleHandler] 🔑 Updating contact signing key from prepare request (len={})", - prepare_request.sender_signing_public_key.len() - ); - // Update in-memory contact manager - if let Err(e) = mgr.update_contact_signing_key( - &counterparty_device_id, - prepare_request.sender_signing_public_key.clone(), - ) { - log::warn!( - "[BilateralBleHandler] ⚠️ Failed to update in-memory contact signing key: {}", - e - ); - // Continue anyway - relationship may still work if contact already has key - } - // Persist to SQLite for durability across restarts - if let Err(e) = crate::storage::client_db::update_contact_public_key( + // TRUST-ROOT PRESERVATION (ADR 0002, matrix rows 6/7): the pairing-established + // signing AK is NEVER re-rooted from wire bytes. Establish it only when the contact + // has none; a *differing* wire AK is a possible substitution and is rejected — the + // pinned AK stands. The SQLite row is the decision point; the in-memory manager is + // mirrored only when the wire AK equals the pinned trust root. + let ak = &prepare_request.sender_signing_public_key; + match crate::storage::client_db::bind_contact_public_key_if_absent( &counterparty_device_id, - &prepare_request.sender_signing_public_key, + ak, ) { - log::warn!( - "[BilateralBleHandler] ⚠️ Failed to persist contact public_key to SQLite: {}", - e - ); - // Non-fatal - in-memory update still happened - } else { - log::info!("[BilateralBleHandler] ✅ Persisted contact public_key to SQLite"); + Ok(crate::storage::client_db::AkBindOutcome::Established) => { + if let Err(e) = + mgr.update_contact_signing_key(&counterparty_device_id, ak.clone()) + { + log::warn!( + "[BilateralBleHandler] ⚠️ Failed to mirror established contact AK into memory: {e}" + ); + } + log::info!( + "[BilateralBleHandler] ✅ Established contact signing AK from prepare request (first-write)" + ); + } + Ok(crate::storage::client_db::AkBindOutcome::AlreadyPinnedMatching) => { + if let Err(e) = + mgr.update_contact_signing_key(&counterparty_device_id, ak.clone()) + { + log::warn!( + "[BilateralBleHandler] ⚠️ Failed to refresh in-memory contact AK: {e}" + ); + } + log::debug!( + "[BilateralBleHandler] contact signing AK matches the pinned trust root" + ); + } + Ok(crate::storage::client_db::AkBindOutcome::RejectedSubstitution) => { + log::warn!( + "[BilateralBleHandler] ⚠️ prepare-request signing AK differs from the pinned QR/BLE AK — ignoring (possible substitution); trust root preserved" + ); + } + Ok(crate::storage::client_db::AkBindOutcome::NoContact) => { + log::warn!( + "[BilateralBleHandler] ⚠️ no contact record for prepare-request device — signing AK not established" + ); + } + Err(e) => { + log::warn!( + "[BilateralBleHandler] ⚠️ Failed to persist contact signing AK: {e}" + ); + } } } else { log::warn!( @@ -3072,40 +3094,59 @@ impl BilateralBleHandler { }; if let Some(counterparty_device_id) = counterparty_device_id { - log::info!( - "[BilateralBleHandler] 🔑 handle_prepare_response: Updating contact signing key from response (len={})", - prepare_response.responder_signing_public_key.len() - ); - - // Update in-memory contact manager - { - let mut mgr = self.bilateral_tx_manager.write().await; - if let Err(e) = mgr.update_contact_signing_key( + // TRUST-ROOT PRESERVATION (ADR 0002, matrix rows 6/7): first-write-wins; never + // re-root the pinned QR/BLE AK from the response wire. Mirror the in-memory manager + // only when the wire AK equals the pinned trust root. + if !prepare_response.responder_signing_public_key.is_empty() { + let ak = &prepare_response.responder_signing_public_key; + match crate::storage::client_db::bind_contact_public_key_if_absent( &counterparty_device_id, - prepare_response.responder_signing_public_key.clone(), + ak, ) { - log::warn!( - "[BilateralBleHandler] ⚠️ Failed to update in-memory contact signing key in handle_prepare_response: {}", - e - ); + Ok(crate::storage::client_db::AkBindOutcome::Established) => { + let mut mgr = self.bilateral_tx_manager.write().await; + if let Err(e) = + mgr.update_contact_signing_key(&counterparty_device_id, ak.clone()) + { + log::warn!( + "[BilateralBleHandler] ⚠️ Failed to mirror established responder AK into memory: {e}" + ); + } + log::info!( + "[BilateralBleHandler] ✅ Established contact signing AK from prepare response (first-write)" + ); + } + Ok(crate::storage::client_db::AkBindOutcome::AlreadyPinnedMatching) => { + let mut mgr = self.bilateral_tx_manager.write().await; + if let Err(e) = + mgr.update_contact_signing_key(&counterparty_device_id, ak.clone()) + { + log::warn!( + "[BilateralBleHandler] ⚠️ Failed to refresh in-memory responder AK: {e}" + ); + } + log::debug!( + "[BilateralBleHandler] responder signing AK matches the pinned trust root" + ); + } + Ok(crate::storage::client_db::AkBindOutcome::RejectedSubstitution) => { + log::warn!( + "[BilateralBleHandler] ⚠️ prepare-response signing AK differs from the pinned QR/BLE AK — ignoring (possible substitution); trust root preserved" + ); + } + Ok(crate::storage::client_db::AkBindOutcome::NoContact) => { + log::warn!( + "[BilateralBleHandler] ⚠️ no contact record for prepare-response device — signing AK not established" + ); + } + Err(e) => { + log::warn!( + "[BilateralBleHandler] ⚠️ Failed to persist responder signing AK: {e}" + ); + } } } - // Persist to SQLite for durability across restarts - if let Err(e) = crate::storage::client_db::update_contact_public_key( - &counterparty_device_id, - &prepare_response.responder_signing_public_key, - ) { - log::warn!( - "[BilateralBleHandler] ⚠️ Failed to persist responder public_key to SQLite in handle_prepare_response: {}", - e - ); - } else { - log::info!( - "[BilateralBleHandler] ✅ Persisted responder public_key to SQLite (handle_prepare_response)" - ); - } - // Bind the responder's Kyber (ML-KEM-768) capability alongside the signing key: // the per-step EK receipt built in send_bilateral_confirm (immediately after this) // encapsulates to it and fail-closes without a copy on the contact record. diff --git a/dsm_client/deterministic_state_machine/dsm_sdk/src/handlers/app_router_impl.rs b/dsm_client/deterministic_state_machine/dsm_sdk/src/handlers/app_router_impl.rs index e3fee114..a998a840 100644 --- a/dsm_client/deterministic_state_machine/dsm_sdk/src/handlers/app_router_impl.rs +++ b/dsm_client/deterministic_state_machine/dsm_sdk/src/handlers/app_router_impl.rs @@ -96,7 +96,10 @@ pub(crate) struct QuorumDeviceIdentity { /// serves a divergent Kyber key (equivocation) breaks agreement. pub(crate) kyber_public_key: Vec, /// SPHINCS+ (device AK) signature binding `kyber_public_key` to the device - /// identity; verified at contact-add against the QR-carried AK. + /// identity. Verified by the consuming client against the pinned QR/BLE AK + /// before the Kyber key is used or cached (ADR 0002) — on this node-served + /// path that is the repair/hydrate flow, NOT at contact-add (the QR carries + /// no Kyber key). pub(crate) kyber_binding_sig: Vec, } @@ -3044,7 +3047,9 @@ pub(crate) async fn fetch_quorum_device_identity( } // Kyber material is MANDATORY (DSM beta, no legacy path). A record // without a well-formed ML-KEM-768 key + binding is invalid; the - // cryptographic binding is verified at contact-add against the AK. + // cryptographic binding is verified against the pinned AK before the + // Kyber key is cached/used (repair/hydrate paths; ADR 0002), NOT at + // contact-add. if decoded.kyber_public_key.len() != 1184 { last_error = Some(format!( "device identity from {} had invalid kyber_public_key length {}", diff --git a/dsm_client/deterministic_state_machine/dsm_sdk/src/storage/client_db/contacts.rs b/dsm_client/deterministic_state_machine/dsm_sdk/src/storage/client_db/contacts.rs index b308eed1..bf6745d3 100644 --- a/dsm_client/deterministic_state_machine/dsm_sdk/src/storage/client_db/contacts.rs +++ b/dsm_client/deterministic_state_machine/dsm_sdk/src/storage/client_db/contacts.rs @@ -1299,39 +1299,6 @@ pub fn has_unpaired_contacts() -> bool { } } -/// Update a contact's public key (e.g., after receiving signing key via BLE prepare) -pub fn update_contact_public_key(device_id: &[u8], public_key: &[u8]) -> Result<()> { - if device_id.len() != 32 { - return Err(anyhow!("Invalid device_id length")); - } - - let binding = get_connection()?; - let conn = binding.lock().unwrap_or_else(|poisoned| { - log::warn!("DB lock poisoned, recovering"); - poisoned.into_inner() - }); - - let rows_changed = conn.execute( - "UPDATE contacts SET public_key = ?1 WHERE device_id = ?2", - params![public_key, device_id], - )?; - - if rows_changed == 0 { - info!( - "No contact found with device_id={:?} to update public_key", - &device_id[..8] - ); - } else { - info!( - "Updated contact public_key: device_id={:?} key_len={}", - &device_id[..8], - public_key.len() - ); - } - - Ok(()) -} - /// FIRST-WRITE-WINS persist of a counterparty's ML-KEM capability. /// /// Writes ONLY when the contact currently has no key. A locally-bound nonempty @@ -1368,6 +1335,75 @@ pub fn bind_contact_kyber_key_if_absent(device_id: &[u8], kyber_public_key: &[u8 Ok(rows_changed > 0) } +/// Outcome of a first-write-wins attempt to set a contact's signing AK from wire bytes. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AkBindOutcome { + /// The contact had no AK; this call established it. + Established, + /// A matching AK was already pinned; nothing changed. + AlreadyPinnedMatching, + /// A *different* AK was already pinned; the wire AK was REJECTED and the pinned AK stands + /// (possible substitution/equivocation). + RejectedSubstitution, + /// No contact exists for this `device_id`. + NoContact, +} + +/// FIRST-WRITE-WINS persist of a counterparty's signing AK — the pairing trust root (ADR 0002). +/// +/// The signing AK is established out-of-band by the QR/BLE pairing and is the root every Kyber +/// identity binding is verified against. It is NEVER re-rooted from wire bytes: this writes ONLY +/// when the contact currently has no AK. A non-empty pinned AK is preserved; a *differing* wire AK +/// is reported as a substitution attempt and rejected. Rotation, if DSM ever needs it, is an +/// explicit re-pairing, never a side effect of a transfer handshake. +/// +/// (This closes matrix rows 6/7. Authenticating the BLE-delivered *Kyber* key against the pinned AK +/// — rows 8/9 — is the separate release-blocking BLE P0 work and is deliberately not done here.) +pub fn bind_contact_public_key_if_absent( + device_id: &[u8], + public_key: &[u8], +) -> Result { + if device_id.len() != 32 { + return Err(anyhow!("Invalid device_id length")); + } + + let binding = get_connection()?; + let conn = binding.lock().unwrap_or_else(|poisoned| { + log::warn!("DB lock poisoned, recovering"); + poisoned.into_inner() + }); + + let existing: Option>> = conn + .query_row( + "SELECT public_key FROM contacts WHERE device_id = ?1", + params![device_id], + |row| row.get::<_, Option>>(0), + ) + .optional()?; + + match existing { + None => Ok(AkBindOutcome::NoContact), + Some(pinned) => { + let pinned = pinned.unwrap_or_default(); + if pinned.is_empty() { + // First establishment. The WHERE clause re-checks emptiness so a concurrent + // establisher cannot be clobbered. + conn.execute( + "UPDATE contacts SET public_key = ?1 + WHERE device_id = ?2 + AND (public_key IS NULL OR length(public_key) = 0)", + params![public_key, device_id], + )?; + Ok(AkBindOutcome::Established) + } else if pinned == public_key { + Ok(AkBindOutcome::AlreadyPinnedMatching) + } else { + Ok(AkBindOutcome::RejectedSubstitution) + } + } + } +} + /// Remove a contact by its contact_id. Returns Ok(true) if a row was deleted, Ok(false) if not found. pub fn remove_contact(contact_id: &str) -> Result { let binding = get_connection()?; @@ -1506,6 +1542,64 @@ mod tests { assert_eq!(loaded.public_key, vec![0xBBu8; 64]); } + /// Trust-root preservation (ADR 0002, matrix rows 6/7): the pairing-established signing AK is + /// first-write-wins and is NEVER re-rooted from wire bytes. A differing wire AK is reported as a + /// substitution and the pinned AK is preserved byte-for-byte. + #[test] + #[serial] + fn bind_contact_public_key_if_absent_is_first_write_wins_and_rejects_substitution() { + init_test_db(); + let device_id = [0x5Au8; 32]; + + // No contact yet → NoContact (nothing to establish). + assert_eq!( + bind_contact_public_key_if_absent(&device_id, &[0x01u8; 64]).unwrap(), + AkBindOutcome::NoContact + ); + + // Contact with an EMPTY AK → first write establishes it. + let mut c = make_contact(device_id, "peer"); + c.public_key = Vec::new(); + store_contact(&c).expect("store empty-AK contact"); + let established_ak = vec![0xE1u8; 64]; + assert_eq!( + bind_contact_public_key_if_absent(&device_id, &established_ak).unwrap(), + AkBindOutcome::Established + ); + assert_eq!( + get_contact_by_device_id(&device_id) + .unwrap() + .unwrap() + .public_key, + established_ak, + "the empty slot was established from the wire" + ); + + // Same AK again → AlreadyPinnedMatching, unchanged. + assert_eq!( + bind_contact_public_key_if_absent(&device_id, &established_ak).unwrap(), + AkBindOutcome::AlreadyPinnedMatching + ); + + // A DIFFERENT wire AK → RejectedSubstitution; the pinned AK is preserved byte-for-byte. + let attacker_ak = vec![0xEEu8; 64]; + assert_eq!( + bind_contact_public_key_if_absent(&device_id, &attacker_ak).unwrap(), + AkBindOutcome::RejectedSubstitution + ); + assert_eq!( + get_contact_by_device_id(&device_id) + .unwrap() + .unwrap() + .public_key, + established_ak, + "a differing wire AK must NEVER re-root the pinned AK" + ); + + // Malformed device_id → error. + assert!(bind_contact_public_key_if_absent(&[0u8; 16], &established_ak).is_err()); + } + /// Cold-peer RPA rotation: after a paired peer's BLE address rotates, the canonical re-persist /// (`update_contact_ble_status(device_id, None, Some(new))`, driven by the probe's on-match /// `observeGattIdentityRead`) must re-point the contact so the FRESH address resolves and the diff --git a/dsm_storage_node/src/api/identity/device_api.rs b/dsm_storage_node/src/api/identity/device_api.rs index deb8ccff..e1404736 100644 --- a/dsm_storage_node/src/api/identity/device_api.rs +++ b/dsm_storage_node/src/api/identity/device_api.rs @@ -36,10 +36,6 @@ pub enum RegisterError { InvalidGenesisHash, InvalidKyberKey, InvalidKyberBinding, - /// The binding is well-formed but does not verify against the device's AK. - /// Distinct from `InvalidKyberBinding` (absent/malformed) so a forgery is - /// never reported as a formatting problem. - KyberBindingDoesNotVerify, DeviceAlreadyExists, DeviceNotFound, DatabaseError(String), @@ -57,10 +53,6 @@ impl IntoResponse for RegisterError { StatusCode::BAD_REQUEST, "Invalid or missing kyber_public_key (ML-KEM-768, 1184 bytes required)", ), - RegisterError::KyberBindingDoesNotVerify => ( - StatusCode::BAD_REQUEST, - "kyber_binding_sig does not bind this Kyber key to (device_id, genesis_hash) under the device's AK", - ), RegisterError::InvalidKyberBinding => ( StatusCode::BAD_REQUEST, "Invalid or missing kyber_binding_sig", @@ -102,7 +94,9 @@ pub async fn register_device( return Err(RegisterError::InvalidGenesisHash); } - // Validate Kyber material — MANDATORY (DSM beta has no legacy path). + // Validate Kyber material — MANDATORY (DSM beta has no legacy path). The node + // is a dumb indexer: it enforces length/presence only; the cryptographic + // identity binding is verified client-side against the peer's AK. if req.kyber_public_key.len() != 1184 { return Err(RegisterError::InvalidKyberKey); } @@ -110,48 +104,6 @@ pub async fn register_device( return Err(RegisterError::InvalidKyberBinding); } - // VERIFY BEFORE PERSISTENCE. - // - // This block used to read: "the node is a dumb indexer: it enforces - // length/presence only; the cryptographic identity binding is verified - // client-side against the peer's AK." That client-side verification did not - // exist — `verify_kyber_identity_binding` had ZERO callers anywhere in the - // repository. The node stored a signature nothing ever checked, so an - // ML-KEM key was bound to a device identity purely by assertion, and a - // substituted key would have been served to every peer that fetched it. - // - // Being a dumb indexer is about not interpreting CONTENT. It was never a - // reason to persist an identity claim without checking the signature that - // makes it a claim at all. - // - // `verify_kyber_identity_binding` re-derives - // `H(domain ‖ device_id ‖ genesis_hash ‖ kyber_pubkey)` and requires - // `Ok(true)` from `sphincs_verify` — an `Err` and an `Ok(false)` are both - // refusals, and neither reaches the database. - let device_id_arr: [u8; 32] = req - .device_id - .as_slice() - .try_into() - .map_err(|_| RegisterError::InvalidDeviceId)?; - let genesis_arr: [u8; 32] = req - .genesis_hash - .as_slice() - .try_into() - .map_err(|_| RegisterError::InvalidGenesisHash)?; - if let Err(e) = dsm_sdk::sdk::kyber_identity::verify_kyber_identity_binding( - &device_id_arr, - &genesis_arr, - &req.kyber_public_key, - &req.kyber_binding_sig, - &req.pubkey, - ) { - log::warn!( - "device registration refused: kyber identity binding failed to verify for {}: {e}", - text_id::encode_base32_crockford(&req.device_id) - ); - return Err(RegisterError::KyberBindingDoesNotVerify); - } - // Convert to Base32 for DB storage (DB uses string device_id) let device_id_b32 = text_id::encode_base32_crockford(&req.device_id); diff --git a/dsm_storage_node/tests/kyber_binding_enforced.rs b/dsm_storage_node/tests/kyber_binding_enforced.rs deleted file mode 100644 index abde122b..00000000 --- a/dsm_storage_node/tests/kyber_binding_enforced.rs +++ /dev/null @@ -1,310 +0,0 @@ -// SPDX-License-Identifier: MIT OR Apache-2.0 - -//! The node must VERIFY the ML-KEM identity binding before persisting it. -//! -//! Until this branch it did not. `register_device` checked only length and -//! presence, under a comment asserting that "the cryptographic identity binding -//! is verified client-side against the peer's AK". -//! -//! That client-side verification is PARTIAL, not absent. `dsm_sdk`'s -//! `repair_contact_identity_from_quorum` (handlers/app_router_impl.rs:360) does -//! call `verify_kyber_identity_binding`, and does it correctly — against the -//! QR-established `contact_ak` captured at :318, deliberately before repair -//! overwrites `public_key`. But that is ONE repair path, not the general fetch -//! path, so the node was still persisting bindings that nothing had checked at -//! the time of writing. A device could bind any ML-KEM key to its identity by -//! assertion and the node would store it and serve it. -//! -//! These tests drive the REAL handler against a real (in-memory) database and -//! then read the persisted rows back. They deliberately do not test the -//! standalone verifier: the verifier was already correct, and testing it was -//! exactly the thing that failed to notice nothing called it. -//! -//! No skip path. The pre-existing `device_api::tests` return early unless -//! `DSM_RUN_DB_TESTS=1`, so they are vacuous in CI and could not have caught -//! this. - -#![cfg(feature = "local-dev")] -#![allow(clippy::disallowed_methods)] - -use axum::body::Bytes; -use axum::Extension; -use dsm::crypto::signatures::SignatureKeyPair; -use dsm::types::proto as pb; -use dsm_sdk::util::text_id; -use dsm_storage_node::{ - api::identity::device_api::register_device, - db, - replication::{ReplicationConfig, ReplicationManager}, - AppState, -}; -use prost::Message; -use std::sync::Arc; - -async fn make_state() -> AppState { - let pool = db::create_pool(":memory:", true).expect("create_pool"); - db::init_db(&pool).await.expect("init_db"); - let replication_config = ReplicationConfig { - replication_factor: 3, - gossip_interval_ticks: 100, - failure_timeout_ticks: 300, - gossip_fanout: 3, - max_concurrent_jobs: 10, - }; - let replication_manager = Arc::new( - ReplicationManager::new_for_tests( - replication_config, - "test-node".to_string(), - "http://localhost:8080".to_string(), - ) - .expect("ReplicationManager::new_for_tests"), - ); - AppState::new( - "test-node".to_string(), - "http://localhost:8080", - None, - Arc::new(pool), - replication_manager, - ) -} - -const KYBER_PK_LEN: usize = 1184; - -/// The canonical binding digest, rebuilt here independently of the SDK so this -/// suite does not simply echo the implementation it is gating. -fn canonical_binding_digest(device_id: &[u8; 32], genesis: &[u8; 32], kyber_pk: &[u8]) -> [u8; 32] { - let mut h = blake3::Hasher::new(); - h.update(b"DSM/kyber-identity-binding"); - h.update(&[0u8]); // the encoder's one delimiter - h.update(device_id); - h.update(genesis); - h.update(kyber_pk); - *h.finalize().as_bytes() -} - -struct Device { - id: [u8; 32], - genesis: [u8; 32], - kyber_pk: Vec, - ak: SignatureKeyPair, -} - -fn device(seed: u8) -> Device { - Device { - id: [seed; 32], - genesis: [seed.wrapping_add(1); 32], - kyber_pk: vec![seed.wrapping_add(2); KYBER_PK_LEN], - ak: SignatureKeyPair::generate_from_entropy( - format!("DSM/test/kyber-enforce/{seed}").as_bytes(), - ) - .expect("AK"), - } -} - -fn request(d: &Device, kyber_pk: &[u8], sig: Vec) -> Bytes { - let req = pb::RegisterDeviceRequest { - device_id: d.id.to_vec(), - pubkey: d.ak.public_key().to_vec(), - genesis_hash: d.genesis.to_vec(), - kyber_public_key: kyber_pk.to_vec(), - kyber_binding_sig: sig, - }; - let mut buf = Vec::new(); - req.encode(&mut buf).expect("encode"); - Bytes::from(buf) -} - -fn valid_sig(d: &Device) -> Vec { - let digest = canonical_binding_digest(&d.id, &d.genesis, &d.kyber_pk); - d.ak.sign(&digest).expect("sign binding") -} - -async fn stored(state: &AppState, d: &Device) -> Option<(Vec, Vec, Vec, Vec)> { - db::get_device(&state.db_pool, &text_id::encode_base32_crockford(&d.id)) - .await - .expect("get_device") -} - -/// ANTI-VACUITY, and the acceptance half of the gate: a correctly generated -/// canonical binding is accepted AND persisted. Without this, a handler that -/// refused everything would pass every rejection test below. -#[tokio::test] -async fn a_canonical_binding_is_accepted_and_persisted() { - let state = make_state().await; - let d = device(0x10); - - let res = register_device( - Extension(Arc::new(state.clone())), - request(&d, &d.kyber_pk, valid_sig(&d)), - ) - .await; - assert!(res.is_ok(), "a valid canonical binding must register"); - - let row = stored(&state, &d).await.expect("device row must exist"); - assert_eq!( - row.2, d.kyber_pk, - "the Kyber key must be persisted verbatim" - ); - assert!(!row.3.is_empty(), "the binding signature must be persisted"); -} - -/// A FORGERY: a well-formed signature by the right key over the wrong message. -/// `sphincs_verify` answers `Ok(false)` here, not `Err` — so a handler that -/// tested "no error" rather than `Ok(true)` would accept it. -#[tokio::test] -async fn a_forged_binding_is_refused_and_nothing_is_persisted() { - let state = make_state().await; - let d = device(0x20); - - let forged = d.ak.sign(b"a different message entirely").expect("sign"); - let res = register_device( - Extension(Arc::new(state.clone())), - request(&d, &d.kyber_pk, forged), - ) - .await; - - assert!(res.is_err(), "a forged binding must be refused"); - assert!( - stored(&state, &d).await.is_none(), - "a refused registration left a row behind — verification must happen \ - BEFORE persistence, not alongside it" - ); -} - -/// KEY SUBSTITUTION — the attack the binding exists to stop. A valid signature -/// over key A is replayed with key B under the same device identity. -#[tokio::test] -async fn a_substituted_kyber_key_is_refused_and_nothing_is_persisted() { - let state = make_state().await; - let d = device(0x30); - - // Signature is over d.kyber_pk; the request carries a different key. - let other_pk = vec![0xEEu8; KYBER_PK_LEN]; - let res = register_device( - Extension(Arc::new(state.clone())), - request(&d, &other_pk, valid_sig(&d)), - ) - .await; - - assert!(res.is_err(), "a substituted Kyber key must be refused"); - assert!( - stored(&state, &d).await.is_none(), - "a substituted key was persisted — the node would serve it to peers" - ); -} - -/// OLD-DOMAIN artifact: a binding signed under the pre-cut double-NUL digest -/// (impact-table row B4). It must fail, with no compatibility path. -#[tokio::test] -async fn an_old_domain_binding_is_refused() { - let state = make_state().await; - let d = device(0x40); - - let mut old = blake3::Hasher::new(); - old.update(b"DSM/kyber-identity-binding\0"); // literal carried its own NUL - old.update(&[0u8]); // and the helper appended another - old.update(&d.id); - old.update(&d.genesis); - old.update(&d.kyber_pk); - let old_digest = *old.finalize().as_bytes(); - let stale = d.ak.sign(&old_digest).expect("sign old-domain"); - - let res = register_device( - Extension(Arc::new(state.clone())), - request(&d, &d.kyber_pk, stale), - ) - .await; - - assert!( - res.is_err(), - "a binding signed under the pre-cut domain still registers — there is a \ - compatibility verifier that must not exist" - ); - assert!(stored(&state, &d).await.is_none()); -} - -/// MALFORMED: a signature of the wrong length. `sphincs::verify` fails closed on -/// a length mismatch rather than erroring, so this also exercises the -/// `Ok(false)` path rather than the `Err` path. -#[tokio::test] -async fn a_malformed_binding_is_refused() { - let state = make_state().await; - let d = device(0x50); - - let res = register_device( - Extension(Arc::new(state.clone())), - request(&d, &d.kyber_pk, vec![0u8; 7]), - ) - .await; - - assert!( - res.is_err(), - "a truncated binding signature must be refused" - ); - assert!(stored(&state, &d).await.is_none()); -} - -/// A binding signed by a DIFFERENT AK than the one presented as `pubkey`. -#[tokio::test] -async fn a_binding_signed_by_another_key_is_refused() { - let state = make_state().await; - let d = device(0x60); - let impostor = device(0x61); - - let digest = canonical_binding_digest(&d.id, &d.genesis, &d.kyber_pk); - let wrong_signer = impostor.ak.sign(&digest).expect("sign"); - - let res = register_device( - Extension(Arc::new(state.clone())), - request(&d, &d.kyber_pk, wrong_signer), - ) - .await; - - assert!( - res.is_err(), - "a binding signed by another AK must be refused" - ); - assert!(stored(&state, &d).await.is_none()); -} - -/// RELOAD does not bypass verification. A row that was accepted stays readable -/// across a fresh read, and — critically — a rejected registration cannot be -/// "completed" by retrying without a valid binding. -#[tokio::test] -async fn a_rejected_registration_cannot_be_completed_by_retrying() { - let state = make_state().await; - let d = device(0x70); - - // Rejected. - let bad = register_device( - Extension(Arc::new(state.clone())), - request(&d, &d.kyber_pk, vec![0xABu8; 64]), - ) - .await; - assert!(bad.is_err()); - assert!(stored(&state, &d).await.is_none()); - - // Retrying with the same invalid binding is still refused — no partial row - // from the first attempt makes the second one succeed. - let again = register_device( - Extension(Arc::new(state.clone())), - request(&d, &d.kyber_pk, vec![0xABu8; 64]), - ) - .await; - assert!(again.is_err()); - assert!(stored(&state, &d).await.is_none()); - - // A correct binding then registers cleanly, and reads back. - let good = register_device( - Extension(Arc::new(state.clone())), - request(&d, &d.kyber_pk, valid_sig(&d)), - ) - .await; - assert!( - good.is_ok(), - "a valid binding must register after rejections" - ); - - let row = stored(&state, &d).await.expect("row after success"); - assert_eq!(row.2, d.kyber_pk); -}