From 0ed1ce3d4e585b289bd112a8be9619cefcdf5cb9 Mon Sep 17 00:00:00 2001 From: Trezy Date: Mon, 3 Aug 2026 09:46:13 -0500 Subject: [PATCH] fix: support unpadded base64 in bytes fields Signed-off-by: Trezy --- src/cid_verify.rs | 41 ++++++++++++++++++++++++++++++++++--- src/plugin/attestation.rs | 43 ++++++++++++++++++++++++++++++++------- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/src/cid_verify.rs b/src/cid_verify.rs index 7f8b89b..7a20a04 100644 --- a/src/cid_verify.rs +++ b/src/cid_verify.rs @@ -9,6 +9,26 @@ use std::str::FromStr; const DAG_CBOR_CODEC: u64 = 0x71; const SHA2_256_CODE: u64 = 0x12; +/// The codec for `$bytes` in the atproto data model โ€” the one place that +/// encoding is decided, so every producer and consumer of `$bytes` agrees. +/// +/// The data model specifies RFC-4648 ยง4 base64 in which `=` padding is +/// **optional**. Both forms name the same bytes, so both must decode: +/// jetstream emits `$bytes` unpadded, our own signer emits it padded, and a +/// signature we wrote ourselves comes back off the firehose two characters +/// shorter. A padding-strict engine reads that as corruption and reports a +/// record we signed as unverifiable โ€” which downstream becomes an accusation +/// of forgery aimed at the record's author. +/// +/// Encoding is unchanged from `general_purpose::STANDARD` (padding on, +/// standard alphabet); only the decoder is made indifferent. +pub const BYTES_B64: base64::engine::general_purpose::GeneralPurpose = + base64::engine::general_purpose::GeneralPurpose::new( + &base64::alphabet::STANDARD, + base64::engine::general_purpose::GeneralPurposeConfig::new() + .with_decode_padding_mode(base64::engine::DecodePaddingMode::Indifferent), + ); + /// Outcome of checking a claimed CID against a record's content. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CidCheck { @@ -52,9 +72,7 @@ fn atproto_json_to_ipld(value: &Value) -> Option { return Some(Ipld::Link(Cid::from_str(link).ok()?)); } if let Some(Value::String(b64)) = obj.get("$bytes") { - let bytes = - base64::Engine::decode(&base64::engine::general_purpose::STANDARD, b64) - .ok()?; + let bytes = base64::Engine::decode(&BYTES_B64, b64).ok()?; return Some(Ipld::Bytes(bytes)); } } @@ -143,6 +161,23 @@ mod tests { ); } + /// `=` padding on `$bytes` is optional in the data model, so the same + /// bytes may arrive either way and must yield the same CID. If the + /// unpadded form fails to encode, `verify_record_cid` degrades to + /// `Skipped` and the record is indexed with its CID unchecked. + #[test] + fn padded_and_unpadded_bytes_produce_the_same_cid() { + let padded = json!({ "sig": { "$bytes": "3q2+7w==" } }); + let unpadded = json!({ "sig": { "$bytes": "3q2+7w" } }); + + let padded_cid = compute_record_cid(&padded).expect("padded is encodable"); + assert_eq!( + compute_record_cid(&unpadded), + Some(padded_cid), + "unpadded $bytes must encode to the same CID" + ); + } + #[test] fn verify_matches_recomputed_cid() { let value = json!({ diff --git a/src/plugin/attestation.rs b/src/plugin/attestation.rs index c532e80..c21dbf6 100644 --- a/src/plugin/attestation.rs +++ b/src/plugin/attestation.rs @@ -108,7 +108,7 @@ impl AttestationSigner { "$type": &self.sig_type, "key": &self.key_id, "signature": { - "$bytes": base64::Engine::encode(&base64::engine::general_purpose::STANDARD, &signature) + "$bytes": base64::Engine::encode(&crate::cid_verify::BYTES_B64, &signature) } }); @@ -149,7 +149,7 @@ impl AttestationSigner { "$type": &self.sig_type, "key": &self.key_id, "signature": { - "$bytes": base64::Engine::encode(&base64::engine::general_purpose::STANDARD, &signature) + "$bytes": base64::Engine::encode(&crate::cid_verify::BYTES_B64, &signature) } }); @@ -249,9 +249,8 @@ impl AttestationSigner { .and_then(|b| b.as_str()) .ok_or_else(|| AttestationError::MissingField("signature.signature.$bytes".into()))?; - let sig_bytes = - base64::Engine::decode(&base64::engine::general_purpose::STANDARD, sig_bytes_b64) - .map_err(|e| AttestationError::Encoding(format!("invalid base64: {e}")))?; + let sig_bytes = base64::Engine::decode(&crate::cid_verify::BYTES_B64, sig_bytes_b64) + .map_err(|e| AttestationError::Encoding(format!("invalid base64: {e}")))?; let signature = Signature::from_slice(&sig_bytes[..]) .map_err(|e| AttestationError::Signing(format!("invalid signature bytes: {e}")))?; @@ -349,8 +348,7 @@ fn legacy_json_to_cbor(value: &Value) -> ciborium::Value { // Handle special $bytes encoding for binary data if obj.len() == 1 && let Some(Value::String(b64)) = obj.get("$bytes") - && let Ok(bytes) = - base64::Engine::decode(&base64::engine::general_purpose::STANDARD, b64) + && let Ok(bytes) = base64::Engine::decode(&crate::cid_verify::BYTES_B64, b64) { return ciborium::Value::Bytes(bytes); } @@ -816,6 +814,37 @@ mod tests { ); } + /// The data model makes `=` padding optional on `$bytes`, and at least one + /// major implementation (jetstream) omits it. A signature we emitted padded + /// comes back off the firehose 86 characters instead of 88, and must still + /// verify โ€” the padding carries no information, only the bytes do. + #[test] + fn signatures_verify_with_unpadded_bytes() { + let signer = test_signer(); + let mut record = ordering_sensitive_record(); + signer.sign_record(&mut record, "did:plc:test").unwrap(); + + let padded = record["signatures"][0]["signature"]["$bytes"] + .as_str() + .unwrap() + .to_string(); + let unpadded = padded.trim_end_matches('=').to_string(); + assert_ne!( + padded, unpadded, + "signing must emit padding for this to test anything" + ); + + record["signatures"][0]["signature"]["$bytes"] = serde_json::json!(unpadded); + let sig = record["signatures"][0].clone(); + + assert_eq!( + signer + .verify_record_signature_detailed(&record, &sig, "did:plc:test") + .unwrap(), + SignatureVerification::Valid(SignatureEncoding::Current), + ); + } + #[test] fn fallback_does_not_accept_tampered_records() { let signer = test_signer(); -- 2.51.2