Use jacquard commit signing

Cargo.lock

@@ -6279,6 +6279,7 @@ dependencies = [
  "bs58",
  "bytes",
  "chrono",
+ "ciborium",
  "cid",
  "ctor",
  "dotenvy",

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"] }

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<Cid>,
-    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<Cid>,
     signing_key: &SigningKey,
 ) -> Result<(Vec<u8>, 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<Cid>,
-        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,

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<String> = 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");
+}

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,

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<u8>, 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)
 }

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))