Add PIN prefix anti-phishing verification

[wksantiago]

Summary

• Derives two BIP-39 words from PIN prefix + device secret using HMAC-SHA256
• Allows users to verify device authenticity before entering full PIN
• Includes compile-time wordlist validation and secret length bounds

Test plan

• All 19 unit tests pass
• Determinism, uniqueness, and bounds checking verified

Summary by CodeRabbit

• New Features

  • Added anti-phishing PIN-prefix feature that derives BIP39 words from a PIN and device secret.
  • Embedded full BIP39 English wordlist for on-device word lookup.

• Tests

  • Added comprehensive native tests covering derivation, determinism, edge cases, state handling, and buffer validation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds a PIN-prefix anti‑phishing feature: BIP39 wordlist, pin-prefix API (derive/get words, manage digits), HMAC‑SHA256 derivation implementation, unit tests, and build/test CMake changes.

Changes

Cohort / File(s)	Summary
Core PIN Prefix Module main/pin_prefix.h, main/pin_prefix.c	New public API and implementation for PIN-prefix state and word derivation: pin_prefix_t, pin_prefix_derive_words, pin_prefix_get_words, pin_prefix_set_digit, pin_prefix_clear, pin_prefix_is_ready. Validates inputs, performs HMAC-SHA256, extracts two 11-bit indices, and zeroes sensitive buffers.
BIP39 Wordlist main/bip39_wordlist.h	Adds static BIP39_WORDLIST[2048] header with 2048 lowercase words and include guards; used for index→word lookups.
Build Configuration main/CMakeLists.txt, test/native/CMakeLists.txt	Adds pin_prefix.c to main build and a test_pin_prefix native test executable; test CMake optionally enables OpenSSL and defines HAS_OPENSSL.
Test Infrastructure & Mocks test/native/test_pin_prefix.c, test/native/mocks/mbedtls/md.h	Adds comprehensive unit tests for determinism, edge cases, buffer handling, and distribution; provides HMAC implementation for tests with OpenSSL path (when available) and standalone SHA‑256 HMAC fallback.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant PinMod as pin_prefix module
    participant HMAC as HMAC-SHA256
    participant WL as BIP39 wordlist

    Caller->>PinMod: pin_prefix_derive_words(prefix, secret)
    PinMod->>HMAC: HMAC-SHA256(context || digits, key=secret)
    HMAC-->>PinMod: 32-byte digest
    PinMod->>PinMod: extract two 11-bit indices
    PinMod-->>Caller: word1_index, word2_index

    Caller->>PinMod: pin_prefix_get_words(prefix, secret)
    PinMod->>PinMod: pin_prefix_derive_words(...)
    PinMod->>WL: bip39_get_word(index1)
    WL-->>PinMod: word1_string
    PinMod->>WL: bip39_get_word(index2)
    WL-->>PinMod: word2_string
    PinMod-->>Caller: word1, word2

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• Add PIN prefix anti-phishing verification #86: Implements the same PIN-prefix anti‑phishing feature (pin_prefix_t, HMAC-based derivation of two 11-bit BIP39 indices and BIP39 wordlist).

Suggested reviewers

• kwsantiago

Poem

🐰 I hop on code paths, digits in a line,
HMAC hums secrets into words that shine,
Two tiny words to guard a PIN's gate,
I nibble bugs and celebrate! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'Add PIN prefix anti-phishing verification' accurately describes the main change: introducing PIN prefix functionality for anti-phishing verification with BIP39 word derivation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In `@main/bip39_wordlist.h`:
- Around line 1-8: The file fails clang-format; reformat main/bip39_wordlist.h
to match project style by running the repository's clang-format configuration
(or apply the same formatting rules) so the BIP39_WORDLIST array and header
guards follow the project's indentation and spacing conventions; update spacing
around preprocessor lines (`#ifndef`, `#define`, `#endif`), the static const
declaration, and the array entries (BIP39_WORDLIST) so the file passes CI
clang-format checks.

