malpercio.dev / ezpds
An easy-to-host PDS on the ATProtocol, iPhone and MacOS. Maintain control of your keys and data, always.
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

fix(identity-wallet): address PR review — critical ceremony bugs and logging improvements

CRITICAL FIXES:
- [Critical 1] rotation_key_public now sends device_key.key_id instead of device_key.multibase (line 315)
- device_key.multibase is raw base58btc without 'did:key:' prefix; relay validates field starts with 'did:key:z'
- device_key.key_id contains the full did:key: URI required by relay

- [Critical 2] signed_creation_op changed from String to serde_json::Value (line 54, 315-319)
- String type caused double-encoded JSON: reqwest serializes String as JSON literal, relay received \"json\" and verify failed
- Now parses genesis_op.signed_op_json at call site to deserialize once before sending
- Errors map to SigningFailed with structured error logging

- [Critical 3] Removed #[serde(rename_all = "camelCase")] from CreateDidResponse (line 58)
- Relay serializes session_token in snake_case per HTTP API conventions
- With camelCase rename, client received empty string for session_token field
- Now correctly deserializes relay's snake_case response

HIGH-PRIORITY FIXES:
- [High 4] Error bodies now logged on non-success responses (line 270-271, 326-329)
- GET /v1/relay/keys non-success: logs status and body as tracing::error\! instead of silently returning RelayKeyFetchFailed
- POST /v1/dids non-success: logs status and body with structured error field
- Improves observability when relay returns error details

- [High 5] Removed #[serde(default)] from RelaySigningKey fields (line 41-45)
- public_key and algorithm fields now required; serde fails loudly if relay truncates response
- Prevents silent contract violation where client accepts empty strings

- [High 6] Promoted warn\! to error\! for critical failures (lines 280, 329, 334, 337, 350)
- Step 3 genesis op signing failure: error\! (was warn\!)
- Step 5 POST /v1/dids deserialization on 2xx: error\! (was warn\!)
- Step 6 session-token persistence: error\! (was warn\!)
- Step 7 DID persistence: error\! with did field for forensics (was warn\!)

- [High 7] Fixed DIDCeremonyError::KeyNotFound #[error] message (line 133)
- Old: 'device key not found; call get_or_create before ceremony' (misleading, variant used for all failures)
- New: 'failed to get or create device key' (accurate for all failure modes)

MEDIUM-PRIORITY FIXES:
- [Medium 8] Fixed byte-count comment in insert_test_key (get_relay_signing_key.rs:65-68)
- Old: '60 zero-bytes base64-encoded + padding = 84 chars' (wrong: 60 bytes → 80 chars, 63 bytes → 84 chars)
- New: Correctly states '84-char placeholder encodes 63 bytes (with padding characters)'

- [Medium 9] Log DID in Step 7 keychain failure (lib.rs:350)
- Added did field to error\! log for forensic trail when DID persistence fails
- If write fails after relay created DID, DID value now captured in logs

- [Medium 10] Added 64-byte signature length validation (plc.rs:216-221)
- build_did_plc_genesis_op_with_external_signer now validates callback returns exactly 64 bytes
- Wrong-length signature silently produces incorrect DID; now returns CryptoError::PlcOperation

- [Medium 11] Added doc comment to build_did_plc_genesis_op wrapper (plc.rs:253-263)
- Clarifies this is a convenience wrapper for extractable keys using inline signing callback
- Documents delegation to external_signer variant

- [Medium 12] Fixed Step 3 comment about Secure Enclave (lib.rs:281)
- Old: 'private key never leaves the SE' (inaccurate on Simulator/macOS)
- New: 'On device, private key never leaves SE; on Simulator and macOS, software key used instead'

- [Medium 13] Tightened publicKey assertion in test (get_relay_signing_key.rs:111)
- Old: assert\!(json["publicKey"].is_string(), ...) (only checks presence)
- New: assert_eq\!(json["publicKey"], "zTestPublicKey123") (verifies actual value)

VERIFICATION:
✓ cargo build -p relay -p crypto: success
✓ cargo test -p relay get_relay: 4/4 tests pass
✓ cargo test -p crypto: 44/44 tests pass
✓ cargo test -p identity-wallet --lib -- tests:: --skip device_key: 21/21 serialization tests pass
✓ cargo clippy --workspace -- -D warnings: no warnings introduced
✓ cargo fmt --all --check: all files properly formatted

authored by

Malpercio and committed by
Tangled 41886ac4 94cb35af

