tests: revise comments — remove debug-finding language and issue tracking

Drop 'Regression: Issue #N' / 'Bug #N' numbering, 'dead while loop',
'the old code', 'This catches the bug where' and similar changelog-style
language from test comments. Keep concise descriptions of what each test
validates. Also simplify data-loader test headers and the verify
interrupt comment.
This commit is contained in:
Leonardo Francisco 2026-04-12 22:51:49 -04:00 committed by lewohart
parent 812b534653
commit 78fcfd3cd4
2 changed files with 55 additions and 94 deletions

View file

@ -98,10 +98,9 @@ verify_interrupt_cb (FpiUsbTransfer *transfer,
int_type, transfer->actual_length);
}
/* During match wait, type 3 = match found, anything else = no match.
* python-validity treats any interrupt type != 3 as "finger not recognized"
* and skips cmd 0x60 (get_match_result), going straight to cleanup.
* The sensor only sends one interrupt, so waiting for another hangs. */
/* Match result: type 3 = match found, anything else = no match.
* On no-match, skip cmd 0x60 (get_match_result) and go to cleanup;
* the sensor only sends one interrupt per match attempt. */
if (fpi_ssm_get_cur_state (ssm) == VERIFY_WAIT_MATCH_INT)
{
if (int_type == 3)

View file

@ -1377,7 +1377,7 @@ test_reboot_cmd_format (void)
}
/* ================================================================
* Regression: fwext_file_clear on already-cleared struct
* fwext_file_clear on already-cleared struct
*
* Double-free guard.
* ================================================================ */
@ -2149,9 +2149,8 @@ build_match_payload (guint32 user_dbid,
/* ================================================================
* R1: parse_match_result with valid TLV data
*
* Regression: Issue #1 dead while loop would never extract fields.
* Verifies that user_dbid, subtype, and hash are correctly parsed
* from a TLV dictionary matching python-validity's parse_dict() format.
* from a TLV dictionary.
* ================================================================ */
static void
test_parse_match_result_valid (void)
@ -2176,11 +2175,10 @@ test_parse_match_result_valid (void)
}
/* ================================================================
* R1b: parse_match_result iterates ALL TLV entries
* R1b: parse_match_result iterates all TLV entries
*
* Regression: The dead while loop would break after first entry.
* Build a dict with tag 3 (subtype) BEFORE tag 1 (user_dbid) to
* ensure the parser doesn't stop after the first entry.
* Tags appear in non-sequential order (tag 3 before tag 1) to
* verify the parser processes every entry, not just the first.
* ================================================================ */
static void
test_parse_match_result_multi_tags (void)
@ -2223,7 +2221,7 @@ test_parse_match_result_multi_tags (void)
/* ================================================================
* R1c: parse_match_result with empty dict (no match)
*
* Regression: A no-match scenario should return ok=TRUE but matched=FALSE.
* A no-match scenario should return ok=TRUE but matched=FALSE.
* ================================================================ */
static void
test_parse_match_result_empty (void)
@ -2304,11 +2302,10 @@ test_parse_match_result_unknown_tags (void)
}
/* ================================================================
* R2: validity_db_build_identity rejects NULL
* R2: validity_db_build_identity rejects NULL user_id
*
* Regression: Issue #2 NULL user_id was passed to build_identity
* which would then be passed to g_variant_new_string(NULL) crash.
* The guard should return NULL.
* A NULL user_id must return NULL instead of crashing in
* g_variant_new_string().
* ================================================================ */
static void
test_build_identity_null (void)
@ -2331,8 +2328,8 @@ test_build_identity_null (void)
/* ================================================================
* R2b: validity_db_build_identity with valid UUID
*
* Regression: Ensures UUID identity bytes works correctly
* (complementary to the NULL test above).
* UUID identity bytes round-trip (complementary to the NULL
* test above).
* ================================================================ */
static void
test_build_identity_valid_uuid (void)
@ -2357,8 +2354,8 @@ test_build_identity_valid_uuid (void)
/* ================================================================
* R3: Gallery matching by subtype
*
* Regression: Issue #3 identify always returned first gallery print
* regardless of actual subtype. Now it should match by finger subtype.
* Identify must match by finger subtype, not just return the
* first gallery entry.
* ================================================================ */
static void
test_gallery_match_by_subtype (void)
@ -2432,10 +2429,9 @@ test_gallery_match_empty (void)
}
/* ================================================================
* R4: enroll_user_dbid field exists separately from delete_storage_dbid
* R4: enroll_user_dbid and delete_storage_dbid are separate fields
*
* Regression: Issue #4 delete_storage_dbid was abused for enrollment.
* This compile-time test verifies both fields exist independently.
* Both fields must exist independently in the device struct.
* ================================================================ */
static void
test_struct_separate_fields (void)
@ -2458,10 +2454,7 @@ test_struct_separate_fields (void)
/* ================================================================
* R5: del_record command format
*
* Regression: Issue #5 delete SSM never sent del_record cmd.
* Verify cmd 0x48 produces correct format: 0x48 | dbid(2LE).
* (This already exists in test-validity-db.c but we double-check
* the format critical for delete functionality.)
* ================================================================ */
static void
test_del_record_format (void)
@ -2476,10 +2469,9 @@ test_del_record_format (void)
}
/* ================================================================
* R6: match_finger command is exactly 13 bytes (single allocation)
* R6: match_finger command is exactly 13 bytes
*
* Regression: Issue #6 build_cmd_match_finger allocated 12 bytes,
* freed, then re-allocated 13 bytes. Now single allocation.
* build_cmd_match_finger must produce a single 13-byte buffer.
* ================================================================ */
static void
test_match_finger_size (void)
@ -2504,8 +2496,7 @@ test_match_finger_size (void)
/* ================================================================
* R7: Clear storage SSM states exist
*
* Regression: Issue #7 clear_storage was a stub returning NOT_SUPPORTED.
* Verify the CLEAR_* enum states exist (compile-time regression test).
* CLEAR_* enum states must be defined for clear_storage to work.
* ================================================================ */
static void
test_clear_storage_states_exist (void)
@ -2968,12 +2959,10 @@ test_unwrap_invalid (void)
}
/* ================================================================
* Regression: Bug #1 Flash parse requires PSK for private key
* Flash parse requires PSK before private key decryption
*
* Private key block (ID 4) is encrypted with PSK. Calling parse_flash
* without first deriving PSK must fail (HMAC mismatch), proving the
* ordering dependency. This catches the bug where flash_read SSM
* parsed flash data BEFORE PSK derivation had occurred.
* Private key block (ID 4) is encrypted with PSK. parse_flash
* without prior PSK derivation must fail (HMAC mismatch).
* ================================================================ */
static void
test_flash_parse_needs_psk (void)
@ -2996,11 +2985,11 @@ test_flash_parse_needs_psk (void)
* We use a minimal cert (just 16 bytes of dummy data) and a privkey block
* that's encrypted with the proper PSK. */
/* Step 1: Build a cert body */
/* Build a cert body */
guint8 cert_body[16];
memset (cert_body, 0xAA, sizeof (cert_body));
/* Step 2: Build a private-key body encrypted with PSK */
/* Build a private-key body encrypted with PSK */
guint8 priv_plaintext[96]; /* d(32) + pad for block alignment */
memset (priv_plaintext, 0xBB, sizeof (priv_plaintext));
@ -3085,12 +3074,10 @@ test_flash_parse_needs_psk (void)
}
/* ================================================================
* Regression: Bug #2 READ_FLASH command format
* READ_FLASH command format
*
* The READ_FLASH command must be exactly 13 bytes matching
* python-validity: pack('<BBBHLL', 0x40, partition, 1, 0, addr, size).
* Old code only sent 10 bytes, missing the access flag and reserved
* field. This test verifies the command constant and expected layout.
* Must be exactly 13 bytes: pack('<BBBHLL', 0x40, partition, 1, 0,
* addr, size) including the access flag and reserved field.
* ================================================================ */
static void
test_flash_cmd_format (void)
@ -3107,13 +3094,13 @@ test_flash_cmd_format (void)
FP_WRITE_UINT32_LE (&cmd[5], 0x0000); /* offset */
FP_WRITE_UINT32_LE (&cmd[9], 0x1000); /* size */
/* Verify total size is 13 (not 10 like the old bug) */
/* Verify total size is 13 */
g_assert_cmpuint (sizeof (cmd), ==, 13);
/* Verify byte layout matches python-validity's pack('<BBBHLL', ...) */
g_assert_cmpint (cmd[0], ==, 0x40); /* command */
g_assert_cmpint (cmd[1], ==, 0x01); /* partition */
g_assert_cmpint (cmd[2], ==, 0x01); /* access flag (was missing) */
g_assert_cmpint (cmd[2], ==, 0x01); /* access flag */
g_assert_cmpint (cmd[3], ==, 0x00); /* reserved lo */
g_assert_cmpint (cmd[4], ==, 0x00); /* reserved hi */
/* offset at bytes 5-8 (LE uint32 = 0) */
@ -3129,17 +3116,11 @@ test_flash_cmd_format (void)
}
/* ================================================================
* Regression: Bug #3 Flash response has 6-byte header
* Flash response has 6-byte header before block data
*
* After vcsfw_cmd_send strips the 2-byte VCSFW status, the flash
* response still contains a 6-byte header: [size:4 LE][unknown:2].
* Actual flash data starts at offset 6. Old code passed the raw
* response directly to parse_flash(), corrupting the block parsing.
* This test verifies:
* 1) The 6-byte header is correctly structured (size field matches)
* 2) parse_flash works on correctly unwrapped data (offset +6)
* 3) The raw response differs from the unwrapped payload, proving
* that skipping the header is necessary
* After stripping the 2-byte VCSFW status, the flash response
* contains [size:4 LE][unknown:2] before the actual block data.
* parse_flash must receive data starting at offset 6.
* ================================================================ */
static void
test_flash_response_header (void)
@ -3176,26 +3157,18 @@ test_flash_response_header (void)
g_clear_error (&error);
validity_tls_free (&tls);
/* Verify the bug scenario: passing the raw response (with the 6-byte
* header) gives DIFFERENT data to the parser than the correctly unwrapped
* payload. The first 4 bytes of the raw response are the LE size field
* (0x04 0x00 0x00 0x00), which would be misinterpreted as block_id=0x0004
* (PRIVKEY block with size 0). This is a data corruption the parser
* receives wrong input either way, but the key point is that the raw
* response and the unwrapped payload are NOT the same buffer content. */
/* The raw response (with 6-byte header) differs from the unwrapped
* payload passing it directly to parse_flash would corrupt block
* parsing since the size field (0x04 0x00 ...) looks like block_id=4. */
g_assert_cmpuint (sizeof (response), !=, payload_len);
g_assert_true (memcmp (response, payload, payload_len) != 0);
}
/* ================================================================
* Regression: Bug #4 TLS handshake expects raw TLS records
* TLS handshake uses raw TLS records (not VCSFW-wrapped)
*
* parse_server_hello expects raw TLS records starting with a content
* type byte (0x16 for Handshake). The old code used vcsfw_cmd_send
* which strips 2 bytes of VCSFW status, corrupting the TLS record.
* This test verifies that:
* - A valid TLS Handshake record header is accepted
* - Data prefixed with a 2-byte VCSFW status is rejected
* parse_server_hello expects data starting with content type 0x16.
* A 2-byte VCSFW status prefix would corrupt the TLS record.
* ================================================================ */
static void
test_server_hello_rejects_vcsfw_prefix (void)
@ -3279,12 +3252,10 @@ test_server_hello_rejects_vcsfw_prefix (void)
}
/* ================================================================
* Regression: Bug #5 Client hello has 0x44 prefix (not VCSFW cmd)
* Client hello uses 0x44 prefix (not a VCSFW command)
*
* TLS handshake messages use 0x44000000 as a 4-byte prefix, NOT a
* standard VCSFW command byte. This test verifies the prefix and that
* the TLS record immediately follows (no VCSFW status expected in
* response).
* TLS handshake messages are prefixed with 0x44000000, followed
* by the raw TLS record. No VCSFW status in the response.
* ================================================================ */
static void
test_client_hello_tls_prefix (void)
@ -5195,10 +5166,7 @@ test_data_load_common_valid (void)
}
/* ================================================================
* T8.13: Enroll db_write_enable returns NULL with no loaded data
*
* Verifies that the enroll/verify path fails gracefully when the
* runtime data files haven't been loaded (e.g., package not installed).
* T8.13: db_write_enable returns NULL without loaded data
* ================================================================ */
static void
test_data_enroll_dbe_missing (void)
@ -5232,10 +5200,7 @@ test_data_enroll_dbe_missing (void)
}
/* ================================================================
* T8.14: Enroll db_write_enable returns data when store populated
*
* Verifies that after loading data files, the consumers can
* access the blob through the data store accessors.
* T8.14: db_write_enable returns data when store is populated
* ================================================================ */
static void
test_data_enroll_dbe_loaded (void)
@ -5295,10 +5260,7 @@ test_data_enroll_dbe_loaded (void)
}
/* ================================================================
* T8.15: load_device corrupt init.bin fails with error
*
* Verifies that a corrupted data file in the device directory
* is detected by HMAC verification and the load is rejected.
* T8.15: load_device rejects corrupt HMAC
* ================================================================ */
static void
test_data_load_device_corrupt (void)
@ -5526,7 +5488,7 @@ main (int argc, char *argv[])
/* VERIFY tests */
/* R1: parse_match_result regression tests (Issue #1: dead while loop) */
/* R1: parse_match_result */
g_test_add_func ("/validity/verify/parse_match_result_valid",
test_parse_match_result_valid);
g_test_add_func ("/validity/verify/parse_match_result_multi_tags",
@ -5540,13 +5502,13 @@ main (int argc, char *argv[])
g_test_add_func ("/validity/verify/match_result_clear",
test_match_result_clear);
/* R2: identity builder NULL regression (Issue #2: NULL crash) */
/* R2: identity builder */
g_test_add_func ("/validity/verify/build_identity_null",
test_build_identity_null);
g_test_add_func ("/validity/verify/build_identity_valid_uuid",
test_build_identity_valid_uuid);
/* R3: gallery matching by subtype (Issue #3: always returned first) */
/* R3: gallery matching */
g_test_add_func ("/validity/verify/gallery_match_by_subtype",
test_gallery_match_by_subtype);
g_test_add_func ("/validity/verify/gallery_match_fallback",
@ -5554,19 +5516,19 @@ main (int argc, char *argv[])
g_test_add_func ("/validity/verify/gallery_match_empty",
test_gallery_match_empty);
/* R4: struct field separation (Issue #4: field abuse) */
/* R4: struct field separation */
g_test_add_func ("/validity/verify/struct_separate_fields",
test_struct_separate_fields);
/* R5: del_record command format (Issue #5: delete SSM non-functional) */
/* R5: del_record command format */
g_test_add_func ("/validity/verify/del_record_format",
test_del_record_format);
/* R6: match_finger single allocation (Issue #6: double alloc) */
/* R6: match_finger allocation */
g_test_add_func ("/validity/verify/match_finger_size",
test_match_finger_size);
/* R7: clear/delete storage SSM states (Issue #7: stub) */
/* R7: clear/delete storage SSM states */
g_test_add_func ("/validity/verify/clear_storage_states",
test_clear_storage_states_exist);
g_test_add_func ("/validity/verify/delete_states",
@ -5592,7 +5554,7 @@ main (int argc, char *argv[])
g_test_add_func ("/validity/tls/client-hello", test_build_client_hello);
g_test_add_func ("/validity/tls/unwrap/invalid", test_unwrap_invalid);
/* Regression tests for hardware-discovered bugs */
/* TLS regression tests */
g_test_add_func ("/validity/tls/regression/flash-parse-needs-psk",
test_flash_parse_needs_psk);
g_test_add_func ("/validity/tls/regression/flash-cmd-format",