fix(security): peers must not be able to grant themselves Trusted
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ff2cb5aa0e
commit
c0cfc72a05
@@ -191,6 +191,7 @@ pub async fn accept_invite(
|
||||
}
|
||||
|
||||
let node = FederatedNode {
|
||||
trust_source: Some(super::types::TrustSource::Invite),
|
||||
did: did.clone(),
|
||||
pubkey,
|
||||
onion,
|
||||
|
||||
@@ -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};
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -94,6 +94,40 @@ pub struct FederatedNode {
|
||||
/// with `last_sync_error` when the peer recovers.
|
||||
#[serde(default)]
|
||||
pub last_sync_error_at: Option<String>,
|
||||
/// 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<TrustSource>,
|
||||
}
|
||||
|
||||
/// 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(),
|
||||
|
||||
Reference in New Issue
Block a user