diff --git a/Cargo.lock b/Cargo.lock index 3a85357..764634d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6279,6 +6279,7 @@ dependencies = [ "bs58", "bytes", "chrono", + "ciborium", "cid", "ctor", "dotenvy", diff --git a/Cargo.toml b/Cargo.toml index cfad3af..294f779 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ webauthn-rs-proto = "0.5.4" [features] external-infra = [] [dev-dependencies] +ciborium = "0.2" ctor = "0.6.3" testcontainers = "0.26.2" testcontainers-modules = { version = "0.14.0", features = ["postgres"] } diff --git a/src/api/repo/record/utils.rs b/src/api/repo/record/utils.rs index acbb766..ab07234 100644 --- a/src/api/repo/record/utils.rs +++ b/src/api/repo/record/utils.rs @@ -2,33 +2,13 @@ use crate::state::AppState; use bytes::Bytes; use cid::Cid; use jacquard::types::{integer::LimitedU32, string::Tid}; +use jacquard_repo::commit::Commit; use jacquard_repo::storage::BlockStore; -use k256::ecdsa::{Signature, SigningKey, signature::Signer}; -use serde::Serialize; +use k256::ecdsa::SigningKey; use serde_json::json; +use std::str::FromStr; use uuid::Uuid; -/* - * Why custom commit signing instead of jacquard's Commit::sign()? - * - * Jacquard previously had a bug in how it created unsigned bytes for signing: - * it set sig to empty bytes and serialized (6-field CBOR map), while the - * ATProto spec creates a struct *without* the sig field (5-field CBOR map). - * These produce different CBOR bytes, so signatures didn't verify with relays. - * - * The bug has been fixed in jacquard, but the fix is untested here. - * TODO: Switch back to jacquard's Commit::sign() and verify it works. - */ - -#[derive(Serialize)] -struct UnsignedCommit<'a> { - data: Cid, - did: &'a str, - prev: Option, - rev: &'a str, - version: i64, -} - pub fn create_signed_commit( did: &str, data: Cid, @@ -36,36 +16,17 @@ pub fn create_signed_commit( prev: Option, signing_key: &SigningKey, ) -> Result<(Vec, Bytes), String> { - let unsigned = UnsignedCommit { - data, - did, - prev, - rev, - version: 3, - }; - let unsigned_bytes = serde_ipld_dagcbor::to_vec(&unsigned) - .map_err(|e| format!("Failed to serialize unsigned commit: {:?}", e))?; - let sig: Signature = signing_key.sign(&unsigned_bytes); - let sig_bytes = Bytes::copy_from_slice(&sig.to_bytes()); - #[derive(Serialize)] - struct SignedCommit<'a> { - data: Cid, - did: &'a str, - prev: Option, - rev: &'a str, - #[serde(with = "serde_bytes")] - sig: &'a [u8], - version: i64, - } - let signed = SignedCommit { - data, - did, - prev, - rev, - sig: &sig_bytes, - version: 3, - }; - let signed_bytes = serde_ipld_dagcbor::to_vec(&signed) + let did = jacquard::types::string::Did::new(did) + .map_err(|e| format!("Invalid DID: {:?}", e))?; + let rev = jacquard::types::string::Tid::from_str(rev) + .map_err(|e| format!("Invalid TID: {:?}", e))?; + let unsigned = Commit::new_unsigned(did, data, rev, prev); + let signed = unsigned + .sign(signing_key) + .map_err(|e| format!("Failed to sign commit: {:?}", e))?; + let sig_bytes = signed.sig().clone(); + let signed_bytes = signed + .to_cbor() .map_err(|e| format!("Failed to serialize signed commit: {:?}", e))?; Ok((signed_bytes, sig_bytes)) } @@ -423,7 +384,7 @@ pub async fn create_record_internal( let uri = format!("at://{}/{}/{}", did, collection, rkey); Ok((uri, result.commit_cid)) } -use std::str::FromStr; + pub async fn sequence_identity_event( state: &AppState, did: &str, diff --git a/tests/commit_signing.rs b/tests/commit_signing.rs new file mode 100644 index 0000000..a10e0d4 --- /dev/null +++ b/tests/commit_signing.rs @@ -0,0 +1,121 @@ +use cid::Cid; +use jacquard::types::{integer::LimitedU32, string::Tid}; +use jacquard_repo::commit::Commit; +use k256::ecdsa::SigningKey; +use std::str::FromStr; + +#[test] +fn test_commit_signing_produces_valid_signature() { + let signing_key = SigningKey::random(&mut rand::thread_rng()); + + let did = "did:plc:testuser123456789abcdef"; + let data_cid = + Cid::from_str("bafyreib2rxk3ryblouj3fxza5jvx6psmwewwessc4m6g6e7pqhhkwqomfi").unwrap(); + let rev = Tid::now(LimitedU32::MIN); + + let did_typed = jacquard::types::string::Did::new(did).unwrap(); + let unsigned = Commit::new_unsigned(did_typed, data_cid, rev, None); + let signed = unsigned.sign(&signing_key).unwrap(); + + let pubkey_bytes = signing_key.verifying_key().to_encoded_point(true); + let pubkey = jacquard::types::crypto::PublicKey { + codec: jacquard::types::crypto::KeyCodec::Secp256k1, + bytes: std::borrow::Cow::Owned(pubkey_bytes.as_bytes().to_vec()), + }; + + signed.verify(&pubkey).expect("signature should verify"); +} + +#[test] +fn test_commit_signing_with_prev() { + let signing_key = SigningKey::random(&mut rand::thread_rng()); + + let did = "did:plc:testuser123456789abcdef"; + let data_cid = + Cid::from_str("bafyreib2rxk3ryblouj3fxza5jvx6psmwewwessc4m6g6e7pqhhkwqomfi").unwrap(); + let prev_cid = + Cid::from_str("bafyreigxmvutyl3k5m4guzwxv3xf34gfxjlykgfdqkjmf32vwb5vcjxlui").unwrap(); + let rev = Tid::now(LimitedU32::MIN); + + let did_typed = jacquard::types::string::Did::new(did).unwrap(); + let unsigned = Commit::new_unsigned(did_typed, data_cid, rev, Some(prev_cid)); + let signed = unsigned.sign(&signing_key).unwrap(); + + let pubkey_bytes = signing_key.verifying_key().to_encoded_point(true); + let pubkey = jacquard::types::crypto::PublicKey { + codec: jacquard::types::crypto::KeyCodec::Secp256k1, + bytes: std::borrow::Cow::Owned(pubkey_bytes.as_bytes().to_vec()), + }; + + signed.verify(&pubkey).expect("signature should verify"); +} + +#[test] +fn test_unsigned_commit_has_5_fields() { + let did = "did:plc:test"; + let data_cid = + Cid::from_str("bafyreib2rxk3ryblouj3fxza5jvx6psmwewwessc4m6g6e7pqhhkwqomfi").unwrap(); + let rev = Tid::from_str("3masrxv55po22").unwrap(); + + let did_typed = jacquard::types::string::Did::new(did).unwrap(); + let unsigned = Commit::new_unsigned(did_typed, data_cid, rev, None); + + let unsigned_bytes = serde_ipld_dagcbor::to_vec(&unsigned).unwrap(); + + let decoded: ciborium::Value = ciborium::from_reader(&unsigned_bytes[..]).unwrap(); + if let ciborium::Value::Map(map) = decoded { + assert_eq!( + map.len(), + 5, + "Unsigned commit must have exactly 5 fields (data, did, prev, rev, version) - no sig field" + ); + let keys: Vec = map + .iter() + .filter_map(|(k, _)| { + if let ciborium::Value::Text(s) = k { + Some(s.clone()) + } else { + None + } + }) + .collect(); + assert!(keys.contains(&"data".to_string())); + assert!(keys.contains(&"did".to_string())); + assert!(keys.contains(&"prev".to_string())); + assert!(keys.contains(&"rev".to_string())); + assert!(keys.contains(&"version".to_string())); + assert!( + !keys.contains(&"sig".to_string()), + "Unsigned commit must NOT contain sig field" + ); + } else { + panic!("Expected CBOR map"); + } +} + +#[test] +fn test_create_signed_commit_helper() { + use tranquil_pds::api::repo::record::utils::create_signed_commit; + + let signing_key = SigningKey::random(&mut rand::thread_rng()); + let did = "did:plc:testuser123456789abcdef"; + let data_cid = + Cid::from_str("bafyreib2rxk3ryblouj3fxza5jvx6psmwewwessc4m6g6e7pqhhkwqomfi").unwrap(); + let rev = Tid::now(LimitedU32::MIN).to_string(); + + let (signed_bytes, sig) = create_signed_commit(did, data_cid, &rev, None, &signing_key) + .expect("signing should succeed"); + + assert!(!signed_bytes.is_empty()); + assert_eq!(sig.len(), 64); + + let commit = Commit::from_cbor(&signed_bytes).expect("should parse as valid commit"); + + let pubkey_bytes = signing_key.verifying_key().to_encoded_point(true); + let pubkey = jacquard::types::crypto::PublicKey { + codec: jacquard::types::crypto::KeyCodec::Secp256k1, + bytes: std::borrow::Cow::Owned(pubkey_bytes.as_bytes().to_vec()), + }; + + commit.verify(&pubkey).expect("signature should verify"); +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 7e4fd74..8f66028 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -305,25 +305,23 @@ pub async fn verify_new_account(client: &Client, did: &str) -> String { .await .expect("Failed to get verification code"); - let verification_code = body_text - .lines() - .find(|line| line.contains("verification code:") || line.contains("code is:")) - .and_then(|line| { - if line.contains("verification code:") { - line.split("verification code:") - .nth(1) - .map(|s| s.trim().to_string()) - } else { - line.split("code is:").nth(1).map(|s| s.trim().to_string()) - } + let lines: Vec<&str> = body_text.lines().collect(); + let verification_code = lines + .iter() + .enumerate() + .find(|(_, line)| { + line.contains("verification code is:") || line.contains("code is:") }) - .unwrap_or_else(|| { + .and_then(|(i, _)| lines.get(i + 1).map(|s| s.trim().to_string())) + .or_else(|| { body_text - .lines() - .find(|line| line.trim().starts_with("MX") && line.contains('-')) - .map(|s| s.trim().to_string()) - .unwrap_or_default() - }); + .split_whitespace() + .find(|word| { + word.contains('-') && word.chars().filter(|c| *c == '-').count() >= 3 + }) + .map(|s| s.to_string()) + }) + .unwrap_or_else(|| body_text.clone()); let confirm_payload = json!({ "did": did, @@ -480,29 +478,23 @@ async fn create_account_and_login_internal(client: &Client, make_admin: bool) -> .fetch_one(&pool) .await .expect("Failed to get verification from comms_queue"); - let verification_code = body_text - .lines() - .find(|line| line.contains("verification code:") || line.contains("code is:")) - .and_then(|line| { - if line.contains("verification code:") { - line.split("verification code:") - .nth(1) - .map(|s| s.trim().to_string()) - } else if line.contains("code is:") { - line.split("code is:").nth(1).map(|s| s.trim().to_string()) - } else { - None - } + let lines: Vec<&str> = body_text.lines().collect(); + let verification_code = lines + .iter() + .enumerate() + .find(|(_, line)| { + line.contains("verification code is:") || line.contains("code is:") }) - .unwrap_or_else(|| { + .and_then(|(i, _)| lines.get(i + 1).map(|s| s.trim().to_string())) + .or_else(|| { body_text .split_whitespace() .find(|word| { word.contains('-') && word.chars().filter(|c| *c == '-').count() >= 3 }) - .unwrap_or(&body_text) - .to_string() - }); + .map(|s| s.to_string()) + }) + .unwrap_or_else(|| body_text.clone()); let confirm_payload = json!({ "did": did, diff --git a/tests/import_with_verification.rs b/tests/import_with_verification.rs index ba223ef..d0a3aff 100644 --- a/tests/import_with_verification.rs +++ b/tests/import_with_verification.rs @@ -3,12 +3,14 @@ use cid::Cid; use common::*; use ipld_core::ipld::Ipld; use jacquard::types::{integer::LimitedU32, string::Tid}; -use k256::ecdsa::{Signature, SigningKey, signature::Signer}; +use jacquard_repo::commit::Commit; +use k256::ecdsa::SigningKey; use reqwest::StatusCode; use serde_json::json; use sha2::{Digest, Sha256}; use sqlx::PgPool; use std::collections::BTreeMap; +use std::str::FromStr; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -89,27 +91,11 @@ fn create_did_document( } fn create_signed_commit(did: &str, data_cid: &Cid, signing_key: &SigningKey) -> (Vec, Cid) { - let rev = Tid::now(LimitedU32::MIN).to_string(); - let unsigned = Ipld::Map(BTreeMap::from([ - ("data".to_string(), Ipld::Link(*data_cid)), - ("did".to_string(), Ipld::String(did.to_string())), - ("prev".to_string(), Ipld::Null), - ("rev".to_string(), Ipld::String(rev.clone())), - ("sig".to_string(), Ipld::Bytes(vec![])), - ("version".to_string(), Ipld::Integer(3)), - ])); - let unsigned_bytes = serde_ipld_dagcbor::to_vec(&unsigned).unwrap(); - let signature: Signature = signing_key.sign(&unsigned_bytes); - let sig_bytes = signature.to_bytes().to_vec(); - let signed = Ipld::Map(BTreeMap::from([ - ("data".to_string(), Ipld::Link(*data_cid)), - ("did".to_string(), Ipld::String(did.to_string())), - ("prev".to_string(), Ipld::Null), - ("rev".to_string(), Ipld::String(rev)), - ("sig".to_string(), Ipld::Bytes(sig_bytes)), - ("version".to_string(), Ipld::Integer(3)), - ])); - let signed_bytes = serde_ipld_dagcbor::to_vec(&signed).unwrap(); + let rev = Tid::now(LimitedU32::MIN); + let did = jacquard::types::string::Did::new(did).expect("valid DID"); + let unsigned = Commit::new_unsigned(did, *data_cid, rev, None); + let signed = unsigned.sign(signing_key).expect("signing failed"); + let signed_bytes = signed.to_cbor().expect("serialization failed"); let cid = make_cid(&signed_bytes); (signed_bytes, cid) } diff --git a/tests/jwt_security.rs b/tests/jwt_security.rs index 5c885e0..f503ba9 100644 --- a/tests/jwt_security.rs +++ b/tests/jwt_security.rs @@ -692,25 +692,23 @@ async fn test_refresh_token_replay_protection() { "SELECT body FROM comms_queue WHERE user_id = (SELECT id FROM users WHERE did = $1) AND comms_type = 'email_verification' ORDER BY created_at DESC LIMIT 1", did ).fetch_one(&pool).await.unwrap(); - let code = body_text - .lines() - .find(|line| line.contains("verification code:") || line.contains("code is:")) - .and_then(|line| { - if line.contains("verification code:") { - line.split("verification code:") - .nth(1) - .map(|s| s.trim().to_string()) - } else { - line.split("code is:").nth(1).map(|s| s.trim().to_string()) - } + let lines: Vec<&str> = body_text.lines().collect(); + let code = lines + .iter() + .enumerate() + .find(|(_, line)| { + line.contains("verification code is:") || line.contains("code is:") }) - .unwrap_or_else(|| { + .and_then(|(i, _)| lines.get(i + 1).map(|s| s.trim().to_string())) + .or_else(|| { body_text - .lines() - .find(|line| line.trim().starts_with("MX") && line.contains('-')) - .map(|s| s.trim().to_string()) - .unwrap_or_default() - }); + .split_whitespace() + .find(|word| { + word.contains('-') && word.chars().filter(|c| *c == '-').count() >= 3 + }) + .map(|s| s.to_string()) + }) + .unwrap_or_else(|| body_text.clone()); let confirm = http_client .post(format!("{}/xrpc/com.atproto.server.confirmSignup", url))