From a2b7b62960fe0614bf0537acde940bbee36fc941 Mon Sep 17 00:00:00 2001 From: Huskies Agent Date: Fri, 17 Jul 2026 14:07:27 +0000 Subject: [PATCH] huskies: merge 1191 bug /identity node_id field reports the CRDT id, not the node_identity.key the signature uses --- server/src/http/identity.rs | 86 +++++++++++++++++++++---------------- server/src/node_identity.rs | 50 +++++++++++++++++---- 2 files changed, 90 insertions(+), 46 deletions(-) diff --git a/server/src/http/identity.rs b/server/src/http/identity.rs index 8cf4b59c..e80f3423 100644 --- a/server/src/http/identity.rs +++ b/server/src/http/identity.rs @@ -4,12 +4,19 @@ //! authentication is required; only the public half of the keypair is //! disclosed. //! +//! `node_id`/`pubkey` always report the file-based node identity +//! ([`crate::node_identity`], backed by `.huskies/node_identity.key`) — the +//! stable identity operators see logged at startup and configure in +//! `trusted_keys`. This is a distinct keypair from the CRDT document's +//! author id ([`crate::crdt_state::our_node_id`]); the two are not +//! interchangeable, so `/identity` never reports the CRDT id. +//! //! `GET /identity?nonce=` (story 1173) additionally signs the nonce with -//! the node's CRDT signing key ([`crate::crdt_state::sign_challenge`]) so a -//! caller can verify the response actually came from the sled it expects, -//! not just whichever container answered on that URL. The signing call -//! acquires the CRDT state mutex exactly once (lock → sign → unlock) with no -//! nested CRDT calls while held, avoiding the bug-1170 self-deadlock class. +//! this same file-based identity key +//! ([`crate::node_identity::sign_challenge_with_identity`]) so a caller can +//! verify the response actually came from the sled it expects, not just +//! whichever container answered on that URL — and so the signature always +//! verifies against the `node_id` reported alongside it. use poem::handler; use poem::web::{Json, Query}; @@ -18,7 +25,7 @@ use serde::{Deserialize, Serialize}; /// Query parameters accepted by `GET /identity`. #[derive(Deserialize)] pub struct IdentityQuery { - /// Optional challenge nonce to sign with the node's CRDT signing key. + /// Optional challenge nonce to sign with the node's identity key. #[serde(default)] pub nonce: Option, } @@ -26,46 +33,42 @@ pub struct IdentityQuery { /// JSON response body for `GET /identity`. #[derive(Serialize)] pub struct IdentityResponse { - /// Node ID: lowercase hex-encoding of the 32-byte Ed25519 public key. + /// Node ID: lowercase hex-encoding of the 32-byte Ed25519 public key + /// from the file-based node identity (`node_identity.key`). pub node_id: String, - /// Lowercase hex-encoding of the 32-byte Ed25519 public key. + /// Lowercase hex-encoding of the 32-byte Ed25519 public key. Always + /// equal to `node_id`. pub pubkey: String, - /// Ed25519 signature (hex) over `nonce`, made with the CRDT signing key. - /// `None` when no `nonce` was supplied, or before the CRDT layer has - /// initialised (in which case `node_id`/`pubkey` fall back to the - /// file-based node identity). + /// Ed25519 signature (hex) over `nonce`, made with the file-based node + /// identity's private key — the same key backing `node_id`/`pubkey`. + /// `None` when no `nonce` was supplied, or before the node identity has + /// initialised. #[serde(skip_serializing_if = "Option::is_none")] pub signature: Option, } /// `GET /identity` — return this node's Ed25519 public key, optionally -/// signing a caller-supplied nonce with the CRDT signing key. +/// signing a caller-supplied nonce with the same key. /// /// Returns `{"node_id": "<64-hex>", "pubkey": "<64-hex>", "signature": "<128-hex>"}` -/// when a `nonce` query parameter is given and the CRDT layer is -/// initialised. Otherwise omits `signature` and falls back to the -/// file-based node identity (legacy shape). No authentication required; +/// when a `nonce` query parameter is given and the node identity is +/// initialised. Otherwise omits `signature`. No authentication required; /// private keys are never exposed. #[handler] pub fn identity_handler( Query(IdentityQuery { nonce }): Query, ) -> Json { - if let Some(ref nonce) = nonce - && let Some((pubkey_hex, signature_hex)) = crate::crdt_state::sign_challenge(nonce) - { - return Json(IdentityResponse { - node_id: pubkey_hex.clone(), - pubkey: pubkey_hex, - signature: Some(signature_hex), - }); - } - match crate::node_identity::get_identity() { - Some(id) => Json(IdentityResponse { - node_id: id.node_id.clone(), - pubkey: id.pubkey_hex.clone(), - signature: None, - }), + Some(id) => { + let signature = nonce + .as_deref() + .and_then(crate::node_identity::sign_challenge_with_identity); + Json(IdentityResponse { + node_id: id.node_id.clone(), + pubkey: id.pubkey_hex.clone(), + signature, + }) + } None => Json(IdentityResponse { node_id: "uninitialized".to_string(), pubkey: "uninitialized".to_string(), @@ -101,8 +104,10 @@ mod tests { } #[tokio::test] - async fn identity_endpoint_signs_nonce_with_crdt_key_when_initialised() { - crate::crdt_state::init_for_test(); + async fn identity_endpoint_signs_nonce_with_identity_key() { + let tmp = tempfile::tempdir().unwrap(); + let key_path = tmp.path().join("node_identity.key"); + crate::node_identity::init_identity(&key_path).unwrap(); let app = Route::new().at("/identity", get(identity_handler)); let cli = TestClient::new(app); @@ -122,13 +127,20 @@ mod tests { crate::node_identity::verify_message_strict(pubkey, b"deadbeef", signature), "signature must verify against the returned pubkey and nonce" ); - // The signed node_id must be the CRDT node id, not the file-based one. - assert_eq!(node_id, crate::crdt_state::our_node_id().unwrap()); + // The reported node_id must match the identity that actually signed — + // the file-based node_identity.key, not the (possibly different) CRDT + // document id. + assert_eq!( + node_id, + crate::node_identity::get_identity().unwrap().node_id + ); } #[tokio::test] - async fn identity_endpoint_without_nonce_omits_signature_even_when_crdt_initialised() { - crate::crdt_state::init_for_test(); + async fn identity_endpoint_without_nonce_omits_signature() { + let tmp = tempfile::tempdir().unwrap(); + let key_path = tmp.path().join("node_identity.key"); + crate::node_identity::init_identity(&key_path).unwrap(); let app = Route::new().at("/identity", get(identity_handler)); let cli = TestClient::new(app); diff --git a/server/src/node_identity.rs b/server/src/node_identity.rs index 5ed0ea34..4c0e2d3a 100644 --- a/server/src/node_identity.rs +++ b/server/src/node_identity.rs @@ -161,16 +161,22 @@ pub struct NodeIdentity { /// Global node identity, initialised once at server startup. static IDENTITY: OnceLock = OnceLock::new(); -/// Load or create the node's Ed25519 keypair, storing it in a `0600` file. +/// Signing keypair backing [`IDENTITY`]. +/// +/// Kept in a separate `OnceLock` (rather than as a field on [`NodeIdentity`]) +/// so that [`get_identity`] can never hand callers access to key material — +/// only [`sign_challenge_with_identity`] can use it. +static SIGNING_KEYPAIR: OnceLock = OnceLock::new(); + +/// Load or create the node's Ed25519 signing key, storing it in a `0600` file. /// /// - **First boot**: generates a new keypair with `ed25519-dalek`, writes the -/// 32-byte signing-key seed to `path` with Unix mode `0600`, then returns -/// the derived `NodeIdentity`. -/// - **Subsequent boots**: reads the 32-byte seed from `path`, reconstructs -/// the keypair deterministically, and returns the same `NodeIdentity`. +/// 32-byte signing-key seed to `path` with Unix mode `0600`, then returns it. +/// - **Subsequent boots**: reads the 32-byte seed from `path` and reconstructs +/// the keypair deterministically. /// /// The file stores the raw 32-byte seed only. No headers, no PEM, no base64. -pub fn load_or_create_keypair_file(path: &std::path::Path) -> std::io::Result { +fn load_or_create_signing_key(path: &std::path::Path) -> std::io::Result { let signing_key = if path.exists() { let bytes = std::fs::read(path)?; let seed: [u8; 32] = bytes.try_into().map_err(|_| { @@ -216,8 +222,16 @@ pub fn load_or_create_keypair_file(path: &std::path::Path) -> std::io::Result std::io::Result { + let signing_key = load_or_create_signing_key(path)?; + let pubkey_hex = hex_encode(&signing_key.verifying_key().to_bytes()); Ok(NodeIdentity { node_id: pubkey_hex.clone(), pubkey_hex, @@ -229,8 +243,14 @@ pub fn load_or_create_keypair_file(path: &std::path::Path) -> std::io::Result std::io::Result<()> { if IDENTITY.get().is_none() { - let identity = load_or_create_keypair_file(path)?; + let signing_key = load_or_create_signing_key(path)?; + let pubkey_hex = hex_encode(&signing_key.verifying_key().to_bytes()); + let identity = NodeIdentity { + node_id: pubkey_hex.clone(), + pubkey_hex, + }; let _ = IDENTITY.set(identity); + let _ = SIGNING_KEYPAIR.set(signing_key); } Ok(()) } @@ -241,6 +261,18 @@ pub fn get_identity() -> Option<&'static NodeIdentity> { IDENTITY.get() } +/// Sign `nonce` with the file-based node identity's private key. +/// +/// Returns the signature (hex) if [`init_identity`] has been called; +/// `None` otherwise. The returned signature always verifies against +/// [`get_identity`]'s `pubkey_hex`, so callers of `GET /identity` can trust +/// that the reported `node_id` and the challenge-response signature refer +/// to the same key. +pub fn sign_challenge_with_identity(nonce: &str) -> Option { + let keypair = SIGNING_KEYPAIR.get()?; + Some(sign_challenge(keypair, nonce)) +} + // ── Internal helpers ────────────────────────────────────────────────── fn hex_encode(bytes: &[u8]) -> String {