diff --git a/Cargo.lock b/Cargo.lock index b7050bc6..e8409e18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3497,6 +3497,7 @@ dependencies = [ "chrono", "clap", "dirs", + "ed25519-dalek", "gitlawb-core", "icaptcha-client", "mockito", diff --git a/crates/gitlawb-attest/src/attestation.rs b/crates/gitlawb-attest/src/attestation.rs index 0e29e3a9..5b55ab77 100644 --- a/crates/gitlawb-attest/src/attestation.rs +++ b/crates/gitlawb-attest/src/attestation.rs @@ -105,16 +105,30 @@ impl Attestation { let bytes = canonical_signing_bytes(&self.type_, &self.payload, &self.cert_hash)?; let vk = verifying_key_from_did_key(&self.signer)?; + self.verify_sig_with_key(&vk, &bytes)?; + + Ok(vk) + } + + /// The signature check itself, against a key the caller supplies. + /// + /// Split out so the STRICTNESS of the check stays provable on its own. + /// `verifying_key_from_did_key` now refuses a small-order key, so a weak + /// signer can no longer reach this line through `verify_signature`. Without + /// this seam there would be nothing left that fails when `verify_strict` is + /// downgraded to the non-strict `verify`, and that regression would land + /// silently. Check order in `verify_signature` is unchanged. + fn verify_sig_with_key(&self, vk: &VerifyingKey, bytes: &[u8]) -> Result<()> { let sig_bytes: [u8; 64] = B64U .decode(&self.sig) .map_err(|e| Error::Signature(format!("base64url: {e}")))? .try_into() .map_err(|_| Error::Signature("signature must be 64 bytes".to_string()))?; let sig = Signature::from_bytes(&sig_bytes); - vk.verify_strict(&bytes, &sig) + vk.verify_strict(bytes, &sig) .map_err(|e| Error::Signature(format!("ed25519: {e}")))?; - Ok(vk) + Ok(()) } /// Reparse `payload` as `P`. Errors if the type discriminator does not @@ -181,7 +195,7 @@ fn canonical_signing_bytes( Ok(out) } -fn did_key_from_verifying_key(key: &VerifyingKey) -> String { +pub(crate) fn did_key_from_verifying_key(key: &VerifyingKey) -> String { let mut buf = Vec::with_capacity(ED25519_MULTICODEC.len() + 32); buf.extend_from_slice(&ED25519_MULTICODEC); buf.extend_from_slice(&key.to_bytes()); @@ -202,6 +216,19 @@ fn verifying_key_from_did_key(did: &str) -> Result { "did:key must use base58btc (z-prefix): {did}" ))); } + // Refuse an oversized id before decoding. base58 decoding is quadratic in + // its input and `signer` comes off an attacker-supplied attestation, so a + // short request could otherwise buy a large decode. An ed25519 did:key + // method-id is a fixed 48 characters, so this bound is slack rather than a + // behavior change, and it only helps ahead of the decode. Mirrors the same + // cap in gitlawb-core's Did::to_verifying_key, which this crate cannot call + // (gitlawb-core is a dev-dependency here). + const MAX_METHOD_ID_LEN: usize = 64; + if method_id.len() > MAX_METHOD_ID_LEN { + return Err(Error::Did( + "did:key method-specific id too long".to_string(), + )); + } let (base, bytes) = multibase::decode(method_id).map_err(|e| Error::Did(format!("multibase: {e}")))?; if base != multibase::Base::Base58Btc { @@ -217,8 +244,20 @@ fn verifying_key_from_did_key(did: &str) -> Result { let key_bytes: [u8; 32] = bytes[ED25519_MULTICODEC.len()..] .try_into() .expect("length checked above"); - VerifyingKey::from_bytes(&key_bytes) - .map_err(|e| Error::Did(format!("invalid ed25519 key: {e}"))) + let key = VerifyingKey::from_bytes(&key_bytes) + .map_err(|e| Error::Did(format!("invalid ed25519 key: {e}")))?; + + // `from_bytes` only decompresses, so it accepts a small-order point. This + // crate parses did:key itself rather than going through gitlawb-core's + // `Did`, and it cannot go through it (gitlawb-core is a dev-dependency + // here), so the rejection is mirrored rather than inherited. Signature + // verification is already strict, so this is defense in depth against a + // future consumer of this parser that does not verify strictly. + if key.is_weak() { + return Err(Error::Did("small-order ed25519 key".to_string())); + } + + Ok(key) } #[cfg(test)] @@ -559,6 +598,111 @@ mod tests { /// weak (small-order) public key satisfies the verification equation /// but must be rejected. The identity point is such a key: with R the /// identity and S = 0, [S]B - [k]A is the identity for any message. + /// gitlawb-attest parses did:key itself and never routes through + /// gitlawb-core's `Did`, which it cannot: gitlawb-core is a dev-dependency + /// here. So the choke-point rejection has to be mirrored in this parser or + /// this crate keeps handing out keys the rest of the system rejects. + #[test] + fn verifying_key_from_did_key_rejects_a_small_order_key() { + let mut weak = [0u8; 32]; + weak[0] = 1; // compressed identity point + let mut buf = Vec::with_capacity(ED25519_MULTICODEC.len() + 32); + buf.extend_from_slice(&ED25519_MULTICODEC); + buf.extend_from_slice(&weak); + let did = format!( + "did:key:{}", + multibase::encode(multibase::Base::Base58Btc, &buf) + ); + + let err = + verifying_key_from_did_key(&did).expect_err("a small-order did:key must not resolve"); + match err { + Error::Did(msg) => assert!( + msg.contains("small-order"), + "rejection must name the small-order key, got: {msg}" + ), + other => panic!("expected Error::Did, got {other:?}"), + } + } + + /// base58 decoding is quadratic in its input and `signer` is attacker + /// controlled, so an oversized method-id must be refused BEFORE the decode + /// or a hostile attestation buys a large decode for a short request. The + /// assertion is on the length error specifically: a multibase error here + /// would mean the cap ran too late to matter. + #[test] + fn verifying_key_from_did_key_refuses_an_oversized_method_id_before_decoding() { + let oversized = format!("did:key:z{}", "1".repeat(65_536)); + let err = verifying_key_from_did_key(&oversized) + .expect_err("an oversized method-id must be refused"); + match err { + Error::Did(msg) => assert!( + msg.contains("too long"), + "must fail on the length cap, not after decoding: {msg}" + ), + other => panic!("expected Error::Did, got {other:?}"), + } + } + + /// Strictness regression guard. The parser now refuses a small-order key, + /// so `verify_signature` can no longer carry a weak signer down to the + /// signature check; this drives that check directly to prove it is still + /// STRICT. Downgrading `verify_strict` to `verify` makes this go red, + /// which nothing else in the suite would catch. + #[test] + fn verify_sig_with_key_is_strict_about_a_weak_key() { + let mut weak_key_bytes = [0u8; 32]; + weak_key_bytes[0] = 1; // compressed identity point + let weak_vk = VerifyingKey::from_bytes(&weak_key_bytes).unwrap(); + assert!(weak_vk.is_weak()); + + let cert_hash = sample_cert_hash(); + let mut att = dummy_attestation(&SigningKey::generate(&mut OsRng), cert_hash); + // R = identity, S = 0: satisfies the non-strict verification equation + // for any message under a small-order key. + let mut forged = [0u8; 64]; + forged[0] = 1; + att.sig = B64U.encode(forged); + + let bytes = canonical_signing_bytes(&att.type_, &att.payload, &att.cert_hash).expect("jcs"); + let err = att + .verify_sig_with_key(&weak_vk, &bytes) + .expect_err("strict verification must reject a small-order key"); + assert!( + matches!(err, Error::Signature(_)), + "expected a signature error, got {err:?}" + ); + } + + /// Accept control for the seam: a genuine signature still verifies through + /// the same helper, so the guard above is not simply rejecting everything. + #[test] + fn verify_sig_with_key_accepts_a_genuine_signature() { + let cert_hash = sample_cert_hash(); + let sk = SigningKey::generate(&mut OsRng); + let att = dummy_attestation(&sk, cert_hash); + let bytes = canonical_signing_bytes(&att.type_, &att.payload, &att.cert_hash).expect("jcs"); + att.verify_sig_with_key(&sk.verifying_key(), &bytes) + .expect("a real signature must verify"); + } + + /// Control for the guard above: a real did:key must still resolve. + #[test] + fn verifying_key_from_did_key_still_accepts_a_real_key() { + let sk = SigningKey::from_bytes(&[3u8; 32]); + let vk = sk.verifying_key(); + let mut buf = Vec::with_capacity(ED25519_MULTICODEC.len() + 32); + buf.extend_from_slice(&ED25519_MULTICODEC); + buf.extend_from_slice(&vk.to_bytes()); + let did = format!( + "did:key:{}", + multibase::encode(multibase::Base::Base58Btc, &buf) + ); + + let got = verifying_key_from_did_key(&did).expect("a real did:key must resolve"); + assert_eq!(got.to_bytes(), vk.to_bytes()); + } + #[test] fn verify_rejects_weak_key_signature() { let mut weak_key_bytes = [0u8; 32]; @@ -572,7 +716,13 @@ mod tests { let forged_sig = B64U.encode(forged); // Build an attestation with the weak key as signer and the forged - // signature. The weak-key check happens at verify time. + // signature. Rejection now happens at DID RESOLUTION rather than at + // verify time: `verifying_key_from_did_key` refuses a small-order key + // before `verify_strict` is reached, so the observed error is + // `Error::Did`, not `Error::Signature`. `verify_strict` remains the + // second layer for malleability cases that do not involve a weak + // public key (a small-order R under an honest key), which this fixture + // cannot construct and therefore no longer covers. let mut att = dummy_attestation(&SigningKey::generate(&mut OsRng), cert_hash); let mut buf = Vec::with_capacity(ED25519_MULTICODEC.len() + 32); buf.extend_from_slice(&ED25519_MULTICODEC); @@ -584,9 +734,12 @@ mod tests { att.sig = forged_sig; let err = att.verify_signature(cert_hash).unwrap_err(); - assert!( - matches!(err, Error::Signature(_)), - "signature under a weak (small-order) public key must be rejected" - ); + match err { + Error::Did(msg) => assert!( + msg.contains("small-order"), + "weak signer must be refused as a small-order key, got: {msg}" + ), + other => panic!("expected Error::Did for a small-order signer, got {other:?}"), + } } } diff --git a/crates/gitlawb-attest/src/verifier.rs b/crates/gitlawb-attest/src/verifier.rs index c0790c59..f6a2601d 100644 --- a/crates/gitlawb-attest/src/verifier.rs +++ b/crates/gitlawb-attest/src/verifier.rs @@ -121,7 +121,12 @@ impl Registry { attestation: &Attestation, expected_cert_hash: [u8; 32], ) -> Result { - attestation.verify_signature(expected_cert_hash)?; + // The key that actually verified is the trust anchor for the signer a + // consumer sees. `verify_signature` recovers it from the artifact's + // `signer` field and rejects a field that does not match the signing + // key, so this is the canonical DID of the key that signed — never a + // raw artifact string that could disagree with it. + let verified_key = attestation.verify_signature(expected_cert_hash)?; let fully = match self.by_type.get(attestation.type_.as_str()) { Some(v) => { @@ -138,7 +143,7 @@ impl Registry { Ok(VerifiedAttestation { type_: attestation.type_.clone(), - signer: attestation.signer.clone(), + signer: crate::attestation::did_key_from_verifying_key(&verified_key), cert_hash: attestation.cert_hash.clone(), fully_verified: fully, }) @@ -171,7 +176,7 @@ impl Registry { #[cfg(test)] mod tests { use super::*; - use crate::attestation::{Attestation, AttestationPayload}; + use crate::attestation::{did_key_from_verifying_key, Attestation, AttestationPayload}; use ed25519_dalek::SigningKey; use rand::rngs::OsRng; use serde::{Deserialize, Serialize}; @@ -374,4 +379,52 @@ mod tests { let err = reg.verify(&att, cert_hash).unwrap_err(); assert!(matches!(err, Error::Payload(_))); } + + /// The signer a consumer sees must be the canonical DID of the key that + /// actually verified, never a raw artifact string that could disagree. + /// `verify_signature` derives the key from the artifact's `signer` field + /// and the parser is canonical (base58btc, exact length, ed25519 + /// multicodec), so a forged field fails verification outright: the + /// must-not here is that no `VerifiedAttestation` ever carries a signer + /// that did not verify. + #[test] + fn verified_signer_is_anchored_to_the_key_that_verified() { + let signer = fresh(); + let other = fresh(); + assert_ne!( + signer.verifying_key().to_bytes(), + other.verifying_key().to_bytes(), + "fixture precondition: distinct signing keys" + ); + + let cert_hash = sample_hash(); + let reg = Registry::new(); + + // Control: an honest artifact reports the signer's canonical DID, and + // it agrees with the artifact field. + let att = signed_demo(&signer, cert_hash, "ok"); + let v = reg.verify(&att, cert_hash).unwrap(); + let expected = did_key_from_verifying_key(&signer.verifying_key()); + assert_eq!(v.signer, expected, "signer must be the verified key's DID"); + assert_eq!(v.signer, att.signer, "honest artifact: field and key agree"); + + // Must-not, both directions: a signer field naming a DIFFERENT key + // than the one that signed must fail verification, so the artifact + // string can never redirect the reported signer. + let mut forged = signed_demo(&signer, cert_hash, "ok"); + forged.signer = did_key_from_verifying_key(&other.verifying_key()); + let err = reg.verify(&forged, cert_hash).unwrap_err(); + assert!( + matches!(err, Error::Signature(_)), + "a signer field that does not match the signing key must fail, got {err:?}" + ); + + let mut forged_rev = signed_demo(&other, cert_hash, "ok"); + forged_rev.signer = did_key_from_verifying_key(&signer.verifying_key()); + let err_rev = reg.verify(&forged_rev, cert_hash).unwrap_err(); + assert!( + matches!(err_rev, Error::Signature(_)), + "mismatched signer field must fail verification, got {err_rev:?}" + ); + } } diff --git a/crates/gitlawb-core/src/did.rs b/crates/gitlawb-core/src/did.rs index e3775845..29bbdb04 100644 --- a/crates/gitlawb-core/src/did.rs +++ b/crates/gitlawb-core/src/did.rs @@ -71,7 +71,8 @@ impl Did { /// Resolve the Ed25519 verifying key from a `did:key`. /// - /// Returns `Err` if the DID is not a `did:key` or the key bytes are invalid. + /// Returns `Err` if the DID is not a `did:key`, the key bytes are malformed, + /// or the key is a small-order (weak) point. pub fn to_verifying_key(&self) -> Result { if !self.is_did_key() { return Err(Error::InvalidDid(format!( @@ -106,7 +107,25 @@ impl Did { .try_into() .map_err(|_| Error::InvalidDid("ed25519 key must be 32 bytes".to_string()))?; - VerifyingKey::from_bytes(&key_bytes).map_err(|e| Error::InvalidDid(e.to_string())) + let key = + VerifyingKey::from_bytes(&key_bytes).map_err(|e| Error::InvalidDid(e.to_string()))?; + + // `from_bytes` only decompresses, so it accepts a small-order point. + // Such a key satisfies the verification equation for any message, and + // every X25519 shared secret derived from it is the all-zero key: the + // order-1 and order-2 points convert to the all-zero Montgomery u + // directly, and the order-4 and order-8 points are annihilated by the + // scalar clamping instead. `is_weak` covers the whole torsion set. + // Mixed-order points are correctly NOT rejected, since clamping clears + // the cofactor component. Rejecting + // at resolution is what makes this the choke point: every consumer that + // resolves a DID through here inherits the rejection, and a recipient + // that cannot resolve already fails closed downstream. + if key.is_weak() { + return Err(Error::InvalidDid("small-order ed25519 key".to_string())); + } + + Ok(key) } /// Return the full DID string as a `&str`. @@ -369,4 +388,36 @@ mod tests { ); did.to_verifying_key().expect("a real did:key must resolve"); } + + /// A `did:key` can encode a small-order (weak) Ed25519 point. Such a key + /// satisfies the verification equation for any message and, converted to + /// Montgomery form for X25519, yields the all-zero shared secret, so an + /// envelope sealed to it is decryptable by anyone. Resolution is the choke + /// point every `Did`-based consumer shares, so it fails closed here. + #[test] + fn to_verifying_key_rejects_a_small_order_key() { + // The compressed identity point, the canonical small-order key. + let mut weak = [0u8; 32]; + weak[0] = 1; + let mut prefixed = Vec::with_capacity(ED25519_MULTICODEC.len() + 32); + prefixed.extend_from_slice(ED25519_MULTICODEC); + prefixed.extend_from_slice(&weak); + let did: Did = format!( + "did:key:{}", + multibase::encode(multibase::Base::Base58Btc, &prefixed) + ) + .parse() + .expect("a small-order did:key is still well-formed"); + + let err = did + .to_verifying_key() + .expect_err("a small-order did:key must not resolve"); + match err { + Error::InvalidDid(msg) => assert!( + msg.contains("small-order"), + "rejection must name the small-order key, got: {msg}" + ), + other => panic!("expected Error::InvalidDid, got {other:?}"), + } + } } diff --git a/crates/gitlawb-core/src/encrypt.rs b/crates/gitlawb-core/src/encrypt.rs index d8659be7..6f930b4f 100644 --- a/crates/gitlawb-core/src/encrypt.rs +++ b/crates/gitlawb-core/src/encrypt.rs @@ -11,6 +11,20 @@ use zeroize::Zeroizing; /// X25519 public key (Montgomery u) for an Ed25519 verifying key. fn x25519_public(vk: &VerifyingKey) -> Result<[u8; 32]> { use curve25519_dalek::edwards::CompressedEdwardsY; + + // Every X25519 shared secret derived from a small-order point is the + // all-zero key: order-1 and order-2 convert to the all-zero Montgomery u + // directly, order-4 and order-8 are annihilated by the scalar clamping in + // x25519_secret_from_seed. Either way the per-recipient wrap could be + // rebuilt by anyone. Resolution already refuses such a key (see + // Did::to_verifying_key); this guard covers the SEAL side on its own terms + // for a caller that obtained the key some other way. The open side + // (open_blob's attacker-supplied `eph`) is covered by + // is_low_order_montgomery. + if vk.is_weak() { + return Err(anyhow::anyhow!("verifying key is a small-order point")); + } + let edwards = CompressedEdwardsY::from_slice(vk.as_bytes()) .ok() .and_then(|c| c.decompress()) @@ -18,6 +32,26 @@ fn x25519_public(vk: &VerifyingKey) -> Result<[u8; 32]> { Ok(edwards.to_montgomery().to_bytes()) } +/// True when the X25519 exchange between an attacker-supplied `u` and this +/// reader's scalar yields the all-zero shared secret. +/// +/// Asked of the RESULT, not of the encoding, and that distinction is the whole +/// guard. The previous version decompressed `u` to an Edwards point and asked +/// `is_small_order`, which reports "safe" for any low-order encoding that does +/// not decompress at all. Measured across the seven standard low-order +/// encodings, six were caught and `u = p - 1` was not: it fails to decompress, +/// so the check returned false, and the exchange is still the all-zero secret. +/// An entry built on it unwraps for every reader. +/// +/// Enumerating encodings cannot be exhaustive, because the attack depends on the +/// shared secret rather than on the bytes that produced it, and twist and +/// non-canonical inputs keep arriving. Asking the result needs no list. +fn yields_all_zero_shared_secret(u: &[u8; 32], scalar: &[u8; 32]) -> bool { + use curve25519_dalek::montgomery::MontgomeryPoint; + + MontgomeryPoint(*u).mul_clamped(*scalar).to_bytes() == [0u8; 32] +} + /// X25519 secret scalar for an Ed25519 seed (SHA-512 of seed, lower 32, clamped). /// Returns the scalar wrapped in `Zeroizing`, and scrubs the intermediate /// SHA-512 digest, so no copy of this secret material lingers in freed memory. @@ -128,7 +162,10 @@ pub fn open_blob(envelope: &[u8], keypair: &Keypair) -> Result> { .context("decode header")?; let body = &envelope[p + hlen..]; - let my_x = XSecret::from(*x25519_secret_from_seed(&keypair.to_seed())); + // The raw scalar is kept alongside the box secret so the exchange can be + // tested BEFORE a box is built from an attacker-supplied ephemeral. + let my_x_scalar = x25519_secret_from_seed(&keypair.to_seed()); + let my_x = XSecret::from(*my_x_scalar); // Identities are blinded: no entry says which recipient it belongs to, so // try each one. The ChaChaBox AEAD tag authenticates, so exactly the @@ -140,7 +177,12 @@ pub fn open_blob(envelope: &[u8], keypair: &Keypair) -> Result> { .ok() .and_then(|b| <[u8; 32]>::try_from(b.as_slice()).ok()) { - Some(b) => XPublic::from(b), + // An ephemeral whose exchange with this reader is the all-zero + // shared secret would unwrap for anyone, so the entry is skipped. + // Tested on the exchange itself: an encoding-shaped check misses + // every low-order input that does not decompress to Edwards. + Some(b) if !yields_all_zero_shared_secret(&b, &my_x_scalar) => XPublic::from(b), + Some(_) => continue, None => continue, }; // from_slice panics on a wrong length, and the envelope is attacker @@ -293,4 +335,362 @@ mod tests { header["nonce"] = bad_nonce; assert!(open_blob(&reframe(&header), &reader).is_err()); } + + /// A low-order ephemeral public forces the all-zero shared secret, so an + /// entry built on it would unwrap for anyone. The header's `eph` is + /// attacker-controlled, so the open side must skip it. u = 0 and u = 1 are + /// the Montgomery u-coordinates that decompress to a small-order Edwards + /// point (u = 0 is the one that produces the all-zero shared secret); the + /// other small-order u values fail to decompress and are already skipped + /// by the decode path. + #[test] + fn all_zero_shared_secret_is_detected_for_every_standard_low_order_encoding() { + // The seven standard X25519 low-order encodings, driven as a set rather + // than as the two that happen to decompress. The encoding-shaped check + // this replaced reported `p - 1` as safe, and its exchange is all-zero. + let mut one = [0u8; 32]; + one[0] = 1; + let hex = |h: &str| -> [u8; 32] { + let mut out = [0u8; 32]; + for i in 0..32 { + out[i] = u8::from_str_radix(&h[i * 2..i * 2 + 2], 16).unwrap(); + } + out + }; + let vectors = [ + ("u=0", [0u8; 32]), + ("u=1", one), + ( + "order8-a", + hex("e0eb7a7c3b41b8ae1656e3faf19fc46ada098deb9c32b1fd866205165f49b800"), + ), + ( + "order8-b", + hex("5f9c95bca3508c24b1d0b1559c83ef5b04445cc4581c8e86d8224eddd09f1157"), + ), + ( + "p-1", + hex("ecffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f"), + ), + ( + "p", + hex("edffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f"), + ), + ( + "p+1", + hex("eeffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f"), + ), + ]; + let kp = Keypair::generate(); + let scalar = x25519_secret_from_seed(&kp.to_seed()); + for (name, u) in vectors { + assert!( + yields_all_zero_shared_secret(&u, &scalar), + "{name} is a low-order encoding and must be detected" + ); + } + + // And the other direction, or a guard that returned true always would + // pass everything above while making the envelope unopenable. + let real_u = x25519_public(&kp.verifying_key()).unwrap(); + assert!( + !yields_all_zero_shared_secret(&real_u, &scalar), + "a real key must not be treated as low-order" + ); + } + + /// The open side must skip an entry whose header-supplied `eph` is a + /// low-order point: such an entry forces the all-zero shared secret and + /// would unwrap for any reader. A poisoned entry among honest ones must + /// not DoS the envelope — the honest entry still opens. + #[test] + fn open_blob_skips_a_low_order_ephemeral_and_still_opens_honest_entry() { + use curve25519_dalek::montgomery::MontgomeryPoint; + let reader = Keypair::generate(); + let env = seal_blob(b"private blob contents", &[reader.verifying_key()]).unwrap(); + + // Split the envelope framing into header JSON and body. + let mut p = MAGIC.len() + 1; + let hlen = u32::from_le_bytes(env[p..p + 4].try_into().unwrap()) as usize; + p += 4; + let header_bytes = &env[p..p + hlen]; + let body = &env[p + hlen..]; + + // A low-order Montgomery u (u = 0) encoded as the entry's eph. + let low_order_u = MontgomeryPoint([0u8; 32]).to_bytes(); + let poisoned = serde_json::json!({ + "eph": B64.encode(low_order_u), + "nonce": B64.encode([0u8; 24]), + "wrap": B64.encode([0u8; 32]), + }); + + let reframe = |header: &serde_json::Value| -> Vec { + let hj = serde_json::to_vec(header).unwrap(); + let mut out = Vec::new(); + out.extend_from_slice(MAGIC); + out.push(VERSION); + out.extend_from_slice(&(hj.len() as u32).to_le_bytes()); + out.extend_from_slice(&hj); + out.extend_from_slice(body); + out + }; + + let mut header: serde_json::Value = serde_json::from_slice(header_bytes).unwrap(); + // Prepend the poisoned entry. The reader's honest entry must still + // unwrap, so the envelope opens despite the low-order entry. + let recipients = header["recipients"].as_array_mut().unwrap(); + recipients.insert(0, poisoned); + assert_eq!( + open_blob(&reframe(&header), &reader).unwrap(), + b"private blob contents", + "the honest entry must still open despite a low-order eph entry" + ); + } + + /// Must-not for the open-side guard: an envelope whose ONLY entry is built + /// on a low-order ephemeral must not open for any reader. Without the + /// guard the all-zero shared secret unwraps for everyone, so the attacker + /// controls the plaintext and the envelope's confidentiality is void. + #[test] + fn open_blob_rejects_a_poisoned_only_envelope() { + use chacha20poly1305::{aead::Aead, KeyInit, XChaCha20Poly1305, XNonce}; + use crypto_box::aead::AeadCore; + use curve25519_dalek::montgomery::MontgomeryPoint; + + // Build a poisoned envelope from scratch: one recipient entry whose + // eph is the low-order u = 0. The wrap is the encryption of a content + // key under the all-zero shared secret, so without the guard any + // reader's ChaChaBox against u = 0 decrypts it. + let content_key = [0x42u8; 32]; + let zero_box = ChaChaBox::new(&XPublic::from([0u8; 32]), &XSecret::from([7u8; 32])); + let nonce = ChaChaBox::generate_nonce(&mut OsRng); + let wrap = zero_box.encrypt(&nonce, &content_key[..]).unwrap(); + + let body_nonce = [0x24u8; 24]; + let body_cipher = XChaCha20Poly1305::new_from_slice(&content_key).unwrap(); + let body = body_cipher + .encrypt( + XNonce::from_slice(&body_nonce), + b"attacker-chosen plaintext".as_slice(), + ) + .unwrap(); + + let header = serde_json::json!({ + "alg": "xchacha20poly1305", + "nonce": B64.encode(body_nonce), + "recipients": [{ + "eph": B64.encode(MontgomeryPoint([0u8; 32]).to_bytes()), + "nonce": B64.encode(nonce), + "wrap": B64.encode(wrap), + }], + }); + let header_json = serde_json::to_vec(&header).unwrap(); + let mut env = Vec::new(); + env.extend_from_slice(MAGIC); + env.push(VERSION); + env.extend_from_slice(&(header_json.len() as u32).to_le_bytes()); + env.extend_from_slice(&header_json); + env.extend_from_slice(&body); + + let reader = Keypair::generate(); + let err = open_blob(&env, &reader).unwrap_err(); + assert!( + err.to_string().contains("not a recipient"), + "a poisoned-only envelope must not open for anyone, got: {err}" + ); + } + + /// The same must-not, driven with the low-order encoding that does NOT map + /// to an Edwards point. + /// + /// This is the case an encoding-shaped guard cannot see. `u = p - 1` is a + /// legal X25519 input whose exchange is the all-zero secret for any scalar, + /// and `MontgomeryPoint::to_edwards` returns None for it, so a check written + /// as "decompress, then ask is_small_order" reports it as safe. Measured + /// across the seven standard low-order encodings: six are caught that way + /// and this one is not. + /// + /// Found by a cross-family review of the head that first added the guard, + /// and the comment beside that guard asserted this case was already handled + /// by a decode path that does not exist. That is why the fix rejects on the + /// RESULT of the exchange rather than on a list of encodings: the result is + /// what the attack actually depends on, and it needs no enumeration to be + /// exhaustive. + #[test] + fn open_blob_rejects_a_non_edwards_low_order_ephemeral() { + // No blanket `use ...::Aead` here. Two aead versions are reachable, and a + // blanket import silently resolved the wrap through the one open_blob does + // NOT decrypt with, producing a ciphertext that could never open. The test + // then passed with the guard, without it, and with the original pre-fix + // guard alike, proving nothing. Every call below names its trait. + use chacha20poly1305::{KeyInit, XChaCha20Poly1305, XNonce}; + use crypto_box::aead::AeadCore; + + // u = p - 1 = 2^255 - 20, little endian. + let mut eph = [0xffu8; 32]; + eph[0] = 0xec; + eph[31] = 0x7f; + + let content_key = [0x42u8; 32]; + let zero_box = ChaChaBox::new(&XPublic::from([0u8; 32]), &XSecret::from([7u8; 32])); + let nonce = ChaChaBox::generate_nonce(&mut OsRng); + let wrap = crypto_box::aead::Aead::encrypt(&zero_box, &nonce, &content_key[..]).unwrap(); + + let body_nonce = [0x24u8; 24]; + let body_cipher = XChaCha20Poly1305::new_from_slice(&content_key).unwrap(); + let body = chacha20poly1305::aead::Aead::encrypt( + &body_cipher, + XNonce::from_slice(&body_nonce), + b"attacker-chosen plaintext".as_slice(), + ) + .unwrap(); + + let header = serde_json::json!({ + "alg": "xchacha20poly1305", + "nonce": B64.encode(body_nonce), + "recipients": [{ + "eph": B64.encode(eph), + "nonce": B64.encode(nonce), + "wrap": B64.encode(wrap), + }], + }); + let header_json = serde_json::to_vec(&header).unwrap(); + let mut env = Vec::new(); + env.extend_from_slice(MAGIC); + env.push(VERSION); + env.extend_from_slice(&(header_json.len() as u32).to_le_bytes()); + env.extend_from_slice(&header_json); + env.extend_from_slice(&body); + + let reader = Keypair::generate(); + let err = open_blob(&env, &reader).unwrap_err(); + assert!( + err.to_string().contains("not a recipient"), + "an envelope whose only entry uses a non-Edwards low-order eph must not \ + open for anyone, got: {err}" + ); + } + + /// The compressed identity point: a well-formed Ed25519 encoding that is + /// small-order, so every shared secret derived from it is all-zero. + fn weak_verifying_key() -> VerifyingKey { + let mut weak = [0u8; 32]; + weak[0] = 1; + let vk = VerifyingKey::from_bytes(&weak).expect("identity point decompresses"); + assert!(vk.is_weak(), "fixture precondition: key is small-order"); + vk + } + + /// A small-order recipient key converts to Montgomery u = 0, and X25519 + /// against u = 0 is the all-zero shared secret for ANY scalar, so the + /// wrapping box is reconstructable with no secret at all. Reject at the + /// primitive so the seal side is safe regardless of how the caller + /// obtained the key. + #[test] + fn x25519_public_rejects_a_small_order_key() { + let weak_vk = weak_verifying_key(); + + assert!( + x25519_public(&weak_vk).is_err(), + "a small-order key must not yield an x25519 public key" + ); + } + + /// Control: a legitimate key still converts, and to a non-zero u. + #[test] + fn x25519_public_still_accepts_a_real_key() { + let kp = Keypair::generate(); + let u = x25519_public(&kp.verifying_key()).expect("a real key must convert"); + assert_ne!(u, [0u8; 32], "a real key must not map to the zero point"); + } + + /// The guard has to propagate: no envelope may be produced for a weak + /// recipient even when the DID choke point is bypassed entirely. + #[test] + fn seal_blob_refuses_a_small_order_recipient() { + let weak_vk = weak_verifying_key(); + + assert!( + seal_blob(b"withheld", &[weak_vk]).is_err(), + "sealing to a weak recipient must fail" + ); + + // And a mixed set must fail too: one weak recipient exposes the shared + // content key, so a partial envelope is not an acceptable outcome. + let honest = Keypair::generate(); + assert!( + seal_blob(b"withheld", &[honest.verifying_key(), weak_vk]).is_err(), + "a mixed honest+weak recipient set must fail closed, not seal partially" + ); + } + + /// Control for the two above: the ordinary seal/open round trip is intact. + #[test] + fn legit_seal_open_round_trip_still_works() { + let reader = Keypair::generate(); + let env = seal_blob(b"withheld blob", &[reader.verifying_key()]).expect("seal"); + assert_eq!(open_blob(&env, &reader).expect("open"), b"withheld blob"); + } + + /// The defect this fix exists to close, kept as an executable regression + /// rather than an assertion about absence. It runs the real attack: craft a + /// small-order did:key, get it into a recipient set alongside an honest + /// reader, then rebuild the wrapping box from the all-zero shared secret + /// that a small-order recipient forces. If any guard regresses, this does + /// not merely fail, it fails printing the plaintext it recovered. + #[test] + fn attacker_cannot_recover_plaintext_via_a_weak_recipient() { + use crate::did::Did; + use std::str::FromStr; + + let weak_vk = weak_verifying_key(); + + // Layer 1: the attacker's did:key string must not resolve at all. + let weak_did = Did::from_verifying_key(&weak_vk).to_string(); + assert!( + Did::from_str(&weak_did) + .expect("still a well-formed did:key") + .to_verifying_key() + .is_err(), + "a small-order did:key must not resolve" + ); + + // Layer 2: even handed the key directly, sealing must refuse. One weak + // recipient would expose the single shared content key, so the honest + // reader's blob would be readable by anyone. + let honest = Keypair::generate(); + let secret_plaintext = b"WITHHELD BLOB CONTENTS"; + let envelope = match seal_blob(secret_plaintext, &[honest.verifying_key(), weak_vk]) { + Err(_) => return, // no envelope exists, nothing to attack + Ok(env) => env, + }; + + // Only reachable if a guard regressed. Run the attack and report what + // it got, so the failure names the actual exposure. + let mut p = MAGIC.len() + 1; + let hlen = u32::from_le_bytes(envelope[p..p + 4].try_into().unwrap()) as usize; + p += 4; + let header: serde_json::Value = serde_json::from_slice(&envelope[p..p + hlen]).unwrap(); + let body = &envelope[p + hlen..]; + let body_nonce = B64.decode(header["nonce"].as_str().unwrap()).unwrap(); + + // X25519 against u = 0 is the all-zero shared secret for any scalar, so + // the sealer's box is reconstructable with a key the attacker picks. + let zero_box = ChaChaBox::new(&XPublic::from([0u8; 32]), &XSecret::from([7u8; 32])); + for r in header["recipients"].as_array().unwrap() { + let n = B64.decode(r["nonce"].as_str().unwrap()).unwrap(); + let w = B64.decode(r["wrap"].as_str().unwrap()).unwrap(); + if let Ok(content_key) = zero_box.decrypt(n.as_slice().into(), w.as_slice()) { + let cipher = XChaCha20Poly1305::new_from_slice(&content_key).unwrap(); + let recovered = cipher + .decrypt(XNonce::from_slice(&body_nonce), body) + .expect("body decrypts once the content key is out"); + panic!( + "REGRESSION: attacker recovered plaintext with no private key: {:?}", + String::from_utf8_lossy(&recovered) + ); + } + } + panic!("an envelope was sealed to a small-order recipient; the seal guard regressed"); + } } diff --git a/crates/gitlawb-node/src/auth/mod.rs b/crates/gitlawb-node/src/auth/mod.rs index eee8fd8b..3beb7f14 100644 --- a/crates/gitlawb-node/src/auth/mod.rs +++ b/crates/gitlawb-node/src/auth/mod.rs @@ -34,6 +34,27 @@ use gitlawb_core::http_sig::{ }; use gitlawb_core::identity::verify; +/// The hint for an unresolvable DID, chosen by method. The hint is only true +/// for a method we have no resolver for: a did:key that parsed and then failed +/// on its key material (a small-order key, wrong multicodec, wrong length) +/// would be sent looking for the wrong problem by the alpha-support wording. +fn unresolvable_did_hint(key_id: &Did) -> &'static str { + if key_id.is_did_key() { + "the DID is a did:key whose key material did not resolve" + } else { + "only did:key is supported in alpha" + } +} + +/// Bound the echoed keyid in the unresolvable-DID message. `key_id` is +/// unauthenticated request input, so reflecting it back at whatever length the +/// caller chose would amplify attacker input into the response body. The bound +/// is slack (a did:key method-id is a fixed 48 characters) and only defends +/// the echo, not the resolution. +fn bounded_did_echo(key_id: &str) -> String { + key_id.chars().take(96).collect() +} + /// Axum middleware that enforces HTTP Signature authentication (RFC 9421). /// /// Every write request must carry: @@ -145,11 +166,14 @@ pub async fn require_signature(request: Request, next: Next) -> Response { StatusCode::BAD_REQUEST, Json(json!({ "error": "unresolvable_did", - "message": format!("cannot resolve DID '{}': {e}", sig.key_id), - "hint": "only did:key is supported in alpha", + "message": format!( + "cannot resolve DID '{}': {e}", + bounded_did_echo(&sig.key_id.to_string()) + ), + "hint": unresolvable_did_hint(&sig.key_id), })), ) - .into_response() + .into_response(); } }; @@ -423,6 +447,58 @@ mod tests { assert!(validate_ucan_chain(&token, &node_did, &agent_did).is_ok()); } + /// The two-branch hint selection is the point of the unresolvable-DID + /// change: a did:key that failed on its key material must be told the key + /// material did not resolve, and any other method must be told only did:key + /// is supported. A regression that drops the is_did_key() check would send + /// a did:key caller after the wrong problem, so both branches are pinned. + #[test] + fn unresolvable_did_hint_distinguishes_key_material_from_method() { + // A real did:key whose key material fails to resolve (small-order): + // key-material hint. + let mut weak = [0u8; 32]; + weak[0] = 1; // compressed identity point + let weak_vk = ed25519_dalek::VerifyingKey::from_bytes(&weak).unwrap(); + let weak_did = Did::from_verifying_key(&weak_vk); + assert!( + weak_did.to_verifying_key().is_err(), + "fixture precondition: small-order did:key must not resolve" + ); + assert_eq!( + unresolvable_did_hint(&weak_did), + "the DID is a did:key whose key material did not resolve" + ); + + // A method with no resolver: alpha-support hint. + let web = Did::web("example.com"); + assert!( + web.to_verifying_key().is_err(), + "fixture precondition: did:web must not resolve locally" + ); + assert_eq!( + unresolvable_did_hint(&web), + "only did:key is supported in alpha" + ); + } + + /// The unresolvable-DID message echoes the keyid, which is unauthenticated + /// request input. The echoed value must be bounded so a hostile caller + /// cannot amplify arbitrary-length input into the response body. + #[test] + fn unresolvable_did_message_bounds_the_echoed_keyid() { + let long_keyid = format!("did:key:z{}", "A".repeat(2000)); + let shown = bounded_did_echo(&long_keyid); + assert!( + shown.len() <= 96, + "echoed keyid must be bounded, got {} chars", + shown.len() + ); + assert!( + !shown.contains(&"A".repeat(1000)), + "the full keyid must not survive into the message" + ); + } + #[test] fn validate_ucan_chain_wrong_issuer() { let node = Keypair::generate(); diff --git a/crates/gitlawb-node/src/encrypted_pin.rs b/crates/gitlawb-node/src/encrypted_pin.rs index 19b80651..088bd69e 100644 --- a/crates/gitlawb-node/src/encrypted_pin.rs +++ b/crates/gitlawb-node/src/encrypted_pin.rs @@ -69,6 +69,12 @@ fn resolve_all_recipients(dids: &BTreeSet) -> Result, enum SealPlan { /// An existing envelope already covers exactly this recipient set; nothing to do. SkipUnchanged, + /// The stored tag matches, but the stored reader set contains a DID that + /// must not resolve (e.g. a small-order did:key sealed before the choke + /// point landed). The old envelope is already public and cannot be + /// withdrawn, but it must be surfaced for operator action rather than + /// silently skipped as unchanged. + SkipUnresolvableStored(Vec), /// No recipient DID resolved to a key, so there is nothing to seal to. SkipNoRecipients, /// At least one recipient DID is unresolvable. Fail closed: never seal to a @@ -90,15 +96,30 @@ enum SealPlan { /// recover the blob; reader removal is not retroactive (the old envelope is /// already public). The comparison is on the opaque node-keyed tag, never the /// DID list. +/// +/// A matching stored tag is normally a skip, but the set is still resolved +/// before skipping: a reader that used to resolve and now must not (a +/// small-order did:key sealed before the choke point landed) is surfaced as +/// [`SealPlan::SkipUnresolvableStored`] instead of being silently treated as +/// unchanged. The old envelope is already public and cannot be withdrawn, but +/// the operator must be able to see that a persisted envelope was sealed to a +/// weak recipient. fn plan_seal(node_seed: &[u8; 32], dids: &BTreeSet, stored_tag: Option<&str>) -> SealPlan { let tag = recipients_tag(node_seed, dids); if stored_tag == Some(tag.as_str()) { - return SealPlan::SkipUnchanged; - } - match resolve_all_recipients(dids) { - Ok(keys) if keys.is_empty() => SealPlan::SkipNoRecipients, - Ok(keys) => SealPlan::Seal { keys, tag }, - Err(unresolved) => SealPlan::SkipUnresolvable(unresolved), + // The stored envelope already covers exactly this set, so there is + // nothing to re-seal. Resolve anyway so a now-unresolvable reader in an + // unchanged set is surfaced rather than hidden by the tag match. + match resolve_all_recipients(dids) { + Ok(_) => SealPlan::SkipUnchanged, + Err(unresolved) => SealPlan::SkipUnresolvableStored(unresolved), + } + } else { + match resolve_all_recipients(dids) { + Ok(keys) if keys.is_empty() => SealPlan::SkipNoRecipients, + Ok(keys) => SealPlan::Seal { keys, tag }, + Err(unresolved) => SealPlan::SkipUnresolvable(unresolved), + } } } @@ -131,6 +152,21 @@ pub async fn encrypt_and_pin( // covered (which would permanently lock out the dropped readers, #47). let (keys, tag) = match plan_seal(node_seed, dids, stored_tag.as_deref()) { SealPlan::SkipUnchanged => continue, + SealPlan::SkipUnresolvableStored(unresolved) => { + // A stored envelope already covers this exact reader set, and + // the set now contains a DID that must not resolve (a + // small-order did:key sealed before the choke point landed). + // The old envelope is already public and cannot be withdrawn + // or re-sealed; surface it so the operator can act. + let sample: Vec<&String> = unresolved.iter().take(3).collect(); + tracing::warn!( + oid = %oid, + unresolved_count = unresolved.len(), + unresolved_sample = ?sample, + "stored envelope was sealed to a recipient that must not resolve; operator action may be required" + ); + continue; + } SealPlan::SkipNoRecipients => { tracing::warn!(oid = %oid, "no resolvable recipient keys; skipping encrypted pin"); continue; @@ -206,6 +242,49 @@ mod tests { use super::*; use ed25519_dalek::SigningKey; + /// A did:key encoding a small-order point. Well-formed, indistinguishable + /// from a real one by eye, and refused at resolution since the choke-point + /// guard landed. + fn weak_did_key() -> String { + let mut weak = [0u8; 32]; + weak[0] = 1; // compressed identity point + let vk = ed25519_dalek::VerifyingKey::from_bytes(&weak).expect("decompresses"); + Did::from_verifying_key(&vk).to_string() + } + + /// The #47 fail-closed arm already refuses to seal to a partial recipient + /// set. Because a small-order did:key no longer RESOLVES, it lands in the + /// unresolved set and that existing arm catches it: nothing is sealed, and + /// no new code path was added to make that happen. + #[test] + fn weak_recipient_falls_into_the_existing_fail_closed_arm() { + let weak = weak_did_key(); + let mut dids = BTreeSet::new(); + dids.insert(did_key(9)); + dids.insert(weak.clone()); + + match plan_seal(&[0u8; 32], &dids, None) { + SealPlan::SkipUnresolvable(unresolved) => assert!( + unresolved.contains(&weak), + "the weak DID must be named in the unresolved set, got {unresolved:?}" + ), + other => panic!("expected SkipUnresolvable, got {other:?}"), + } + } + + /// Must-not-over-reject control: an all-legitimate recipient set still seals. + #[test] + fn legitimate_recipient_set_still_seals() { + let mut dids = BTreeSet::new(); + dids.insert(did_key(9)); + dids.insert(did_key(11)); + + match plan_seal(&[0u8; 32], &dids, None) { + SealPlan::Seal { keys, .. } => assert_eq!(keys.len(), 2, "both recipients resolve"), + other => panic!("expected Seal, got {other:?}"), + } + } + fn did_key(seed: u8) -> String { let vk = SigningKey::from_bytes(&[seed; 32]).verifying_key(); Did::from_verifying_key(&vk).to_string() @@ -348,6 +427,43 @@ mod tests { )); } + /// P1 remediation (review): an unchanged reader set is normally a skip, but + /// the skip must not hide a recipient that now fails to resolve. A blob + /// sealed to a small-order did:key before the choke-point guard landed keeps + /// its old CID and stays decryptable-without-a-private-key unless the + /// matching-tag path surfaces it. This pins that the stored-tag match still + /// resolves the set instead of short-circuiting. + #[test] + fn plan_seal_surfaces_unresolvable_recipient_in_unchanged_reader_set() { + let weak = weak_did_key(); + let mut dids = BTreeSet::new(); + dids.insert(did_key(9)); + dids.insert(weak.clone()); + let stored = recipients_tag(&SEED, &dids); + + match plan_seal(&SEED, &dids, Some(&stored)) { + SealPlan::SkipUnresolvableStored(unresolved) => assert!( + unresolved.contains(&weak), + "the weak DID must be named in the unresolved set, got {unresolved:?}" + ), + other => panic!("expected SkipUnresolvableStored, got {other:?}"), + } + } + + /// Degenerate control: an empty set with a matching stored tag stays a + /// silent skip. The state is unreachable in practice (a seal is never + /// recorded for an empty reader set), so this pins only that the + /// resolve-on-match path must not turn it into a re-seal or a warning. + #[test] + fn plan_seal_unchanged_empty_set_still_skips() { + let empty = BTreeSet::new(); + let empty_tag = recipients_tag(&SEED, &empty); + assert!(matches!( + plan_seal(&SEED, &empty, Some(&empty_tag)), + SealPlan::SkipUnchanged + )); + } + #[test] fn plan_seal_reseals_when_recipient_set_changed() { // A stored tag for a DIFFERENT set is a miss: a newly added reader must diff --git a/crates/gl/Cargo.toml b/crates/gl/Cargo.toml index 3d7ddb9f..9c6e4f5e 100644 --- a/crates/gl/Cargo.toml +++ b/crates/gl/Cargo.toml @@ -42,3 +42,4 @@ alloy = { version = "1", default-features = false, features = [ mockito = "1" tempfile = "3" tokio = { workspace = true } +ed25519-dalek = { workspace = true } diff --git a/crates/gl/src/mcp.rs b/crates/gl/src/mcp.rs index ae319c73..6ca3514e 100644 --- a/crates/gl/src/mcp.rs +++ b/crates/gl/src/mcp.rs @@ -775,13 +775,15 @@ async fn call_tool( let did: gitlawb_core::did::Did = did_str .parse() .map_err(|e: gitlawb_core::Error| anyhow::anyhow!("{e}"))?; - if let Ok(vk) = did.to_verifying_key() { - let doc = gitlawb_core::did::DidDocument::new(did, &vk); - Ok(serde_json::to_string_pretty(&doc)?) - } else { - Err(anyhow::anyhow!( - "cannot resolve '{did_str}' locally — only did:key is supported without a resolver" - )) + let is_did_key = did.is_did_key(); + match did.to_verifying_key() { + Ok(vk) => { + let doc = gitlawb_core::did::DidDocument::new(did, &vk); + Ok(serde_json::to_string_pretty(&doc)?) + } + Err(e) => Err(anyhow::anyhow!(did_resolve_failure_message( + did_str, is_did_key, &e + ))), } } @@ -1341,6 +1343,76 @@ fn write_message(writer: &mut impl Write, value: &Value) -> Result<()> { Ok(()) } +/// Message for a `did_resolve` failure. A `did:key` that fails to resolve is a +/// different situation from a DID method we have no resolver for, so saying +/// "only did:key is supported" about something that IS a did:key sends the +/// reader looking for the wrong problem. +fn did_resolve_failure_message( + did_str: &str, + is_did_key: bool, + err: &gitlawb_core::Error, +) -> String { + if is_did_key { + format!("cannot resolve '{did_str}': {err}") + } else { + format!("cannot resolve '{did_str}' locally — only did:key is supported without a resolver") + } +} + +#[cfg(test)] +mod did_resolve_message_tests { + use super::did_resolve_failure_message; + use gitlawb_core::did::Did; + use std::str::FromStr; + + /// A well-formed did:key encoding the compressed identity point, which is + /// small-order. Derived from the point bytes so the fixture states WHY it + /// is weak rather than pinning an opaque string (the literal also trips + /// secret scanners, which cannot tell a public DID from an API key). + fn weak_did_key() -> String { + let mut weak = [0u8; 32]; + weak[0] = 1; // compressed identity point + let vk = ed25519_dalek::VerifyingKey::from_bytes(&weak).expect("decompresses"); + Did::from_verifying_key(&vk).to_string() + } + + #[test] + fn test_did_key_failure_does_not_claim_did_key_is_unsupported() { + // Drive the real parse and the real resolution error rather than + // hardcoding the discriminator, so flipping it at the call site fails. + let did_str = &weak_did_key(); + let did = Did::from_str(did_str).expect("well-formed did:key"); + let err = did + .to_verifying_key() + .expect_err("a small-order did:key must not resolve"); + + let msg = did_resolve_failure_message(did_str, did.is_did_key(), &err); + assert!( + !msg.contains("only did:key is supported"), + "a did:key must not be told did:key is unsupported: {msg}" + ); + assert!( + msg.contains("small-order"), + "the real cause must survive into the message: {msg}" + ); + } + + #[test] + fn test_non_key_method_keeps_the_no_resolver_message() { + let did_str = "did:web:example.com"; + let did = Did::from_str(did_str).expect("did:web parses"); + let err = did + .to_verifying_key() + .expect_err("did:web has no local resolver"); + + let msg = did_resolve_failure_message(did_str, did.is_did_key(), &err); + assert!( + msg.contains("only did:key is supported"), + "an unresolvable method keeps its existing explanation: {msg}" + ); + } +} + #[cfg(test)] mod tests { use super::*;