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:
archipelago
co-authored by
Claude Opus 5
Claude Opus 5
parent
ff2cb5aa0e
commit
c0cfc72a05
@@ -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.
|
||||
|
||||
@@ -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