diff --git a/crates/didbot-identity/src/document.rs b/crates/didbot-identity/src/document.rs index ea8324f6..b28f14b9 100644 --- a/crates/didbot-identity/src/document.rs +++ b/crates/didbot-identity/src/document.rs @@ -176,21 +176,46 @@ impl DidDocument { .is_some_and(|claimed| claimed.eq_ignore_ascii_case(handle)) } + /// Whether `id` is this document's own `fragment` entry. + /// + /// A DID URL is a DID plus a fragment, and the DID half is what says + /// whose entry it is. `ends_with` alone does not read it, so a document + /// for `did:web:victim.example` could list + /// `did:web:attacker.example#atproto` and have it answered as the + /// victim's signing key — a well-formed entry that does not belong to + /// the subject, accepted because the only field that says so was never + /// looked at. The relative form (`#atproto`, no DID at all) is the other + /// spelling the spec asks readers to accept, and it can only mean the + /// document's own subject. + fn owns_fragment(&self, id: &str, fragment: &str) -> bool { + matches!(id.strip_suffix(fragment), Some(base) if base.is_empty() || base == self.id) + } + /// The multibase signing key, taking the first `#atproto` entry of the /// correct type and ignoring any others, as the spec directs. + /// + /// The entry must also be this document's own: an `id` whose DID half is + /// this document's `id`, or absent entirely. pub fn signing_key_multibase(&self) -> Option<&str> { self.verification_method .iter() - .find(|vm| vm.id.ends_with(ATPROTO_KEY_FRAGMENT) && vm.type_ == MULTIKEY_TYPE) + .find(|vm| { + self.owns_fragment(&vm.id, ATPROTO_KEY_FRAGMENT) && vm.type_ == MULTIKEY_TYPE + }) .map(|vm| vm.public_key_multibase.as_str()) } /// The PDS endpoint, taking the first `#atproto_pds` entry of the correct /// type and ignoring any others. + /// + /// The entry must also be this document's own: an `id` whose DID half is + /// this document's `id`, or absent entirely. pub fn pds_endpoint(&self) -> Option<&str> { self.service .iter() - .find(|s| s.id.ends_with(ATPROTO_PDS_FRAGMENT) && s.type_ == PDS_SERVICE_TYPE) + .find(|s| { + self.owns_fragment(&s.id, ATPROTO_PDS_FRAGMENT) && s.type_ == PDS_SERVICE_TYPE + }) .map(|s| s.service_endpoint.as_str()) } } diff --git a/crates/didbot-identity/tests/identity.rs b/crates/didbot-identity/tests/identity.rs index 693ea4ef..72f12089 100644 --- a/crates/didbot-identity/tests/identity.rs +++ b/crates/didbot-identity/tests/identity.rs @@ -813,3 +813,46 @@ fn containing_picks_the_most_specific_zone() { assert_eq!(registry.containing("agent.pds.did.bot"), None); assert_eq!(registry.containing("agent.garden.zone"), None); } + +/// A key or a service entry belongs to whoever the DID half of its id names, +/// and a document does not get to answer for somebody else's. +/// +/// Matching on the fragment alone reads only half of a DID URL: a document +/// for one DID could list an entry qualified with another and have it +/// returned as its own, which is a well-formed key that does not match the +/// DID the caller asked about. +#[test] +fn an_entry_qualified_with_another_did_is_not_this_documents_own() { + let document: DidDocument = serde_json::from_str( + r##"{"@context": [], "id": "did:web:victim.example", + "verificationMethod": [{ + "id": "did:web:attacker.example#atproto", "type": "Multikey", + "controller": "did:web:attacker.example", + "publicKeyMultibase": "zAttackerKey"}], + "service": [{ + "id": "did:web:attacker.example#atproto_pds", + "type": "AtprotoPersonalDataServer", + "serviceEndpoint": "https://attacker.example"}]}"##, + ) + .expect("a document that parses"); + + assert_eq!(document.signing_key_multibase(), None); + assert_eq!(document.pds_endpoint(), None); + + // Both spellings the specification asks readers to accept, and nothing + // else: the document's own DID, and the bare relative fragment. + let own: DidDocument = serde_json::from_str( + r##"{"@context": [], "id": "did:web:victim.example", + "verificationMethod": [{ + "id": "did:web:victim.example#atproto", "type": "Multikey", + "controller": "did:web:victim.example", + "publicKeyMultibase": "zOwnKey"}], + "service": [{ + "id": "#atproto_pds", "type": "AtprotoPersonalDataServer", + "serviceEndpoint": "https://victim.example"}]}"##, + ) + .expect("a document that parses"); + + assert_eq!(own.signing_key_multibase(), Some("zOwnKey")); + assert_eq!(own.pds_endpoint(), Some("https://victim.example")); +}