An easy-to-host PDS on the ATProtocol, iPhone and MacOS. Maintain control of your keys and data, always.
fix(device-key): address PR review — SE orphaning, atomic writes, error discrimination
- CRITICAL-1: SE fast-path now checks both SE_PUB_ACCOUNT and SE_APP_LABEL_ACCOUNT
to ensure state consistency. Errors are now properly discriminated:
ItemNotFound (no key yet) falls through to generation, transient OS errors
(errSecInteractionNotAllowed, etc.) are propagated as KeychainError to prevent
key orphaning on retries.
- CRITICAL-2: Non-atomic SE Keychain writes now roll back SE_PUB_ACCOUNT if
application_label() or the SE_APP_LABEL_ACCOUNT write fails, preventing
permanent inconsistency where fast-path finds public key but sign() fails.
- HIGH-1: search.search() OS errors now map to KeychainError instead of KeyNotFound.
KeyNotFound is reserved for the case where search succeeds but finds no key,
matching the contract that KeyNotFound signals 'call get_or_create first'.
- HIGH-2: Simulator sign() now uses is_not_found() to distinguish ItemNotFound
from transient OS errors, preventing false KeyNotFound returns.
- HIGH-3: Device key errors in create_account now map to KeychainError instead
of Unknown, using the correct error variant already defined.
- Added is_not_found() helper to keychain.rs to centralize errSecItemNotFound
detection (-25300). Use this to distinguish missing items from OS errors.
- Enforced --test-threads=1 via [env] RUST_TEST_THREADS in .cargo/config.toml
to prevent Keychain races in tests (previously advisory comment only).
- Suggestion fixes: Updated to_vec() comment (line 65), removed Phase 2 header,
removed compiler note about ambiguous type, updated normalize_s() comment.
- Test improvements: Added assertions for SigningFailed and InvalidSignature
variants to device_key_error_serializes_as_code test. Added cryptographic
verification test (sign_output_verifies_against_public_key) that confirms
signatures actually verify against the public key from get_or_create().
- Renamed test to device_key_contract_satisfies_relay_format to clarify that
it tests the device_key module contract (format + idempotency) that
create_account depends on, not create_account itself.
- Added libiconv library path to aarch64-apple-darwin rustflags to resolve
linking errors in host proc-macro builds (phf_macros, darling, etc.).
+95
-39
+3
apps/identity-wallet/src-tauri/.cargo/config.toml
+3
apps/identity-wallet/src-tauri/.cargo/config.toml
···
12
12
# Xcode is always installed at /Applications/Xcode.app on macOS.
13
13
14
14
[env]
15
+
RUST_TEST_THREADS = "1"
15
16
CC_aarch64_apple_ios_sim = "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
16
17
CC_aarch64_apple_ios = "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
17
18
CC_aarch64_apple_darwin = "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
···
37
38
38
39
[target.aarch64-apple-darwin]
39
40
linker = "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
41
+
# Ensure host builds can find system libraries like iconv
42
+
rustflags = ["-L", "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib"]
40
43
41
44
[target.aarch64-apple-ios-sim]
42
45
linker = "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
+80
-32
apps/identity-wallet/src-tauri/src/device_key.rs
+80
-32
apps/identity-wallet/src-tauri/src/device_key.rs
···
62
62
// No key yet — generate a new P-256 keypair via the crypto crate.
63
63
let keypair =
64
64
crypto::generate_p256_keypair().map_err(|_| DeviceKeyError::KeyGenerationFailed)?;
65
-
// Deref Zeroizing<[u8; 32]> to [u8; 32], then collect as Vec<u8>.
65
+
// to_vec(): Deref gives &[u8; 32], coerces to &[u8], allocates into Vec<u8>.
66
66
let bytes = keypair.private_key_bytes.to_vec();
67
67
crate::keychain::store_item(ACCOUNT, &bytes).map_err(|e| {
68
68
DeviceKeyError::KeychainError {
···
105
105
const ACCOUNT: &str = "device-rotation-key-priv";
106
106
107
107
// If the key doesn't exist, signal that get_or_create must be called first.
108
-
let private_bytes =
109
-
crate::keychain::get_item(ACCOUNT).map_err(|_| DeviceKeyError::KeyNotFound)?;
108
+
// Distinguish ItemNotFound from other OS errors.
109
+
let private_bytes = crate::keychain::get_item(ACCOUNT).map_err(|e| {
110
+
if crate::keychain::is_not_found(&e) {
111
+
DeviceKeyError::KeyNotFound
112
+
} else {
113
+
DeviceKeyError::KeychainError {
114
+
message: e.to_string(),
115
+
}
116
+
}
117
+
})?;
110
118
111
119
let signing_key =
112
120
SigningKey::from_slice(&private_bytes).map_err(|_| DeviceKeyError::SigningFailed)?;
···
121
129
122
130
// ── Real device (Secure Enclave) path ────────────────────────────────────────
123
131
//
124
-
// Phase 2 implementation using security_framework 3.x safe wrapper.
125
132
// The SE private key is permanent and non-extractable; the public key and
126
133
// application_label (SHA1 hash) are stored in the regular Keychain for lookup.
127
134
···
134
141
135
142
#[cfg(all(target_os = "ios", not(target_env = "sim")))]
136
143
pub fn get_or_create() -> Result<DevicePublicKey, DeviceKeyError> {
137
-
// Fast path: if we already stored the compressed public key, return it directly.
144
+
// Fast path: if we already stored the compressed public key and app_label, return it directly.
138
145
// This avoids SE hardware interaction on every call after first generation.
139
-
if let Ok(compressed) = crate::keychain::get_item(SE_PUB_ACCOUNT) {
140
-
let multibase = multibase::encode(multibase::Base::Base58Btc, &compressed);
141
-
// did:key requires the P-256 multicodec varint prefix [0x80, 0x24] (0x1200 as LEB128).
142
-
const P256_MULTICODEC: &[u8] = &[0x80, 0x24];
143
-
let mut multikey = Vec::with_capacity(2 + compressed.len());
144
-
multikey.extend_from_slice(P256_MULTICODEC);
145
-
multikey.extend_from_slice(&compressed);
146
-
let key_id = format!(
147
-
"did:key:{}",
148
-
multibase::encode(multibase::Base::Base58Btc, &multikey)
149
-
);
150
-
return Ok(DevicePublicKey { multibase, key_id });
146
+
// Check BOTH SE_PUB_ACCOUNT and SE_APP_LABEL_ACCOUNT to ensure state consistency.
147
+
match (
148
+
crate::keychain::get_item(SE_PUB_ACCOUNT),
149
+
crate::keychain::get_item(SE_APP_LABEL_ACCOUNT),
150
+
) {
151
+
(Ok(compressed), Ok(_)) => {
152
+
// Both present — fast path. Return the cached public key.
153
+
let multibase = multibase::encode(multibase::Base::Base58Btc, &compressed);
154
+
// did:key requires the P-256 multicodec varint prefix [0x80, 0x24] (0x1200 as LEB128).
155
+
const P256_MULTICODEC: &[u8] = &[0x80, 0x24];
156
+
let mut multikey = Vec::with_capacity(2 + compressed.len());
157
+
multikey.extend_from_slice(P256_MULTICODEC);
158
+
multikey.extend_from_slice(&compressed);
159
+
let key_id = format!(
160
+
"did:key:{}",
161
+
multibase::encode(multibase::Base::Base58Btc, &multikey)
162
+
);
163
+
return Ok(DevicePublicKey { multibase, key_id });
164
+
}
165
+
(Err(e), _) | (_, Err(e)) if !crate::keychain::is_not_found(&e) => {
166
+
// Transient OS error — do not fall through to generation.
167
+
return Err(DeviceKeyError::KeychainError {
168
+
message: e.to_string(),
169
+
});
170
+
}
171
+
_ => {
172
+
// One or both missing — fall through to generate.
173
+
}
151
174
}
152
175
153
176
// Generate a new SE-backed P-256 key.
···
156
179
// not survive app restart.
157
180
// set_access_control with PRIVATE_KEY_USAGE is required for SE keys — the SE enforces
158
181
// that only explicitly-authorized operations can use the private key for signing.
159
-
//
160
-
// Note: SecAccessControl::create_with_protection takes Option<ProtectionMode> and a raw
161
-
// flags u64. The PRIVATE_KEY_USAGE flag is kSecAccessControlPrivateKeyUsage = 1 << 30.
162
-
// If the compiler reports an ambiguous type on the flags argument, use `0x4000_0000_u64`.
182
+
// The PRIVATE_KEY_USAGE flag is kSecAccessControlPrivateKeyUsage = 1 << 30.
163
183
let access_control = SecAccessControl::create_with_protection(
164
184
Some(ProtectionMode::AccessibleWhenUnlockedThisDeviceOnly),
165
185
1 << 30, // kSecAccessControlPrivateKeyUsage
···
204
224
}
205
225
})?;
206
226
207
-
// Store the application_label (OS-assigned SHA1 of public key, 20 bytes)
208
-
// so sign() can locate the SE private key on future app launches.
209
-
let app_label = priv_key
210
-
.application_label()
211
-
.ok_or(DeviceKeyError::KeyGenerationFailed)?;
227
+
// Get and store application_label. Roll back SE_PUB_ACCOUNT if this fails.
228
+
let app_label = priv_key.application_label().ok_or_else(|| {
229
+
let _ = crate::keychain::delete_item(SE_PUB_ACCOUNT);
230
+
DeviceKeyError::KeychainError {
231
+
message: "SE key created but application_label returned None; do not retry".into(),
232
+
}
233
+
})?;
212
234
crate::keychain::store_item(SE_APP_LABEL_ACCOUNT, &app_label).map_err(|e| {
235
+
let _ = crate::keychain::delete_item(SE_PUB_ACCOUNT);
213
236
DeviceKeyError::KeychainError {
214
237
message: e.to_string(),
215
238
}
···
246
269
.load_refs(true)
247
270
.limit(1);
248
271
249
-
let results = search.search().map_err(|_| DeviceKeyError::KeyNotFound)?;
272
+
let results = search.search().map_err(|e| DeviceKeyError::KeychainError {
273
+
message: e.to_string(),
274
+
})?;
250
275
251
276
// Extract the SecKey from the typed Reference result.
252
277
// SearchResult::Ref wraps a Reference enum; Reference::Key holds the already-wrapped SecKey.
···
265
290
266
291
// Convert DER to raw 64-byte r||s (the format expected by ATProto/did:plc).
267
292
// from_der() is a pure parser — it does NOT normalize low-S. Apple's SE may return
268
-
// high-S signatures. normalize_s() ensures s <= order/2 as required by ATProto.
293
+
// high-S signatures. normalize_s() returns None if already low-S (no-op), Some(normalized) if high-S was reduced.
269
294
let sig = Signature::from_der(&der_sig).map_err(|_| DeviceKeyError::InvalidSignature)?;
270
295
let sig = sig.normalize_s().unwrap_or(sig);
271
296
Ok(sig.to_bytes().to_vec())
···
333
358
);
334
359
}
335
360
361
+
// Verify that signatures produced by sign() actually verify against the public key
362
+
#[test]
363
+
fn sign_output_verifies_against_public_key() {
364
+
use p256::ecdsa::{signature::Verifier, Signature, VerifyingKey};
365
+
let key = get_or_create().expect("must have key");
366
+
let (_, compressed) = multibase::decode(&key.multibase).expect("must decode");
367
+
let verifying_key = VerifyingKey::from_sec1_bytes(&compressed).expect("must parse");
368
+
let data = b"verification test";
369
+
let sig_bytes = sign(data).expect("sign must succeed");
370
+
let sig = Signature::from_bytes(sig_bytes.as_slice().into()).expect("must parse sig");
371
+
verifying_key
372
+
.verify(data, &sig)
373
+
.expect("signature must verify");
374
+
}
375
+
336
376
// sign before get_or_create returns KeyNotFound
337
377
#[test]
338
378
fn sign_before_generate_returns_key_not_found() {
···
357
397
let json2 = serde_json::to_value(&err2).unwrap();
358
398
assert_eq!(json2["code"], "KEY_NOT_FOUND");
359
399
360
-
let err3 = DeviceKeyError::KeychainError {
400
+
let err3 = DeviceKeyError::SigningFailed;
401
+
let json3 = serde_json::to_value(&err3).unwrap();
402
+
assert_eq!(json3["code"], "SIGNING_FAILED");
403
+
404
+
let err4 = DeviceKeyError::InvalidSignature;
405
+
let json4 = serde_json::to_value(&err4).unwrap();
406
+
assert_eq!(json4["code"], "INVALID_SIGNATURE");
407
+
408
+
let err5 = DeviceKeyError::KeychainError {
361
409
message: "os error".into(),
362
410
};
363
-
let json3 = serde_json::to_value(&err3).unwrap();
364
-
assert_eq!(json3["code"], "KEYCHAIN_ERROR");
365
-
assert_eq!(json3["message"], "os error");
411
+
let json5 = serde_json::to_value(&err5).unwrap();
412
+
assert_eq!(json5["code"], "KEYCHAIN_ERROR");
413
+
assert_eq!(json5["message"], "os error");
366
414
}
367
415
368
416
// Ensures DevicePublicKey serializes key_id as keyId (camelCase) for Tauri IPC.
+8
apps/identity-wallet/src-tauri/src/keychain.rs
+8
apps/identity-wallet/src-tauri/src/keychain.rs
···
36
36
pub fn delete_item(account: &str) -> Result<(), KeychainError> {
37
37
delete_generic_password(SERVICE, account).map_err(KeychainError::Security)
38
38
}
39
+
40
+
/// Returns true if the error is errSecItemNotFound (OS status -25300).
41
+
/// Use this to distinguish "item does not exist" from transient OS errors.
42
+
pub fn is_not_found(err: &KeychainError) -> bool {
43
+
match err {
44
+
KeychainError::Security(e) => e.code() == -25300,
45
+
}
46
+
}
+4
-7
apps/identity-wallet/src-tauri/src/lib.rs
+4
-7
apps/identity-wallet/src-tauri/src/lib.rs
···
113
113
handle: String,
114
114
) -> Result<CreateAccountResult, CreateAccountError> {
115
115
// 1. Get or create the device's SE-backed (or simulator-fallback) P-256 key.
116
-
let device_key = device_key::get_or_create().map_err(|e| CreateAccountError::Unknown {
117
-
message: e.to_string(),
118
-
})?;
116
+
let device_key = device_key::get_or_create().map_err(|_| CreateAccountError::KeychainError)?;
119
117
120
118
// 2. POST to relay.
121
119
let req = CreateMobileAccountRequest {
···
332
330
assert!(json["message"].as_str().unwrap().contains("409:"));
333
331
}
334
332
335
-
// create_account uses device_key::get_or_create() as its public key source
336
-
// We verify: (a) the key exists and is correctly formatted, (b) it's stable so
337
-
// create_account always sends the same device_public_key for this device.
333
+
// Tests the device_key contract that create_account depends on: the returned key
334
+
// is correctly formatted (multibase base58btc) and is idempotent (stable across calls).
338
335
#[test]
339
-
fn create_account_uses_device_key_public_key() {
336
+
fn device_key_contract_satisfies_relay_format() {
340
337
let key = crate::device_key::get_or_create()
341
338
.expect("device_key::get_or_create must succeed — create_account depends on it");
342
339
// The relay expects multibase: 'z' + base58btc(33-byte compressed P-256 point).