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

@@ -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=<hex>` (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<String>,
 }
@@ -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<String>,
 }
 
 /// `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<IdentityQuery>,
 ) -> Json<IdentityResponse> {
-    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);

server/src/node_identity.rs

@@ -161,16 +161,22 @@ pub struct NodeIdentity {
 /// Global node identity, initialised once at server startup.
 static IDENTITY: OnceLock<NodeIdentity> = 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<Ed25519KeyPair> = 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<NodeIdentity> {
+fn load_or_create_signing_key(path: &std::path::Path) -> std::io::Result<Ed25519KeyPair> {
     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<No
         sk
     };
 
-    let pubkey_bytes = signing_key.verifying_key().to_bytes();
-    let pubkey_hex = hex_encode(&pubkey_bytes);
+    Ok(signing_key)
+}
+
+/// Load or create the node's Ed25519 keypair, storing it in a `0600` file,
+/// and return its public [`NodeIdentity`].
+///
+/// See [`load_or_create_signing_key`] for the persistence behaviour.
+pub fn load_or_create_keypair_file(path: &std::path::Path) -> std::io::Result<NodeIdentity> {
+    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<No
 /// Should be called once at server startup.  Subsequent calls are no-ops.
 pub fn init_identity(path: &std::path::Path) -> 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<SignatureHex> {
+    let keypair = SIGNING_KEYPAIR.get()?;
+    Some(sign_challenge(keypair, nonce))
+}
+
 // ── Internal helpers ──────────────────────────────────────────────────
 
 fn hex_encode(bytes: &[u8]) -> String {