+46 -21
+21 -17
apps/identity-wallet/src-tauri/src/lib.rs
··· 38 38 #[serde(rename_all = "camelCase")] 39 39 struct RelaySigningKey { 40 40 key_id: String, 41 - #[serde(default)] 42 41 #[allow(dead_code)] 43 42 public_key: String, 44 - #[serde(default)] 45 43 #[allow(dead_code)] 46 44 algorithm: String, 47 45 } ··· 51 49 #[serde(rename_all = "camelCase")] 52 50 struct CreateDidRequest { 53 51 rotation_key_public: String, 54 - signed_creation_op: String, 52 + signed_creation_op: serde_json::Value, 55 53 } 56 54 57 55 /// Response from POST /v1/dids — the promoted DID and upgraded session token. 58 56 #[derive(Deserialize)] 59 - #[serde(rename_all = "camelCase")] 60 57 struct CreateDidResponse { 61 58 did: String, 62 59 session_token: String, ··· 130 127 #[derive(Debug, Serialize, thiserror::Error)] 131 128 #[serde(tag = "code", rename_all = "SCREAMING_SNAKE_CASE")] 132 129 pub enum DIDCeremonyError { 133 - #[error("device key not found; call get_or_create before ceremony")] 130 + #[error("failed to get or create device key")] 134 131 KeyNotFound, 135 132 #[error("failed to fetch relay signing key")] 136 133 RelayKeyFetchFailed, ··· 265 262 message: e.to_string(), 266 263 })?; 267 264 268 - if resp.status().as_u16() == 503 { 265 + let status = resp.status(); 266 + if status.as_u16() == 503 { 269 267 return Err(DIDCeremonyError::NoRelaySigningKey); 270 268 } 271 - if !resp.status().is_success() { 272 - tracing::warn!(status = %resp.status(), "GET /v1/relay/keys returned non-success status"); 269 + if !status.is_success() { 270 + let body = resp.text().await.unwrap_or_default(); 271 + tracing::error!(status = %status, body = %body, "GET /v1/relay/keys returned non-success status"); 273 272 return Err(DIDCeremonyError::RelayKeyFetchFailed); 274 273 } 275 274 276 275 let relay_key: RelaySigningKey = resp.json().await.map_err(|e| { 277 - tracing::warn!(error = %e, "failed to deserialize relay signing key response"); 276 + tracing::error!(error = %e, "failed to deserialize relay signing key response"); 278 277 DIDCeremonyError::RelayKeyFetchFailed 279 278 })?; 280 279 281 280 // Step 3: Build signed genesis op — device key as rotation key, relay key as signing key. 282 - // The sign callback calls device_key::sign() so the private key never leaves the SE. 281 + // On device, the private key never leaves the Secure Enclave; on Simulator and macOS, a software key is used instead. 283 282 let rotation_key = DidKeyUri(device_key.key_id.clone()); 284 283 let signing_key = DidKeyUri(relay_key.key_id.clone()); 285 284 ··· 294 293 }, 295 294 ) 296 295 .map_err(|e| { 297 - tracing::warn!(error = %e, "genesis op signing failed during DID ceremony"); 296 + tracing::error!(error = %e, "genesis op signing failed during DID ceremony"); 298 297 DIDCeremonyError::SigningFailed 299 298 })?; 300 299 ··· 310 309 311 310 // Step 5: POST the signed genesis op to the relay to promote the account to a full DID. 312 311 let create_did_req = CreateDidRequest { 313 - rotation_key_public: device_key.multibase, 314 - signed_creation_op: genesis_op.signed_op_json, 312 + rotation_key_public: device_key.key_id, 313 + signed_creation_op: serde_json::from_str(&genesis_op.signed_op_json).map_err(|e| { 314 + tracing::error!(error = %e, "genesis op JSON is not valid JSON"); 315 + DIDCeremonyError::SigningFailed 316 + })?, 315 317 }; 316 318 317 319 let resp = RELAY_CLIENT ··· 322 324 })?; 323 325 324 326 if !resp.status().is_success() { 325 - tracing::warn!(status = %resp.status(), "POST /v1/dids returned non-success status"); 327 + let status = resp.status(); 328 + let body = resp.text().await.unwrap_or_default(); 329 + tracing::error!(status = %status, body = %body, "POST /v1/dids returned non-success status"); 326 330 return Err(DIDCeremonyError::DidCreationFailed); 327 331 } 328 332 329 333 let create_did_resp: CreateDidResponse = resp.json().await.map_err(|e| { 330 - tracing::warn!(error = %e, "failed to deserialize POST /v1/dids response"); 334 + tracing::error!(error = %e, "failed to deserialize POST /v1/dids response"); 331 335 DIDCeremonyError::DidCreationFailed 332 336 })?; 333 337 334 338 // Step 6: Overwrite session-token with the upgraded full session token. 335 339 keychain::store_item("session-token", create_did_resp.session_token.as_bytes()).map_err( 336 340 |e| { 337 - tracing::warn!(error = %e, "failed to persist upgraded session-token to keychain"); 341 + tracing::error!(error = %e, "failed to persist upgraded session-token to keychain"); 338 342 DIDCeremonyError::KeychainError 339 343 }, 340 344 )?; 341 345 342 346 // Step 7: Persist the DID for use in subsequent app sessions. 343 347 keychain::store_item("did", create_did_resp.did.as_bytes()).map_err(|e| { 344 - tracing::warn!(error = %e, "failed to persist DID to keychain"); 348 + tracing::error!(error = %e, did = %create_did_resp.did, "failed to persist DID to keychain"); 345 349 DIDCeremonyError::KeychainError 346 350 })?; 347 351
+22
crates/crypto/src/plc.rs
··· 214 214 // The callback must return raw 64-byte r‖s P-256 ECDSA signature bytes. 215 215 let sig_bytes = sign(&unsigned_cbor)?; 216 216 217 + if sig_bytes.len() != 64 { 218 + return Err(CryptoError::PlcOperation(format!( 219 + "signing callback returned {} bytes, expected 64", 220 + sig_bytes.len() 221 + ))); 222 + } 223 + 217 224 // Step 4: base64url-encode the signature (no padding). 218 225 let sig_str = URL_SAFE_NO_PAD.encode(&sig_bytes); 219 226 ··· 250 257 }) 251 258 } 252 259 260 + /// Convenience wrapper for callers with extractable P-256 private key bytes. 261 + /// 262 + /// Constructs an inline signing callback from the provided private key bytes and delegates to 263 + /// [`build_did_plc_genesis_op_with_external_signer`]. 264 + /// 265 + /// # Parameters 266 + /// - `rotation_key`: The user's device key (highest-priority rotation key). Placed at `rotationKeys[0]`. 267 + /// - `signing_key`: The relay's signing key. Placed at `rotationKeys[1]` and `verificationMethods.atproto`. 268 + /// - `signing_private_key`: Raw 32-byte P-256 private key scalar for `signing_key`. 269 + /// - `handle`: The account handle, e.g. `"alice.example.com"`. Stored as `"at://alice.example.com"` in `alsoKnownAs`. 270 + /// - `service_endpoint`: The relay's public URL, e.g. `"https://relay.example.com"`. 271 + /// 272 + /// # Errors 273 + /// Returns `CryptoError::PlcOperation` if `signing_private_key` is not a valid P-256 scalar 274 + /// (e.g. all-zero bytes, or a value ≥ the curve order). 253 275 pub fn build_did_plc_genesis_op( 254 276 rotation_key: &DidKeyUri, 255 277 signing_key: &DidKeyUri,
+3 -4
crates/relay/src/routes/get_relay_signing_key.rs
··· 63 63 /// `private_key_encrypted` is a NOT NULL column, but the GET handler never reads it, 64 64 /// so any valid base64 value satisfies the constraint. The real format is 65 65 /// base64(nonce(12) || ciphertext(32) || tag(16)) = 80 base64 chars. The 84-char 66 - /// placeholder below (60 zero-bytes base64-encoded + padding) is intentionally a 67 - /// dummy — replace with a correct 80-char value if a test ever needs to read 68 - /// private_key_encrypted back. 66 + /// placeholder below encodes 63 bytes (with padding characters) — replace with a valid 80-char value 67 + /// (60 bytes, no padding) if a test ever needs to read private_key_encrypted back. 69 68 async fn insert_test_key(db: &sqlx::SqlitePool, key_id: &str, created_at: &str) { 70 69 sqlx::query( 71 70 "INSERT INTO relay_signing_keys \ ··· 108 107 let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); 109 108 assert_eq!(json["keyId"], "did:key:zTestKey1"); 110 109 assert_eq!(json["algorithm"], "p256"); 111 - assert!(json["publicKey"].is_string(), "publicKey must be present"); 110 + assert_eq!(json["publicKey"], "zTestPublicKey123"); 112 111 } 113 112 114 113 #[tokio::test]