In `@main/pin_prefix.c`:
- Around line 19-49: In pin_prefix_derive_words, check the return value of
mbedtls_md_info_from_type(MBEDTLS_MD_SHA256) before calling mbedtls_md_hmac; if
it returns NULL, securely zero input and hmac_out (using secure_memzero) and
return an error (e.g., -1) to avoid dereferencing NULL; then use the non-NULL
md_info when calling mbedtls_md_hmac and proceed as before. Ensure you reference
mbedtls_md_info_from_type, mbedtls_md_hmac, input, hmac_out, and secure_memzero
in the fix so the cleanup always runs on the error path.

In `@test/native/mocks/mbedtls/md.h`:
- Around line 147-163: The early return on ilen > 1024 leaves sensitive stacks
(k_ipad, k_opad, and tk) uncleared; before returning -1 insert explicit
zeroization (e.g. memset(k_ipad, 0, sizeof k_ipad); memset(k_opad, 0, sizeof
k_opad); memset(tk, 0, sizeof tk);) so key material is wiped from the stack,
taking care to zero tk even if keylen was not reduced to 32.

In `@test/native/test_pin_prefix.c`:
- Around line 1-20: Reformat test/native/test_pin_prefix.c to satisfy
clang-format: run the repository's clang-format configuration against the file
(or apply your editor's format command) so spacing, include ordering,
indentation, and macro alignment conform; ensure includes ("crypto_asm.h",
"pin_prefix.h", and the inlined "pin_prefix.c"), macro definitions
TEST/PASS/FAIL, and the TEST_SECRET array are reformatted consistently with the
project's style. Commit the reformatted file so CI passes.
- Around line 8-11: The test currently includes the implementation file
"pin_prefix.c" directly, which breaks build boundaries and can cause duplicate
symbols; remove the '#include "pin_prefix.c"' line from
test/native/test_pin_prefix.c and instead add pin_prefix.c to the test's build
target in the CMakeLists (e.g., add it to the test executable's sources via
target_sources or add_executable) so the implementation is compiled and linked
normally alongside the test; keep the existing includes for pin_prefix.h and
crypto_asm.h unchanged.

🧹 Nitpick comments (4)

main/bip39_wordlist.h (1)

7-7: Avoid multiple wordlist copies across translation units.
Keeping a 2048-entry static array in a header can emit a full copy per TU that includes it. Consider declaring it extern here and moving the definition to a single .c file to avoid flash bloat on embedded targets.

♻️ Suggested header change

-static const char *const BIP39_WORDLIST[2048] = {
+extern const char *const BIP39_WORDLIST[2048];

main/pin_prefix.h (1)

11-16: Clarify buffer sizing for BIP39 words.
BIP39_MAX_WORD_LEN doesn’t indicate whether the NUL terminator is included. Callers who allocate exactly 8 bytes will fail pin_prefix_get_words. Consider an explicit buffer-size constant (or a brief comment) to make the expectation unambiguous.

♻️ Suggested API clarification

-#define BIP39_MAX_WORD_LEN    8
+#define BIP39_MAX_WORD_LEN    8
+#define BIP39_MAX_WORD_BUFSIZE (BIP39_MAX_WORD_LEN + 1)

main/pin_prefix.c (2)

14-17: Make CONTEXT_LEN derive from the string literal.

Avoid manual sync risk if the context string changes.

♻️ Proposed refactor

-#define CONTEXT_LEN           13
+#define CONTEXT_LEN           (sizeof(ANTI_PHISHING_CONTEXT) - 1)

---

58-84: Clear outputs on early failures to avoid stale words.

This prevents callers from accidentally reusing old word buffers after a failed derivation.

♻️ Proposed tweak

     if (!word1 || !word2 || word1_size == 0 || word2_size == 0)
         return -1;
+
+    word1[0] = '\0';
+    word2[0] = '\0';