mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-22 07:48:26 +00:00
Tighten TreeAnnounce validation to match spanning tree specification
Adds TreeAnnounce::validate_semantics() called from handle_tree_announce before any tree-state mutation. Enforces that the ancestry accompanying a parent declaration conforms to the spanning tree rules: - first ancestry entry matches the signed sender - is_root declarations carry a single-entry ancestry - non-root declarations include the signed parent as the second entry - the advertised root is the minimum node_addr in the ancestry Non-conforming announcements are rejected with a warn log and no state change. Adds unit tests for each rejected shape plus an integration test covering the full receive path in a two-node tree. Co-authored-by: Johnathan Corgan <johnathan@corganlabs.com>
This commit is contained in:
committed by
Johnathan Corgan
parent
7e002a3883
commit
b36966be3a
@@ -44,6 +44,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
bind with `EAFNOSUPPORT`
|
||||
([#61](https://github.com/jmcorgan/fips/issues/61),
|
||||
reported by [@SwapMarket](https://github.com/SwapMarket))
|
||||
- Tighten TreeAnnounce ancestry validation to match the spanning
|
||||
tree specification. The receive path now verifies that the
|
||||
ancestry is structurally consistent with the signed parent
|
||||
declaration before mutating tree state.
|
||||
|
||||
## [0.2.0] - 2026-03-22
|
||||
|
||||
|
||||
@@ -5,6 +5,8 @@
|
||||
//! reused by bloom filter tests.
|
||||
|
||||
use super::*;
|
||||
use crate::protocol::TreeAnnounce;
|
||||
use crate::tree::{CoordEntry, ParentDeclaration, TreeCoordinate};
|
||||
|
||||
/// A test node bundling a Node with its transport and packet channel.
|
||||
pub(super) struct TestNode {
|
||||
@@ -717,3 +719,74 @@ async fn test_spanning_tree_disconnected() {
|
||||
verify_tree_convergence_components(&nodes, &[vec![0, 1, 2], vec![3, 4, 5]]);
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
/// Tests that a node ignores a signed TreeAnnounce whose advertised root is not the smallest node_addr in the ancestry.
|
||||
#[tokio::test]
|
||||
async fn test_rejects_tree_announce_with_inconsistent_root() {
|
||||
// Start from a healthy 2-node tree so node B already has a normal, trusted
|
||||
// view of node A's coordinates.
|
||||
let mut nodes = run_tree_test(2, &[(0, 1)], false).await;
|
||||
|
||||
let a_addr = *nodes[0].node.node_addr();
|
||||
let current_root = *nodes[1].node.tree_state().root();
|
||||
let current_depth = nodes[1].node.tree_state().my_coords().depth();
|
||||
let peer_coords_before = nodes[1]
|
||||
.node
|
||||
.get_peer(&a_addr)
|
||||
.unwrap()
|
||||
.coords()
|
||||
.unwrap()
|
||||
.clone();
|
||||
let accepted_before = nodes[1].node.stats().tree.accepted;
|
||||
|
||||
// Use two fixed synthetic ancestors so the forged path is explicit:
|
||||
// - fake_parent = 00000000000000000000000000000000
|
||||
// - fake_root = 00000000000000000000000000000001
|
||||
//
|
||||
// The forged ancestry is therefore:
|
||||
// [A, 000...000, 000...001]
|
||||
//
|
||||
// That makes 000...001 the advertised root because it is the final entry,
|
||||
// even though 000...000 is smaller and appears earlier in the path.
|
||||
let fake_parent = NodeAddr::from_bytes([0u8; 16]);
|
||||
let mut fake_root_bytes = [0u8; 16];
|
||||
fake_root_bytes[15] = 1;
|
||||
let fake_root = NodeAddr::from_bytes(fake_root_bytes);
|
||||
|
||||
// Sign a fresh declaration from A. The 99/12345 values are just a newer
|
||||
// sequence/timestamp so the announce would be acceptable on freshness
|
||||
// grounds if its ancestry semantics were valid.
|
||||
let mut declaration = ParentDeclaration::new(a_addr, fake_parent, 99, 12345);
|
||||
declaration.sign(nodes[0].node.identity()).unwrap();
|
||||
|
||||
let announce = TreeAnnounce::new(
|
||||
declaration,
|
||||
TreeCoordinate::new(vec![
|
||||
CoordEntry::new(a_addr, 99, 12345),
|
||||
CoordEntry::new(fake_parent, 98, 12344),
|
||||
CoordEntry::new(fake_root, 97, 12343),
|
||||
])
|
||||
.unwrap(),
|
||||
);
|
||||
let encoded = announce.encode().unwrap();
|
||||
|
||||
nodes[1]
|
||||
.node
|
||||
.handle_tree_announce(&a_addr, &encoded[1..])
|
||||
.await;
|
||||
|
||||
// B should reject the malformed ancestry before mutating either its local
|
||||
// tree state or its cached view of peer A.
|
||||
assert_eq!(*nodes[1].node.tree_state().root(), current_root);
|
||||
assert_eq!(
|
||||
nodes[1].node.tree_state().my_coords().depth(),
|
||||
current_depth
|
||||
);
|
||||
assert_eq!(nodes[1].node.stats().tree.accepted, accepted_before);
|
||||
assert_eq!(
|
||||
nodes[1].node.get_peer(&a_addr).unwrap().coords().unwrap(),
|
||||
&peer_coords_before
|
||||
);
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
@@ -172,6 +172,15 @@ impl Node {
|
||||
return;
|
||||
}
|
||||
|
||||
if let Err(e) = announce.validate_semantics() {
|
||||
warn!(
|
||||
from = %self.peer_display_name(from),
|
||||
error = %e,
|
||||
"Rejected TreeAnnounce with invalid ancestry"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
use super::error::ProtocolError;
|
||||
use super::link::LinkMessageType;
|
||||
use crate::NodeAddr;
|
||||
use crate::tree::{CoordEntry, ParentDeclaration, TreeCoordinate};
|
||||
use crate::tree::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError};
|
||||
use secp256k1::schnorr::Signature;
|
||||
|
||||
/// Spanning tree announcement carrying parent declaration and ancestry.
|
||||
@@ -35,6 +35,58 @@ impl TreeAnnounce {
|
||||
}
|
||||
}
|
||||
|
||||
/// Validate that the ancestry is structurally consistent with the signed
|
||||
/// declaration.
|
||||
///
|
||||
/// Expected properties:
|
||||
/// - the first ancestry entry is the declaring node's `node_addr`
|
||||
/// - a root declaration has exactly one ancestry entry
|
||||
/// - a non-root declaration has at least two ancestry entries
|
||||
/// - for a non-root declaration, the second ancestry entry matches `parent_id`
|
||||
/// - the final ancestry entry is the advertised root
|
||||
/// - the advertised root is the smallest `node_addr` in the ancestry
|
||||
pub fn validate_semantics(&self) -> Result<(), TreeError> {
|
||||
let entries = self.ancestry.entries();
|
||||
let declared_node = *self.declaration.node_addr();
|
||||
let declared_parent = *self.declaration.parent_id();
|
||||
|
||||
if entries[0].node_addr != declared_node {
|
||||
return Err(TreeError::AncestryNodeMismatch {
|
||||
declared: declared_node,
|
||||
ancestry: entries[0].node_addr,
|
||||
});
|
||||
}
|
||||
|
||||
if self.declaration.is_root() {
|
||||
if entries.len() != 1 {
|
||||
return Err(TreeError::RootDeclarationMismatch);
|
||||
}
|
||||
} else {
|
||||
let ancestry_parent = entries.get(1).ok_or(TreeError::AncestryTooShort)?.node_addr;
|
||||
if ancestry_parent != declared_parent {
|
||||
return Err(TreeError::AncestryParentMismatch {
|
||||
declared: declared_parent,
|
||||
ancestry: ancestry_parent,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let advertised_root = *self.ancestry.root_id();
|
||||
let minimum = entries
|
||||
.iter()
|
||||
.map(|entry| entry.node_addr)
|
||||
.min()
|
||||
.expect("TreeCoordinate is never empty");
|
||||
if advertised_root != minimum {
|
||||
return Err(TreeError::AncestryRootNotMinimum {
|
||||
advertised: advertised_root,
|
||||
minimum,
|
||||
});
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Encode as link-layer plaintext (includes msg_type byte).
|
||||
///
|
||||
/// The declaration must be signed. The encoded format is:
|
||||
@@ -381,4 +433,142 @@ mod tests {
|
||||
let result = announce.encode();
|
||||
assert!(matches!(result, Err(ProtocolError::InvalidSignature)));
|
||||
}
|
||||
|
||||
/// Tests that a well-formed non-root ancestry is accepted.
|
||||
#[test]
|
||||
fn test_tree_announce_validate_semantics_accepts_valid_non_root() {
|
||||
use crate::identity::Identity;
|
||||
|
||||
let identity = Identity::generate();
|
||||
let node_addr = *identity.node_addr();
|
||||
let parent = make_node_addr(2);
|
||||
let root = make_node_addr(1);
|
||||
|
||||
let mut decl = ParentDeclaration::new(node_addr, parent, 5, 1000);
|
||||
decl.sign(&identity).unwrap();
|
||||
|
||||
let ancestry = TreeCoordinate::new(vec![
|
||||
CoordEntry::new(node_addr, 5, 1000),
|
||||
CoordEntry::new(parent, 4, 900),
|
||||
CoordEntry::new(root, 3, 800),
|
||||
])
|
||||
.unwrap();
|
||||
|
||||
let announce = TreeAnnounce::new(decl, ancestry);
|
||||
assert!(announce.validate_semantics().is_ok());
|
||||
}
|
||||
|
||||
/// Tests that an ancestry is rejected if the final node_addr is not the smallest entry in the path.
|
||||
#[test]
|
||||
fn test_tree_announce_validate_semantics_rejects_non_minimal_root() {
|
||||
use crate::identity::Identity;
|
||||
|
||||
let identity = Identity::generate();
|
||||
let node_addr = *identity.node_addr();
|
||||
let smaller = make_node_addr(0);
|
||||
let advertised_root = make_node_addr(1);
|
||||
|
||||
let mut decl = ParentDeclaration::new(node_addr, smaller, 5, 1000);
|
||||
decl.sign(&identity).unwrap();
|
||||
|
||||
let ancestry = TreeCoordinate::new(vec![
|
||||
CoordEntry::new(node_addr, 5, 1000),
|
||||
CoordEntry::new(smaller, 4, 900),
|
||||
CoordEntry::new(advertised_root, 3, 800),
|
||||
])
|
||||
.unwrap();
|
||||
|
||||
let announce = TreeAnnounce::new(decl, ancestry);
|
||||
assert!(matches!(
|
||||
announce.validate_semantics(),
|
||||
Err(TreeError::AncestryRootNotMinimum {
|
||||
advertised,
|
||||
minimum,
|
||||
}) if advertised == advertised_root && minimum == smaller
|
||||
));
|
||||
}
|
||||
|
||||
/// Tests that an ancestry is rejected if the first ancestry hop does not match the signed parent_id.
|
||||
#[test]
|
||||
fn test_tree_announce_validate_semantics_rejects_parent_mismatch() {
|
||||
use crate::identity::Identity;
|
||||
|
||||
let identity = Identity::generate();
|
||||
let node_addr = *identity.node_addr();
|
||||
let declared_parent = make_node_addr(2);
|
||||
let ancestry_parent = make_node_addr(3);
|
||||
|
||||
let mut decl = ParentDeclaration::new(node_addr, declared_parent, 5, 1000);
|
||||
decl.sign(&identity).unwrap();
|
||||
|
||||
let ancestry = TreeCoordinate::new(vec![
|
||||
CoordEntry::new(node_addr, 5, 1000),
|
||||
CoordEntry::new(ancestry_parent, 4, 900),
|
||||
CoordEntry::new(make_node_addr(1), 3, 800),
|
||||
])
|
||||
.unwrap();
|
||||
|
||||
let announce = TreeAnnounce::new(decl, ancestry);
|
||||
assert!(matches!(
|
||||
announce.validate_semantics(),
|
||||
Err(TreeError::AncestryParentMismatch {
|
||||
declared,
|
||||
ancestry,
|
||||
}) if declared == declared_parent && ancestry == ancestry_parent
|
||||
));
|
||||
}
|
||||
|
||||
/// Tests that an ancestry is rejected if the first path entry does not match the signed sender node_addr.
|
||||
#[test]
|
||||
fn test_tree_announce_validate_semantics_rejects_sender_mismatch() {
|
||||
use crate::identity::Identity;
|
||||
|
||||
let identity = Identity::generate();
|
||||
let node_addr = *identity.node_addr();
|
||||
let ancestry_sender = make_node_addr(9);
|
||||
let parent = make_node_addr(2);
|
||||
|
||||
let mut decl = ParentDeclaration::new(node_addr, parent, 5, 1000);
|
||||
decl.sign(&identity).unwrap();
|
||||
|
||||
let ancestry = TreeCoordinate::new(vec![
|
||||
CoordEntry::new(ancestry_sender, 5, 1000),
|
||||
CoordEntry::new(parent, 4, 900),
|
||||
CoordEntry::new(make_node_addr(1), 3, 800),
|
||||
])
|
||||
.unwrap();
|
||||
|
||||
let announce = TreeAnnounce::new(decl, ancestry);
|
||||
assert!(matches!(
|
||||
announce.validate_semantics(),
|
||||
Err(TreeError::AncestryNodeMismatch {
|
||||
declared,
|
||||
ancestry,
|
||||
}) if declared == node_addr && ancestry == ancestry_sender
|
||||
));
|
||||
}
|
||||
|
||||
/// Tests that a self-root declaration is rejected if its ancestry contains extra ancestors.
|
||||
#[test]
|
||||
fn test_tree_announce_validate_semantics_rejects_root_with_ancestors() {
|
||||
use crate::identity::Identity;
|
||||
|
||||
let identity = Identity::generate();
|
||||
let node_addr = *identity.node_addr();
|
||||
|
||||
let mut decl = ParentDeclaration::self_root(node_addr, 5, 1000);
|
||||
decl.sign(&identity).unwrap();
|
||||
|
||||
let ancestry = TreeCoordinate::new(vec![
|
||||
CoordEntry::new(node_addr, 5, 1000),
|
||||
CoordEntry::new(make_node_addr(0), 4, 900),
|
||||
])
|
||||
.unwrap();
|
||||
|
||||
let announce = TreeAnnounce::new(decl, ancestry);
|
||||
assert!(matches!(
|
||||
announce.validate_semantics(),
|
||||
Err(TreeError::RootDeclarationMismatch)
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -25,6 +25,34 @@ pub enum TreeError {
|
||||
#[error("invalid ancestry: does not reach claimed root")]
|
||||
AncestryNotToRoot,
|
||||
|
||||
#[error("invalid ancestry: root declaration must contain only the sender")]
|
||||
RootDeclarationMismatch,
|
||||
|
||||
#[error("invalid ancestry: non-root declaration must include a parent hop")]
|
||||
AncestryTooShort,
|
||||
|
||||
#[error("invalid ancestry: sender {declared} does not match first path entry {ancestry}")]
|
||||
AncestryNodeMismatch {
|
||||
declared: NodeAddr,
|
||||
ancestry: NodeAddr,
|
||||
},
|
||||
|
||||
#[error(
|
||||
"invalid ancestry: signed parent {declared} does not match first ancestry hop {ancestry}"
|
||||
)]
|
||||
AncestryParentMismatch {
|
||||
declared: NodeAddr,
|
||||
ancestry: NodeAddr,
|
||||
},
|
||||
|
||||
#[error(
|
||||
"invalid ancestry: advertised root {advertised} is not the minimum path entry {minimum}"
|
||||
)]
|
||||
AncestryRootNotMinimum {
|
||||
advertised: NodeAddr,
|
||||
minimum: NodeAddr,
|
||||
},
|
||||
|
||||
#[error("signature verification failed for node {0:?}")]
|
||||
InvalidSignature(NodeAddr),
|
||||
|
||||
|
||||
Reference in New Issue
Block a user