id: b90f06b3b05449fdb9ad031b7ce57a2b
parent_id: 
item_type: 1
item_id: 1afe8b0b762043189ace149c4fc388b7
item_updated_time: 1781534129166
title_diff: "[{\"diffs\":[[1,\"Keyfob Station — Code Review & Open Issues\"]],\"start1\":0,\"start2\":0,\"length1\":0,\"length2\":42}]"
body_diff: "[{\"diffs\":[[1,\"# Keyfob Station — Code Review & Open Issues\\\n\\\nCode analysis of the `keyfob-station` Rust workspace (commit `0e0243b` → review fixes → JCShell-driven rework).\\\n\\\n> Companion to **\\\"Ford EoL Key Fob Provisioning System (SCP03)\\\"**.\\\n\\\n---\\\n\\\n## Summary\\\n\\\n11 findings identified during initial code review. **7 fixed** in the review pass; **2 resolved** by the JCShell-driven rework; **1 deferred** to real-card; **1 mock limitation** noted.\\\n\\\n**Verification gate:** 63 tests pass, `clippy -D warnings` clean, `fmt` clean, `self-test` green.\\\n\\\n---\\\n\\\n## JCShell-Driven Rework (2026-06-15 rev 2)\\\n\\\nAnalysis of the reference JCShell script (`fixtures/jcshell/`) revealed the real provisioning flow differs significantly from the initial implementation. The entire personalization pipeline was reworked.\\\n\\\n### What Changed\\\n\\\n| Area | Before | After |\\\n|---|---|---|\\\n| **Personalization model** | Flat `PayloadApdu` list (CLA/INS/P1/P2 + data) | Structured `PersonalizationItem` list with DGI tags + S-DEK encryption flag |\\\n| **STORE DATA INS** | `0xDA` | `0xE2` (GP personalization) |\\\n| **Security level** | `C_MAC_DEC` (0x03) | `C_MAC` only (0x01) — matches `auth mac` |\\\n| **Data encryption** | SCP03 C-DEC wraps all payload data | No C-DEC; private key pre-encrypted with S-DEK before DGI wrapping |\\\n| **Lifecycle transition** | `SET STATUS` (INS=0xF0, P1=0x0F P2=0x00) | `STORE DATA` P1=0x80 (INS=0xE2) — final block triggers UNPERSONALIZED → FACTORY |\\\n| **Post-perso** | Not modeled | Raw `00DB` commands (enable UICC/BLE transport), no SM |\\\n| **Mock applet** | `locked: bool` after SET STATUS | `lifecycle: Lifecycle` enum (Unpersonalized → Factory), C-DEC flag tracks EXTERNAL AUTH P1, accepts raw CLA=0x00 |\\\n| **Source** | `StaticProvisioningSource` only | + `FileContainerSource` (parses container dir → ProvisioningInput) |\\\n\\\n### New Modules\\\n\\\n| Module | Purpose |\\\n|---|---|\\\n| `kf-provision/src/perso.rs` | DGI TLV encoding, S-DEK pre-encryption (AES-CBC/M2/IV=zeros), block splitting (max 245B) |\\\n| `kf-source/src/container.rs` | Container directory parser: FESN.txt, irk_keyfob.key (base64), keyfob_private.der (PKCS#8 EC scalar extraction), keyfob_public.der |\\\n\\\n### New Data Types\\\n\\\n```rust\\\n// kf-source/src/types.rs\\\npub struct PersonalizationItem {\\\n    pub dgi: Vec<u8>,           // 2-byte DGI tag\\\n    pub data: Vec<u8>,          // raw payload\\\n    pub encrypt_with_dek: bool, // S-DEK pre-encrypt before wrapping\\\n}\\\n\\\npub struct RawApdu {            // non-SM commands (post-perso)\\\n    pub cla: u8, pub ins: u8, pub p1: u8, pub p2: u8, pub data: Vec<u8>,\\\n}\\\n```\\\n\\\n`ProvisioningInput` now carries `personalization_items: Vec<PersonalizationItem>` + `post_perso_commands: Vec<RawApdu>` instead of the old `payload_apdus` + `lock_isd`.\\\n\\\n### DGI Encoding Details\\\n\\\n**Wire format:** `TAG(2) || length || data`\\\n- Length `< 255`: 1-byte direct\\\n- Length `≥ 255`: `0xFF` + 2-byte big-endian\\\n\\\n**Max block data:** 245 bytes (measured from the reference script: blocks split at exactly 245B = 255-byte short APDU limit − 8-byte MAC − 2-byte slack).\\\n\\\n**P1/P2 protocol:**\\\n- P1 = `0x00` for all data blocks, `0x80` for the lifecycle-transition block (last)\\\n- P2 = global incrementing counter across all DGIs and split blocks\\\n\\\n---\\\n\\\n## Fixed in Initial Review Pass (2026-06-15)\\\n\\\n### B1 — Audit timestamp was not ISO-8601 ✅ FIXED (HIGH)\\\n**Was:** `now_iso8601()` returned `@<unix_seconds>` instead of ISO-8601.\\\n\\\n**Fix:** Added `time` crate (`formatting` + `macros`). Now formats as `[year]-[month]-[day]T[hour]:[minute]:[second]Z`.\\\n\\\n---\\\n\\\n### B5 — Applet host-cryptogram compare not constant-time ✅ FIXED (LOW)\\\n**Fix:** Changed `!=` to `kf_crypto::ct_eq`.\\\n\\\n---\\\n\\\n### B6 — `Provisioner::run` discarded the typed error ✅ FIXED (MED)\\\n**Fix:** `run()` now returns `(AuditRecord, Option<ProvisionError>)`.\\\n\\\n---\\\n\\\n### B7 — Host-challenge RNG failure panicked ✅ FIXED (LOW)\\\n**Fix:** `generate_host_challenge()` returns `Result` with `ProvisionError::Entropy`.\\\n\\\n---\\\n\\\n### B8–B10 — Test/code hygiene ✅ FIXED (LOW)\\\n- Removed tautological `assert_eq!(x, x)`.\\\n- Cleaned stale comment on `From<ComputeCmacError>`.\\\n- Consolidated `async-trait` to workspace deps; removed duplicate `serde_json`.\\\n\\\n---\\\n\\\n## Resolved by JCShell Rework\\\n\\\n### B3 — `lock_isd()` P1/P2 need confirming ✅ RESOLVED\\\n**Was:** The old code used `SET STATUS` (INS=0xF0, P1=0x0F P2=0x00) to lock the ISD. The P1/P2 values needed confirmation.\\\n\\\n**Resolution:** The real provisioning flow does **not use SET STATUS at all**. Instead, the lifecycle transition is triggered by the final STORE DATA block with `P1=0x80`. The `lock_isd()` builder has been replaced with `lifecycle_transition(p2)`. The simulated applet now uses a `Lifecycle` enum (`Unpersonalized` → `Factory`) instead of a boolean `locked` flag.\\\n\\\n---\\\n\\\n### B11 — Applet encryption-counter desync on empty-body commands ✅ MOOT\\\n**Was:** The mock derived the decryption counter from `received_payloads.len() + 1`, which could diverge from the host on empty-body commands.\\\n\\\n**Resolution:** With C-MAC only security level (no C-DEC), there is **no data encryption** on STORE DATA commands. The applet no longer attempts decryption unless `c_dec_active` is true (set from EXTERNAL AUTHENTICATE P1). The desync path is unreachable in the real flow.\\\n\\\n---\\\n\\\n## Still Deferred\\\n\\\n### B2 — R-ENC IV uses non-standard `0x80` marker ⏳ DEFERRED (MED)\\\n**Location:** `kf-scp03/src/channel.rs`\\\n\\\n**Issue:** For response encryption (R-ENC), the IV is derived then mutated `iv[0] |= 0x80`. Per GP SCP03 (Amendment D §6.2), C-DEC and R-ENC IVs are computed identically. This path is only reached under `SecurityLevel::FULL` (R-ENC), which the pipeline never negotiates (default is now `C_MAC` only).\\\n\\\n**Action:** Drop the `0x80` toggle and verify against a real R-ENC vector when a real-card capture is available.\\\n\\\n---\\\n\\\n### B4 — Test vector is self-derived, not vendor-blessed ⏳ DEFERRED (MED)\\\n**Location:** `kf-scp03/src/vectors.rs`\\\n\\\n**Issue:** Session keys/cryptograms in `TEST_VECTOR` are computed by the same code under test. Tests prove internal consistency, not conformance to an authoritative GP vector.\\\n\\\n**Action:** Swap in official GlobalPlatform \\\"SCP03 Test Vectors\\\" literals when available. Also de-risked by running against a real JCOP card with the known static keys from the JCShell script.\\\n\\\n---\\\n\\\n## Open Questions from JCShell Analysis\\\n\\\n1. **ICA/Root certificates (DGI A004/A005)** — Present in the JCShell script but NOT in the per-fob container directory. Where do these come from? Likely factory-wide constants (same for all fobs) or delivered by the DLL/SO content provider.\\\n2. **SPID derivation** — The directory name encodes `<FESN>-<SPID_HEX>`. The SPID hash is NOT a SHA-1 of the FESN (verified: `SHA1(\\\"1KM0001E\\\") ≠ \\\"59918C...\\\"`). It must come from an external source (KLMS or the container delivery system).\\\n3. **Key Version 255** — The reference script uses KVN=255 (0xFF). Production fobs may use a different value. Confirm with KLMS.\\\n4. **Container delivery mechanism** — Are containers delivered per-fob via network, or batch via USB/media? Affects the `FileContainerSource` directory scan logic.\\\n\\\n---\\\n\\\n*Created 2026-06-15. Updated 2026-06-15 rev 2 with JCShell-driven rework findings.*\"]],\"start1\":0,\"start2\":0,\"length1\":0,\"length2\":7165}]"
metadata_diff: {"new":{"id":"1afe8b0b762043189ace149c4fc388b7","parent_id":"283dd54f183a4ce69366648f091336d1","latitude":"0.00000000","longitude":"0.00000000","altitude":"0.0000","author":"","source_url":"","is_todo":0,"todo_due":0,"todo_completed":0,"source":"joplin-desktop","source_application":"net.cozic.joplin-desktop","application_data":"","order":1781521709944,"markup_language":1,"is_shared":0,"share_id":"","conflict_original_id":"","master_key_id":"","user_data":"","deleted_time":0},"deleted":[]}
encryption_cipher_text: 
encryption_applied: 0
updated_time: 2026-06-15T14:36:56.737Z
created_time: 2026-06-15T14:36:56.737Z
type_: 13