feat: FaceMatch Android app-id structural parse

.gitignore

@@ -20,5 +20,4 @@ src/solidity/snapshots
 output-fixtures
 settings.json
 
-src/noir/lib/facematch/src/android/tests.nr
 src/noir/bin/facematch/android/**/src/tests.nr

src/noir/lib/facematch/src/android/mod.nr

@@ -4,79 +4,127 @@ use sha256;
 use sha512::sha384;
 use sig_check_ecdsa::{verify_nist_p256, verify_nist_p384};
 use utils::{
-    check_zero_padding, find_subarray_index, get_array_slice, poseidon2_hash_packed,
-    unsafe_get_asn1_element_length,
+    check_zero_padding, find_subarray_index, find_subarray_index_strict, get_array_slice,
+    poseidon2_hash_packed, read_der_length, unsafe_get_asn1_element_length,
 };
 
 pub mod constants;
 pub mod token;
 use constants::{ANDROID_KEY_ORIGIN_GENERATED, OID_ANDROID_KEY_ATTESTATION};
-//pub mod tests;
+pub mod tests;
+
+/// Advance from a TLV's tag octet (at `pos`) to the start of its content (descend into
+/// a constructed element).
+fn enter_element<let N: u32>(tbs: [u8; N], pos: u32) -> u32 {
+    let (_, consumed) = read_der_length(tbs, pos + 1);
+    pos + 1 + consumed
+}
+
+/// Advance from a TLV's tag octet (at `pos`) past the whole element (skip it entirely).
+/// Assumes a single-byte tag.
+fn skip_element<let N: u32>(tbs: [u8; N], pos: u32) -> u32 {
+    let (len, consumed) = read_der_length(tbs, pos + 1);
+    pos + 1 + consumed + len
+}
+
+/// Number of identifier (tag) octets at `pos`: 1 for the low-tag form, or 1 + the
+/// base-128 octets of the high-tag form (low 5 bits == 0x1F) that authorization entries
+/// use (e.g. attestationApplicationId [709] = BF 85 45).
+fn tag_length<let N: u32>(tbs: [u8; N], pos: u32) -> u32 {
+    let mut len: u32 = 1;
+    if (tbs[pos] & 0x1F) == 0x1F {
+        let mut done: bool = false;
+        for i in 0..4 {
+            if !done {
+                len += 1;
+                if (tbs[pos + 1 + i] & 0x80) == 0 {
+                    done = true;
+                }
+            }
+        }
+    }
+    len
+}
+
+/// Advance past a whole TLV whose tag may be multi-byte (a context-specific
+/// authorization entry): skip the identifier octets, then the length field and content.
+fn skip_tagged_element<let N: u32>(tbs: [u8; N], pos: u32) -> u32 {
+    let tl: u32 = tag_length(tbs, pos);
+    let (len, consumed) = read_der_length(tbs, pos + tl);
+    pos + tl + consumed + len
+}
+
+/// Assert the single-byte-tag TLV at `pos` carries `tag`, then descend into its content.
+fn expect_enter<let N: u32>(tbs: [u8; N], pos: u32, tag: u8) -> u32 {
+    assert(tbs[pos] == tag, "unexpected DER tag while walking credential TBS");
+    enter_element(tbs, pos)
+}
 
 pub fn get_app_id_from_credential_tbs<let TBS_MAX_LEN: u32, let APP_ID_MAX_LEN: u32>(
     tbs: [u8; TBS_MAX_LEN],
 ) -> ([u8; APP_ID_MAX_LEN], u32) {
-    // Find Android Key Attestation extension OID in the TBS certificate
-    let oid_offset = find_subarray_index(OID_ANDROID_KEY_ATTESTATION, tbs);
+    // Locate the Android Key Attestation extension OID. Strict first-occurrence so the
+    // anchor cannot be pointed at a planted copy inside attacker-controlled bytes.
+    let oid_offset = find_subarray_index_strict(OID_ANDROID_KEY_ATTESTATION, tbs);
     assert(
         oid_offset < TBS_MAX_LEN,
         "Android Key Attestation OID not found in credential certificate",
     );
 
-    // Look for the 6th item in the sequence (tagged with BF 85 45)
-    // The 6th item contains the app ID as an OCTET STRING
-    let APP_ID_SEARCH_DISTANCE = 200; // Search within 200 bytes after the OID
-
-    let mut attestation_app_id_sequence_offset = TBS_MAX_LEN; // Default to not found
-    let mut found_attestation_app_id_sequence = false;
-    // Search for the attestation app id sequence tag pattern (BF 85 45) within the extension
-    // attestationApplicationId [709] EXPLICIT OCTET STRING
-    for i in 0..APP_ID_SEARCH_DISTANCE {
-        if (oid_offset + OID_ANDROID_KEY_ATTESTATION.len() + i + 2 < TBS_MAX_LEN)
-            & !found_attestation_app_id_sequence {
-            if (tbs[oid_offset + OID_ANDROID_KEY_ATTESTATION.len() + i] == 0xbf)
-                & (tbs[oid_offset + OID_ANDROID_KEY_ATTESTATION.len() + i + 1] == 0x85)
-                & (tbs[oid_offset + OID_ANDROID_KEY_ATTESTATION.len() + i + 2] == 0x45) {
-                attestation_app_id_sequence_offset =
-                    oid_offset + OID_ANDROID_KEY_ATTESTATION.len() + i + 4; // Skip the tag and length
-                found_attestation_app_id_sequence = true;
-            }
-        }
+    // Walk the KeyDescription structurally to confine the app-id search to the
+    // CA-attested `softwareEnforced` set: the attacker-chosen `attestationChallenge`
+    // (field 5) precedes it, so a flat scan could match a planted BF 85 45 blob there.
+    // Framing: 04 extension OCTET STRING -> 30 KeyDescription -> 6 fixed fields -> 30
+    // softwareEnforced.
+    let mut p: u32 = oid_offset + OID_ANDROID_KEY_ATTESTATION.len();
+    p = expect_enter(tbs, p, 0x04); // extension value OCTET STRING
+    p = expect_enter(tbs, p, 0x30); // KeyDescription SEQUENCE
+    // Skip the six fixed fields by length, stepping over attestationChallenge.
+    for _i in 0..6 {
+        p = skip_element(tbs, p);
     }
-    assert(
-        found_attestation_app_id_sequence,
-        "Attestation app id sequence not found in app ID extension",
-    );
-    assert(
-        attestation_app_id_sequence_offset < TBS_MAX_LEN,
-        "Attestation app id sequence offset out of bounds",
-    );
+    // Enter softwareEnforced and bound the app-id search to its contents only.
+    assert(tbs[p] == 0x30, "softwareEnforced SEQUENCE not found");
+    let (sw_len, sw_consumed) = read_der_length(tbs, p + 1);
+    let sw_start = p + 1 + sw_consumed;
+    let sw_end = sw_start + sw_len;
+    assert(sw_end <= TBS_MAX_LEN, "softwareEnforced exceeds certificate bounds");
 
-    // Look for OCTET STRING tag (0x04) and read the length dynamically
-    let mut app_id_offset = TBS_MAX_LEN; // Default to not found
-    let mut found_app_id = false;
-    let mut app_id_length: u32 = 0;
-    let OCTET_STRING_SEARCH_DISTANCE = 10;
-
-    // Search for a SET (0x31) tag followed by a SEQUENCE (0x30) tag, followed by an OCTET STRING (0x04) tag
-    // and read the length byte that follows
-    for i in 0..OCTET_STRING_SEARCH_DISTANCE {
-        if (attestation_app_id_sequence_offset + i + 1 < TBS_MAX_LEN) & !found_app_id {
-            if (tbs[attestation_app_id_sequence_offset + i] == 0x31)
-                // Skip the byte encoding the length of the SET
-                & (tbs[attestation_app_id_sequence_offset + i + 2] == 0x30)
-                // Skip the byte encoding the length of the SEQUENCE
-                & (tbs[attestation_app_id_sequence_offset + i + 4] == 0x04) {
-                app_id_length = tbs[attestation_app_id_sequence_offset + i + 5] as u32; // Read the length byte
-                app_id_offset = attestation_app_id_sequence_offset + i + 6; // Skip the OCTET STRING tag and length
-                found_app_id = true;
+    // Walk the softwareEnforced entries and match the [709] DER *tag* (BF 85 45),
+    // skipping each non-matching entry by length. Only tag positions are inspected, so a
+    // value that merely contains those bytes cannot be mistaken for the tag. The entry
+    // cap bounds the walk (softwareEnforced holds only a handful of entries).
+    let app_id_tag: [u8; 3] = [0xBF, 0x85, 0x45];
+    let MAX_SW_ENTRIES: u32 = 32;
+    p = sw_start;
+    let mut tag_offset: u32 = TBS_MAX_LEN;
+    let mut found: bool = false;
+    for _ in 0..MAX_SW_ENTRIES {
+        if !found & (p + 3 <= sw_end) {
+            if (tbs[p] == app_id_tag[0])
+                & (tbs[p + 1] == app_id_tag[1])
+                & (tbs[p + 2] == app_id_tag[2]) {
+                tag_offset = p;
+                found = true;
+            } else {
+                p = skip_tagged_element(tbs, p);
             }
         }
     }
+    assert(found, "attestationApplicationId [709] not found in softwareEnforced");
 
-    // Ensure we found the app ID
-    assert(found_app_id, "App ID OCTET STRING not found in credential certificate");
-    assert(app_id_offset < TBS_MAX_LEN, "App ID not found in credential certificate");
+    // Descend [709] (3-byte tag, length at +3): OCTET STRING -> SEQUENCE -> SET ->
+    // SEQUENCE -> packageName OCTET STRING (the app id).
+    let (_, tag_len_bytes) = read_der_length(tbs, tag_offset + 3);
+    let mut q = tag_offset + 3 + tag_len_bytes;
+    q = expect_enter(tbs, q, 0x04); // OCTET STRING wrapper
+    q = expect_enter(tbs, q, 0x30); // AttestationApplicationId SEQUENCE
+    q = expect_enter(tbs, q, 0x31); // SET OF AttestationPackageInfo
+    q = expect_enter(tbs, q, 0x30); // AttestationPackageInfo SEQUENCE
+    assert(tbs[q] == 0x04, "packageName OCTET STRING not found");
+    let (app_id_length, app_id_len_bytes) = read_der_length(tbs, q + 1);
+    let app_id_offset = q + 1 + app_id_len_bytes;
+    assert(app_id_offset + app_id_length <= TBS_MAX_LEN, "App ID exceeds certificate bounds");
     assert(app_id_length <= APP_ID_MAX_LEN, "App ID length exceeds maximum allowed length");
 
     // Extract the app ID with dynamic length, padding with zeros if needed
@@ -90,6 +138,12 @@ pub fn get_app_id_from_credential_tbs<let TBS_MAX_LEN: u32, let APP_ID_MAX_LEN:
     (app_id, app_id_length)
 }
 
+// NOTE: unlike the hardened `get_app_id_from_credential_tbs`, this still uses non-strict
+// `find_subarray_index` + a flat forward scan over a window that includes the
+// attacker-controlled attestationChallenge. NOT currently exploitable: no circuit `main`
+// calls it (the `// TODO: Only accept GENERATED` in constants.nr is unwired). If it is
+// ever wired into an acceptance path, harden it like `get_app_id`: strict OID anchor +
+// structural walk into softwareEnforced before searching for the tag.
 pub fn get_key_origin_from_credential_tbs<let TBS_MAX_LEN: u32>(tbs: [u8; TBS_MAX_LEN]) -> u8 {
     // Find Android Key Attestation extension OID in the TBS certificate
     let oid_offset = find_subarray_index(OID_ANDROID_KEY_ATTESTATION, tbs);

src/noir/lib/facematch/src/android/tests.nr

@@ -0,0 +1,101 @@
+// Auto-generated regression tests for the Android FaceMatch app-identity parser.
+// The real credential TBS below is captured from the `android_strict_mode_dev`
+// facematch fixture; its attestationApplicationId (in softwareEnforced) is
+// "app.zkpassport.zkpassport".
+use super::get_app_id_from_credential_tbs;
+
+global REAL_CREDENTIAL_TBS: [u8; 1000] = [
+    48, 130, 2, 94, 160, 3, 2, 1, 2, 2, 1, 1, 48, 10, 6, 8, 42, 134, 72, 206, 61, 4, 3, 2, 48, 63,
+    49, 18, 48, 16, 6, 3, 85, 4, 10, 19, 9, 83, 116, 114, 111, 110, 103, 66, 111, 120, 49, 41, 48,
+    39, 6, 3, 85, 4, 3, 19, 32, 50, 49, 52, 51, 102, 57, 50, 48, 98, 56, 54, 101, 51, 52, 98, 100,
+    100, 54, 53, 99, 99, 54, 97, 101, 100, 101, 50, 48, 55, 101, 48, 102, 48, 30, 23, 13, 55, 48,
+    48, 49, 48, 49, 48, 48, 48, 48, 48, 48, 90, 23, 13, 52, 56, 48, 49, 48, 49, 48, 48, 48, 48, 48,
+    48, 90, 48, 31, 49, 29, 48, 27, 6, 3, 85, 4, 3, 19, 20, 65, 110, 100, 114, 111, 105, 100, 32,
+    75, 101, 121, 115, 116, 111, 114, 101, 32, 75, 101, 121, 48, 89, 48, 19, 6, 7, 42, 134, 72, 206,
+    61, 2, 1, 6, 8, 42, 134, 72, 206, 61, 3, 1, 7, 3, 66, 0, 4, 76, 142, 161, 132, 148, 71, 101,
+    172, 205, 118, 196, 252, 134, 68, 108, 217, 41, 187, 27, 245, 211, 127, 239, 23, 90, 158, 82,
+    131, 76, 230, 52, 132, 218, 17, 82, 102, 252, 97, 145, 180, 238, 160, 223, 173, 161, 228, 153,
+    42, 43, 235, 77, 179, 74, 99, 166, 229, 117, 224, 235, 44, 248, 2, 249, 83, 163, 130, 1, 105,
+    48, 130, 1, 101, 48, 14, 6, 3, 85, 29, 15, 1, 1, 255, 4, 4, 3, 2, 7, 128, 48, 130, 1, 81, 6, 10,
+    43, 6, 1, 4, 1, 214, 121, 2, 1, 17, 4, 130, 1, 65, 48, 130, 1, 61, 2, 2, 1, 44, 10, 1, 2, 2, 2,
+    1, 44, 10, 1, 2, 4, 36, 100, 98, 57, 48, 48, 97, 49, 97, 45, 97, 55, 50, 99, 45, 52, 55, 49, 98,
+    45, 56, 98, 54, 102, 45, 55, 53, 51, 49, 100, 48, 57, 100, 50, 55, 97, 102, 4, 0, 48, 91, 191,
+    133, 61, 8, 2, 6, 1, 153, 227, 140, 96, 168, 191, 133, 69, 75, 4, 73, 48, 71, 49, 33, 48, 31, 4,
+    25, 97, 112, 112, 46, 122, 107, 112, 97, 115, 115, 112, 111, 114, 116, 46, 122, 107, 112, 97,
+    115, 115, 112, 111, 114, 116, 2, 2, 3, 135, 49, 34, 4, 32, 250, 198, 23, 69, 220, 9, 3, 120,
+    111, 185, 237, 230, 42, 150, 43, 57, 159, 115, 72, 240, 187, 111, 137, 155, 131, 50, 102, 117,
+    145, 3, 59, 156, 48, 129, 167, 161, 8, 49, 6, 2, 1, 2, 2, 1, 3, 162, 3, 2, 1, 3, 163, 4, 2, 2,
+    1, 0, 165, 8, 49, 6, 2, 1, 4, 2, 1, 6, 170, 3, 2, 1, 1, 191, 131, 119, 2, 5, 0, 191, 133, 62, 3,
+    2, 1, 0, 191, 133, 64, 76, 48, 74, 4, 32, 0, 63, 26, 222, 157, 71, 110, 97, 43, 0, 242, 152, 62,
+    106, 215, 220, 209, 94, 106, 128, 204, 45, 187, 0, 141, 167, 214, 131, 158, 215, 58, 143, 1, 1,
+    255, 10, 1, 0, 4, 32, 127, 211, 148, 157, 174, 28, 18, 161, 19, 217, 129, 202, 208, 201, 188,
+    37, 85, 14, 231, 194, 41, 247, 184, 251, 176, 151, 50, 164, 224, 109, 74, 2, 191, 133, 65, 5, 2,
+    3, 2, 34, 224, 191, 133, 66, 5, 2, 3, 3, 22, 169, 191, 133, 78, 6, 2, 4, 1, 52, 218, 9, 191,
+    133, 79, 6, 2, 4, 1, 52, 218, 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+];
+
+// Byte offset of the attacker-controllable attestationChallenge *content* within the
+// TBS above (KeyDescription field 5, which precedes softwareEnforced).
+global CHALLENGE_CONTENT_OFFSET: u32 = 309;
+
+// A fully-formed attestationApplicationId marker (BF 85 45 ... package "spoofed"),
+// 26 bytes, small enough to fit inside the 36-byte attestationChallenge. A pre-fix
+// flat forward scan from the extension OID would encounter this before the genuine
+// tag in softwareEnforced and return "spoofed"; the structural walk skips the
+// challenge entirely and must ignore it.
+global PLANTED_SPOOF_MARKER: [u8; 26] = [
+    0xBF, 0x85, 0x45, 0x16, // attestationApplicationId [709], len 22
+    0x04, 0x14, //             OCTET STRING, len 20
+    0x30, 0x12, //             AttestationApplicationId SEQUENCE, len 18
+    0x31, 0x0E, //             packages SET, len 14
+    0x30, 0x0C, //             AttestationPackageInfo SEQUENCE, len 12
+    0x04, 0x07, 0x73, 0x70, 0x6F, 0x6F, 0x66, 0x65, 0x64, // packageName OCTET "spoofed"
+    0x02, 0x01, 0x01, //       version INTEGER 1
+    0x31, 0x00, //             signatureDigests SET, empty
+];
+
+fn expected_app_id() -> ([u8; 32], u32) {
+    let s = "app.zkpassport.zkpassport".as_bytes();
+    let mut e = [0u8; 32];
+    for i in 0..s.len() {
+        e[i] = s[i];
+    }
+    (e, s.len())
+}
+
+// Happy path: the genuine app id is read from a real attestation certificate.
+#[test]
+fn test_get_app_id_real_cert() {
+    let (app_id, len): ([u8; 32], u32) = get_app_id_from_credential_tbs(REAL_CREDENTIAL_TBS);
+    let (expected, expected_len) = expected_app_id();
+    assert(len == expected_len, "app id length mismatch");
+    assert(app_id == expected, "app id mismatch");
+}
+
+// Regression (facematch Android app-identity finding): a spoof marker planted inside
+// the attacker-controlled attestationChallenge must NOT be picked up. The structural
+// walk reads the app id from softwareEnforced, so the genuine id is returned and the
+// planted "spoofed" is ignored. Pre-fix, the flat scan returned "spoofed".
+#[test]
+fn test_get_app_id_ignores_planted_challenge() {
+    let mut tbs = REAL_CREDENTIAL_TBS;
+    for i in 0..PLANTED_SPOOF_MARKER.len() {
+        tbs[CHALLENGE_CONTENT_OFFSET + i] = PLANTED_SPOOF_MARKER[i];
+    }
+    let (app_id, len): ([u8; 32], u32) = get_app_id_from_credential_tbs(tbs);
+    let (expected, expected_len) = expected_app_id();
+    assert(len == expected_len, "planted challenge changed the app id length");
+    assert(app_id == expected, "planted challenge spoofed the app id");
+}