From c0cfc72a057bd559b6b779aa2a8ce9d26c4d540b Mon Sep 17 00:00:00 2001 From: archipelago Date: Mon, 3 Aug 2026 10:31:16 -0400 Subject: [PATCH] fix(security): peers must not be able to grant themselves Trusted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported: "peers seem to be slipping into trusted status somehow which is absolutely terrible for security". Two independent fail-open paths, both granting Trusted with no operator decision anywhere in the loop. 1. federation.peer-joined is UNAUTHENTICATED (middleware's no-session list — federated peers call it over Tor without cookies) and reachable on /rpc/v1, which is peer-allowed. It does verify an ed25519 signature, but against THE PUBKEY THE CALLER SUPPLIED, so it proves the caller holds its own key and nothing about whether we ever invited it. A join presenting no invite_token fell through to None => TrustLevel::Trusted.min(claimed_trust) and claimed_trust itself defaults to Trusted when the field is absent. So anything able to reach the node could generate a keypair, omit the token, and be recorded as Trusted. Now capped at Observer: an invite WE minted is the only path to Trusted. `min` is kept so a peer's own lower claim is still honoured — this can only ever reduce trust. 2. merge_transitive_peers added every peer advertised by a Trusted source as Trusted. That makes trust viral rather than transitive-by-one-hop: the merged node is itself synced with, its peers merged in turn, so a single invite anywhere in the graph eventually marked the entire graph Trusted on every node. Now Observer — which is what this feature's own spec always said. NodeStateSnapshot.federated_peers is documented as "adds them as Observers on her side… doesn't auto-promote Observer-via- Bob to Trusted". The code contradicted the comment directly above it. Observer is deliberate rather than Untrusted: the merge exists for routing, and Observer still passes the `!= Untrusted` gates that federation, DWN and messaging actually check, so a legacy peer degrades instead of breaking. Per the operator's decision, existing peers are NOT auto-demoted — silently rewriting live trust relationships across the fleet would be worse than the bug. Instead they are made auditable: FederatedNode.trust_source records WHY a level was granted (invite | uninvited-join | transitive-merge | manual). It deliberately has no default provenance — None means "recorded before this existed", which is exactly the population worth reviewing. The one failing test was asserting the vulnerable behaviour (merge_transitive_peers_skips_source_and_local_node expected Trusted); it now asserts the security property and says why, so the escalation cannot be reintroduced by making a test go green. Verified: 42/42 federation tests, cargo check --all-targets clean. Still open, tracked in .planning/RELEASE-1.7.121-TASKS.md: surface trust_source in the UI, and require the node password to grant Trusted. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/api/rpc/federation/handlers.rs | 32 ++++++++++++++-- core/archipelago/src/federation/invites.rs | 1 + core/archipelago/src/federation/mod.rs | 2 +- core/archipelago/src/federation/storage.rs | 1 + core/archipelago/src/federation/sync.rs | 38 +++++++++++++++++-- core/archipelago/src/federation/types.rs | 35 +++++++++++++++++ 6 files changed, 101 insertions(+), 8 deletions(-) diff --git a/core/archipelago/src/api/rpc/federation/handlers.rs b/core/archipelago/src/api/rpc/federation/handlers.rs index 7a60257d..c3b01efe 100644 --- a/core/archipelago/src/api/rpc/federation/handlers.rs +++ b/core/archipelago/src/api/rpc/federation/handlers.rs @@ -565,9 +565,27 @@ impl RpcHandler { }), None => None, }; - let granted_trust = match invite_trust { - Some(level) => level, - None => TrustLevel::Trusted.min(claimed_trust), + // An invite WE minted is the only thing that may grant Trusted. + // + // This handler is unauthenticated (see middleware.rs: federated peers + // call it over Tor with no session) and reachable on /rpc/v1. Its + // signature check proves only that the caller holds the private key for + // the pubkey IT SUPPLIED — anyone can generate a keypair — so it + // establishes identity, never authorisation. Defaulting an unmatched + // join to Trusted therefore let any party that could reach the node + // self-grant Trusted by simply omitting `invite_token`. + // + // Capped at Observer instead: still recorded, still reachable, still + // passes the `!= Untrusted` gates that federation/DWN/messaging use, so + // a legacy peer re-joining degrades rather than breaks — but it cannot + // reach a level the operator never granted. `min` keeps a peer's own + // lower claim honoured, so this can only ever reduce trust. + let (granted_trust, trust_source) = match invite_trust { + Some(level) => (level, federation::TrustSource::Invite), + None => ( + TrustLevel::Observer.min(claimed_trust), + federation::TrustSource::UninvitedJoin, + ), }; // Reject self-peering. If somehow our own did / onion / pubkey @@ -671,10 +689,16 @@ impl RpcHandler { last_transport_at: None, last_sync_error: None, last_sync_error_at: None, + trust_source: Some(trust_source), }; federation::add_node(&self.config.data_dir, node).await?; - info!(peer_did = %did, trust = %granted_trust, "Peer joined our federation"); + info!( + peer_did = %did, + trust = %granted_trust, + source = ?trust_source, + "Peer joined our federation" + ); // Mirror into mesh state so the inbound peer is addressable from // the chat UI without waiting for the next mesh restart. diff --git a/core/archipelago/src/federation/invites.rs b/core/archipelago/src/federation/invites.rs index 9aff3dca..41df2cff 100644 --- a/core/archipelago/src/federation/invites.rs +++ b/core/archipelago/src/federation/invites.rs @@ -191,6 +191,7 @@ pub async fn accept_invite( } let node = FederatedNode { + trust_source: Some(super::types::TrustSource::Invite), did: did.clone(), pubkey, onion, diff --git a/core/archipelago/src/federation/mod.rs b/core/archipelago/src/federation/mod.rs index 7be272e3..44b781e2 100644 --- a/core/archipelago/src/federation/mod.rs +++ b/core/archipelago/src/federation/mod.rs @@ -24,4 +24,4 @@ pub use storage::{ record_sync_result, remove_node, save_nodes, set_trust_level, update_node, }; pub use sync::{build_local_state, deploy_to_peer, sync_with_peer, sync_with_peer_by_did}; -pub use types::{AppStatus, FederatedNode, NodeStateSnapshot, TrustLevel}; +pub use types::{AppStatus, FederatedNode, NodeStateSnapshot, TrustLevel, TrustSource}; diff --git a/core/archipelago/src/federation/storage.rs b/core/archipelago/src/federation/storage.rs index 0db046bd..57e2dcfa 100644 --- a/core/archipelago/src/federation/storage.rs +++ b/core/archipelago/src/federation/storage.rs @@ -487,6 +487,7 @@ mod tests { fn make_node(did: &str, onion: &str) -> FederatedNode { FederatedNode { + trust_source: None, did: did.to_string(), pubkey: "aabbccdd".to_string(), onion: onion.to_string(), diff --git a/core/archipelago/src/federation/sync.rs b/core/archipelago/src/federation/sync.rs index d961f069..aa97f52e 100644 --- a/core/archipelago/src/federation/sync.rs +++ b/core/archipelago/src/federation/sync.rs @@ -175,12 +175,26 @@ async fn merge_transitive_peers( continue; } } + // TRUST IS NOT TRANSITIVE. This peer was advertised to us by a Trusted + // source; we have no relationship with it and the operator has never + // seen it. Granting Trusted here made trust viral: once merged at + // Trusted, this node is itself synced with, its advertised peers are + // merged in turn, and one federation invite anywhere in the graph + // eventually marked the whole graph Trusted on every node. + // + // Observer is what the merge actually needs — the stated purpose is + // routing ("so we can route directly to them over FIPS without a second + // invite hop"), and Observer is reachable/syncable while being barred + // from expanding the federation further on its own authority (the + // guard at the call site checks for Trusted). Promotion stays an + // operator action. nodes.push(FederatedNode { did: hint.did.clone(), pubkey: hint.pubkey.clone(), onion: hint.onion.clone(), name: hint.name.clone(), - trust_level: TrustLevel::Trusted, + trust_level: TrustLevel::Observer, + trust_source: Some(super::types::TrustSource::TransitiveMerge), added_at: chrono::Utc::now().to_rfc3339(), last_seen: None, last_state: None, @@ -366,6 +380,7 @@ mod tests { fn build_local_state_filters_non_trusted_peers() { let peers = vec![ FederatedNode { + trust_source: None, did: "did:key:zTrusted".into(), pubkey: "aa".into(), onion: "t.onion".into(), @@ -381,6 +396,7 @@ mod tests { last_sync_error_at: None, }, FederatedNode { + trust_source: None, did: "did:key:zObserver".into(), pubkey: "bb".into(), onion: "o.onion".into(), @@ -396,6 +412,7 @@ mod tests { last_sync_error_at: None, }, FederatedNode { + trust_source: None, did: "did:key:zUntrusted".into(), pubkey: "cc".into(), onion: "u.onion".into(), @@ -440,6 +457,7 @@ mod tests { super::super::storage::save_nodes( dir.path(), &[FederatedNode { + trust_source: None, did: "did:key:zSource".into(), pubkey: "aa".into(), onion: "source.onion".into(), @@ -495,9 +513,23 @@ mod tests { let peer = nodes .iter() .find(|n| n.did == "did:key:zPeer") - .expect("trusted transitive peer should be added"); + .expect("transitive peer should be added (routing needs it)"); assert_eq!(peer.name.as_deref(), Some("Kitchen")); - assert_eq!(peer.trust_level, TrustLevel::Trusted); + // TRUST IS NOT TRANSITIVE. This peer was advertised by a Trusted source; + // the operator has never seen it. It is added so we can route to it, at + // Observer — never Trusted. This assertion previously read `Trusted` and + // was pinning the escalation in place: one invite anywhere in the graph + // eventually marked the entire graph Trusted on every node. + assert_eq!( + peer.trust_level, + TrustLevel::Observer, + "a transitively-discovered peer must never be auto-Trusted" + ); + assert_eq!( + peer.trust_source, + Some(super::super::types::TrustSource::TransitiveMerge), + "provenance must be recorded so the operator can audit it" + ); assert_eq!(peer.fips_npub.as_deref(), Some("npub1peer")); } } diff --git a/core/archipelago/src/federation/types.rs b/core/archipelago/src/federation/types.rs index 0758643e..4f1c9941 100644 --- a/core/archipelago/src/federation/types.rs +++ b/core/archipelago/src/federation/types.rs @@ -94,6 +94,40 @@ pub struct FederatedNode { /// with `last_sync_error` when the peer recovers. #[serde(default)] pub last_sync_error_at: Option, + /// HOW this peer's trust level came to be what it is. + /// + /// `None` means "recorded before this field existed" — which is exactly + /// the population an operator needs to audit, because it is the set that + /// may have been granted Trusted by the two fail-open paths this field was + /// added to close (an uninvited `federation.peer-joined`, and transitive + /// merge). It deliberately does NOT default to a made-up provenance: an + /// unknown origin must read as unknown, not as `Invite`. + #[serde(default)] + pub trust_source: Option, +} + +/// Why a federated node holds the trust level it does. +/// +/// Trust must be traceable to an operator decision. Anything that is not is a +/// candidate for review, which is what makes this worth persisting rather than +/// logging. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum TrustSource { + /// Matched an invite THIS node minted — the only path that may grant + /// `Trusted`. The level is the one the operator chose when minting. + Invite, + /// Joined via `federation.peer-joined` without presenting an invite token + /// we recognise. Capped at `Observer`: the caller is unauthenticated and + /// its signature only proves it holds the key it just supplied, never that + /// the operator ever invited it. + UninvitedJoin, + /// Learned from a Trusted peer's advertised peer list (transitive merge). + /// Capped at `Observer`: trust is not transitive, and a peer must not be + /// able to expand our trusted set on its own authority. + TransitiveMerge, + /// Set explicitly by the operator through the federation UI/RPC. + Manual, } /// State snapshot received from a federated peer during sync. @@ -210,6 +244,7 @@ mod tests { #[test] fn test_federated_node_serialization_roundtrip() { let node = FederatedNode { + trust_source: None, did: "did:key:zABC".to_string(), pubkey: "aabbccdd".to_string(), onion: "test.onion".to_string(),