mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
Fix LookupResponse proof verification
Include target_coords in proof_bytes() signed data to prevent transit nodes from substituting fake coordinates. Add mandatory signature verification at the originator using the target's public key from identity_cache (guaranteed available since lookups are only initiated from contexts where the key is already cached). Verification failure discards the response. Identity cache miss (should never happen) logs an error and discards. Add four new tests covering verification success, failure, cache miss, and coordinate substitution detection.
This commit is contained in:
@@ -6,8 +6,8 @@
|
||||
|
||||
use crate::node::{Node, RecentRequest};
|
||||
use crate::protocol::{LookupRequest, LookupResponse};
|
||||
use crate::NodeAddr;
|
||||
use tracing::{debug, trace};
|
||||
use crate::{NodeAddr, PeerIdentity};
|
||||
use tracing::{debug, error, trace, warn};
|
||||
|
||||
impl Node {
|
||||
/// Handle an incoming LookupRequest from a peer.
|
||||
@@ -92,7 +92,7 @@ impl Node {
|
||||
/// Processing steps:
|
||||
/// 1. Decode and validate
|
||||
/// 2. Check recent_requests to determine if we originated or are forwarding
|
||||
/// 3. If originator: cache target_coords in coord_cache
|
||||
/// 3. If originator: verify proof signature, then cache target_coords in coord_cache
|
||||
/// 4. If transit: reverse-path forward to from_peer
|
||||
pub(in crate::node) async fn handle_lookup_response(
|
||||
&mut self,
|
||||
@@ -130,15 +130,48 @@ impl Node {
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// We originated this request — cache the discovered coordinates
|
||||
// We originated this request — verify proof before caching
|
||||
let target = response.target;
|
||||
|
||||
// Look up the target's public key from identity_cache
|
||||
let mut prefix = [0u8; 15];
|
||||
prefix.copy_from_slice(&target.as_bytes()[0..15]);
|
||||
let target_pubkey = match self.lookup_by_fips_prefix(&prefix) {
|
||||
Some((_addr, pubkey)) => pubkey,
|
||||
None => {
|
||||
error!(
|
||||
request_id = response.request_id,
|
||||
target = %self.peer_display_name(&target),
|
||||
"identity_cache miss for lookup target — this is a bug"
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
// Verify the proof signature
|
||||
let (xonly, _parity) = target_pubkey.x_only_public_key();
|
||||
let peer_id = PeerIdentity::from_pubkey(xonly);
|
||||
let proof_data = LookupResponse::proof_bytes(
|
||||
response.request_id,
|
||||
&target,
|
||||
&response.target_coords,
|
||||
);
|
||||
if !peer_id.verify(&proof_data, &response.proof) {
|
||||
warn!(
|
||||
request_id = response.request_id,
|
||||
target = %self.peer_display_name(&target),
|
||||
"LookupResponse proof verification failed, discarding"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
debug!(
|
||||
request_id = response.request_id,
|
||||
target = %self.peer_display_name(&response.target),
|
||||
target = %self.peer_display_name(&target),
|
||||
depth = response.target_coords.depth(),
|
||||
"Received LookupResponse, caching route"
|
||||
"Received LookupResponse, proof verified, caching route"
|
||||
);
|
||||
|
||||
let target = response.target;
|
||||
self.coord_cache.insert(
|
||||
target,
|
||||
response.target_coords,
|
||||
@@ -182,7 +215,7 @@ impl Node {
|
||||
let our_coords = self.tree_state().my_coords().clone();
|
||||
|
||||
// Sign proof: Identity::sign hashes with SHA-256 internally
|
||||
let proof_data = LookupResponse::proof_bytes(request.request_id, &request.target);
|
||||
let proof_data = LookupResponse::proof_bytes(request.request_id, &request.target, &our_coords);
|
||||
let proof = self.identity().sign(&proof_data);
|
||||
|
||||
let response = LookupResponse::new(
|
||||
|
||||
+178
-6
@@ -117,13 +117,18 @@ async fn test_response_decode_error() {
|
||||
async fn test_response_originator_caches_route() {
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
let target = make_node_addr(0xBB);
|
||||
|
||||
// Use the target identity's actual node_addr for consistency
|
||||
let target_identity = Identity::generate();
|
||||
let target = *target_identity.node_addr();
|
||||
let root = make_node_addr(0xF0);
|
||||
let coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap();
|
||||
|
||||
// Create a valid response with a real proof signature
|
||||
let proof_data = LookupResponse::proof_bytes(555, &target);
|
||||
let target_identity = Identity::generate();
|
||||
// Register target identity in cache so verification can find it
|
||||
node.register_identity(target, target_identity.pubkey_full());
|
||||
|
||||
// Create a valid response with a real proof signature (includes coords)
|
||||
let proof_data = LookupResponse::proof_bytes(555, &target, &coords);
|
||||
let proof = target_identity.sign(&proof_data);
|
||||
|
||||
let response = LookupResponse::new(555, target, coords.clone(), proof);
|
||||
@@ -151,7 +156,8 @@ async fn test_response_transit_needs_recent_request() {
|
||||
let root = make_node_addr(0xF0);
|
||||
let coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap();
|
||||
|
||||
let proof_data = LookupResponse::proof_bytes(444, &target);
|
||||
// Transit nodes don't verify proofs, so any valid signature suffices
|
||||
let proof_data = LookupResponse::proof_bytes(444, &target, &coords);
|
||||
let target_identity = Identity::generate();
|
||||
let proof = target_identity.sign(&proof_data);
|
||||
|
||||
@@ -180,6 +186,151 @@ async fn test_response_transit_needs_recent_request() {
|
||||
assert!(!node.coord_cache().contains(&target, now_ms2));
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Unit Tests — LookupResponse Proof Verification
|
||||
// ============================================================================
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_response_proof_verification_success() {
|
||||
// Verify that a properly signed response is accepted and cached
|
||||
// when the origin has the target's pubkey in identity_cache.
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
|
||||
let target_identity = Identity::generate();
|
||||
let target = *target_identity.node_addr();
|
||||
let root = make_node_addr(0xF0);
|
||||
let coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap();
|
||||
|
||||
// Register target in identity_cache
|
||||
node.register_identity(target, target_identity.pubkey_full());
|
||||
|
||||
// Sign with correct proof_bytes (including coords)
|
||||
let proof_data = LookupResponse::proof_bytes(700, &target, &coords);
|
||||
let proof = target_identity.sign(&proof_data);
|
||||
|
||||
let response = LookupResponse::new(700, target, coords.clone(), proof);
|
||||
let payload = &response.encode()[1..];
|
||||
|
||||
node.handle_lookup_response(&from, payload).await;
|
||||
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
assert!(
|
||||
node.coord_cache().contains(&target, now_ms),
|
||||
"Valid proof should result in cached coords"
|
||||
);
|
||||
assert_eq!(node.coord_cache().get(&target, now_ms).unwrap(), &coords);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_response_proof_verification_failure() {
|
||||
// Verify that a response with a bad signature is discarded.
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
|
||||
let target_identity = Identity::generate();
|
||||
let target = *target_identity.node_addr();
|
||||
let root = make_node_addr(0xF0);
|
||||
let coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap();
|
||||
|
||||
// Register target in identity_cache
|
||||
node.register_identity(target, target_identity.pubkey_full());
|
||||
|
||||
// Sign with a DIFFERENT identity (wrong key)
|
||||
let wrong_identity = Identity::generate();
|
||||
let proof_data = LookupResponse::proof_bytes(701, &target, &coords);
|
||||
let proof = wrong_identity.sign(&proof_data);
|
||||
|
||||
let response = LookupResponse::new(701, target, coords, proof);
|
||||
let payload = &response.encode()[1..];
|
||||
|
||||
node.handle_lookup_response(&from, payload).await;
|
||||
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
assert!(
|
||||
!node.coord_cache().contains(&target, now_ms),
|
||||
"Bad signature should NOT result in cached coords"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_response_identity_cache_miss() {
|
||||
// Verify that a response is discarded when the origin lacks the
|
||||
// target's pubkey in identity_cache (should never happen in practice).
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
|
||||
let target_identity = Identity::generate();
|
||||
let target = *target_identity.node_addr();
|
||||
let root = make_node_addr(0xF0);
|
||||
let coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap();
|
||||
|
||||
// Do NOT register target in identity_cache
|
||||
|
||||
let proof_data = LookupResponse::proof_bytes(702, &target, &coords);
|
||||
let proof = target_identity.sign(&proof_data);
|
||||
|
||||
let response = LookupResponse::new(702, target, coords, proof);
|
||||
let payload = &response.encode()[1..];
|
||||
|
||||
node.handle_lookup_response(&from, payload).await;
|
||||
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
assert!(
|
||||
!node.coord_cache().contains(&target, now_ms),
|
||||
"identity_cache miss should discard the response"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_response_coord_substitution_detected() {
|
||||
// Verify that if the proof was signed with correct coords but
|
||||
// different coords are placed in the response, verification fails.
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
|
||||
let target_identity = Identity::generate();
|
||||
let target = *target_identity.node_addr();
|
||||
let root = make_node_addr(0xF0);
|
||||
let real_coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap();
|
||||
let fake_coords = TreeCoordinate::from_addrs(vec![
|
||||
target,
|
||||
make_node_addr(0xEE),
|
||||
root,
|
||||
]).unwrap();
|
||||
|
||||
// Register target in identity_cache
|
||||
node.register_identity(target, target_identity.pubkey_full());
|
||||
|
||||
// Sign proof with real coords
|
||||
let proof_data = LookupResponse::proof_bytes(703, &target, &real_coords);
|
||||
let proof = target_identity.sign(&proof_data);
|
||||
|
||||
// But construct the response with FAKE coords
|
||||
let response = LookupResponse::new(703, target, fake_coords, proof);
|
||||
let payload = &response.encode()[1..];
|
||||
|
||||
node.handle_lookup_response(&from, payload).await;
|
||||
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
assert!(
|
||||
!node.coord_cache().contains(&target, now_ms),
|
||||
"Substituted coords should be detected and response discarded"
|
||||
);
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Unit Tests — RecentRequest Expiry
|
||||
// ============================================================================
|
||||
@@ -304,6 +455,11 @@ async fn test_request_three_node_chain() {
|
||||
let mut nodes = run_tree_test(3, &edges, false).await;
|
||||
|
||||
let node2_addr = *nodes[2].node.node_addr();
|
||||
let node2_pubkey = nodes[2].node.identity().pubkey_full();
|
||||
|
||||
// Pre-populate node0's identity_cache with node2's identity
|
||||
// (in production, DNS resolution or prior handshake would do this)
|
||||
nodes[0].node.register_identity(node2_addr, node2_pubkey);
|
||||
|
||||
// Node0 initiates lookup (doesn't record in recent_requests)
|
||||
nodes[0].node.initiate_lookup(&node2_addr, 8).await;
|
||||
@@ -396,11 +552,27 @@ async fn test_discovery_100_nodes() {
|
||||
let mut nodes = run_tree_test(NUM_NODES, &edges, false).await;
|
||||
verify_tree_convergence(&nodes);
|
||||
|
||||
// Collect all node addresses for lookup targets
|
||||
// Collect all node addresses and public keys for lookup targets
|
||||
let all_addrs: Vec<NodeAddr> = nodes
|
||||
.iter()
|
||||
.map(|tn| *tn.node.node_addr())
|
||||
.collect();
|
||||
let all_pubkeys: Vec<secp256k1::PublicKey> = nodes
|
||||
.iter()
|
||||
.map(|tn| tn.node.identity().pubkey_full())
|
||||
.collect();
|
||||
|
||||
// Pre-populate identity caches: each source needs the target's pubkey
|
||||
// for proof verification. In production, DNS resolution populates this
|
||||
// before lookups are initiated.
|
||||
for (src, node) in nodes.iter_mut().enumerate() {
|
||||
for dst in (0..NUM_NODES).step_by(10) {
|
||||
if src == dst {
|
||||
continue;
|
||||
}
|
||||
node.node.register_identity(all_addrs[dst], all_pubkeys[dst]);
|
||||
}
|
||||
}
|
||||
|
||||
// Each node looks up every 10th other node (~10 targets per node).
|
||||
// Build the full list of (src, dst) pairs.
|
||||
|
||||
@@ -200,11 +200,13 @@ impl LookupResponse {
|
||||
|
||||
/// Get the bytes that should be signed as proof.
|
||||
///
|
||||
/// Format: request_id (8) || target (16)
|
||||
pub fn proof_bytes(request_id: u64, target: &NodeAddr) -> Vec<u8> {
|
||||
let mut bytes = Vec::with_capacity(24);
|
||||
/// Format: request_id (8) || target (16) || coords_encoding (2 + 16×n)
|
||||
pub fn proof_bytes(request_id: u64, target: &NodeAddr, target_coords: &TreeCoordinate) -> Vec<u8> {
|
||||
let coord_size = 2 + target_coords.entries().len() * 16;
|
||||
let mut bytes = Vec::with_capacity(24 + coord_size);
|
||||
bytes.extend_from_slice(&request_id.to_le_bytes());
|
||||
bytes.extend_from_slice(target.as_bytes());
|
||||
encode_coords(target_coords, &mut bytes);
|
||||
bytes
|
||||
}
|
||||
|
||||
@@ -329,11 +331,17 @@ mod tests {
|
||||
#[test]
|
||||
fn test_lookup_response_proof_bytes() {
|
||||
let target = make_node_addr(42);
|
||||
let bytes = LookupResponse::proof_bytes(12345, &target);
|
||||
let coords = make_coords(&[42, 1, 0]);
|
||||
let bytes = LookupResponse::proof_bytes(12345, &target, &coords);
|
||||
|
||||
assert_eq!(bytes.len(), 24); // 8 + 16
|
||||
// 8 (request_id) + 16 (target) + 2 (count) + 3*16 (coords) = 74
|
||||
assert_eq!(bytes.len(), 74);
|
||||
assert_eq!(&bytes[0..8], &12345u64.to_le_bytes());
|
||||
assert_eq!(&bytes[8..24], target.as_bytes());
|
||||
|
||||
// Verify coordinate encoding is present
|
||||
let count = u16::from_le_bytes([bytes[24], bytes[25]]);
|
||||
assert_eq!(count, 3); // 3 entries in coords
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -372,7 +380,7 @@ mod tests {
|
||||
// Create a dummy signature for testing
|
||||
let secp = Secp256k1::new();
|
||||
let keypair = secp256k1::Keypair::new(&secp, &mut rand::thread_rng());
|
||||
let proof_data = LookupResponse::proof_bytes(999, &target);
|
||||
let proof_data = LookupResponse::proof_bytes(999, &target, &coords);
|
||||
use sha2::Digest;
|
||||
let digest: [u8; 32] = sha2::Sha256::digest(&proof_data).into();
|
||||
let sig = secp.sign_schnorr(&digest, &keypair);
|
||||
|
||||
Reference in New Issue
Block a user