test fix for exponents

[x-senpai-x]

No description provided.

[Copilot]

Pull request overview

Updates the RSA exponent-related unit tests for the verify_rsa_pubkey_in_tbs helper in the Noir passport utilities, switching from minimal byte arrays to a more realistic DER-shaped TBS builder (build_test_tbs).

Changes:

• Replaces old small-buffer exponent tests with new tests built on build_test_tbs and a 256-byte modulus.
• Adjusts test inputs to match the function’s DER-structure expectations (modulus INTEGER + exponent INTEGER).
• Reorders the exponent tests to live after the build_test_tbs helper.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Bisht13]

/claude-review

[github-actions]

Code Review

Summary

This PR ("test fix for exponents") targets the v1 branch and replaces five small, structurally-incorrect unit tests in noir-examples/noir-passport/utils/data-check/tbs-pubkey/src/lib.nr with five new tests that use a realistic 256-byte modulus and a properly-constructed DER TBS (via the existing build_test_tbs helper). The old tests used a 4-byte modulus and a hand-rolled 32-byte buffer that did not reflect the actual SPKI structure expected by verify_rsa_pubkey_in_tbs. The new tests are a clear improvement in test fidelity.

Review

Correctness & Soundness

There is one consistent bug across all five new tests (and, for context, the pre-existing tests share the same pattern):

tbs_certificate_len does not cover the exponent bytes (blocking)

Every new test passes tbs_len = 271 — which equals offset + DSC_KEY_SIZE = 15 + 256, i.e., the byte immediately past the last modulus byte. However, exponent_offset = offset + 258 = 273, so the exponent value bytes (at TBS indices 273–275) lie beyond tbs_certificate_len = 271.

The function asserts pubkey_offset + DSC_KEY_SIZE <= tbs_certificate_len (line 20) to ensure the modulus is in the authenticated TBS region. But the exponent bounds are only checked against the compile-time TBS_CERT_SIZE = 300 (lines 106/115), not against tbs_certificate_len. As a result, all five positive tests pass with the exponent residing in the unauthenticated portion of the TBS buffer.

This means the tests do not confirm the intended security property — that both modulus and exponent are authenticated by the certificate signature.

Fix for all five new tests:

let tbs_len: u32 = offset + 261; // 276 — covers through the last byte of a 3-byte exponent

or simply 300u32 (the full physical TBS array length, always safe).

Note: the function itself should also be hardened with an additional assertion:

assert(exponent_offset + exponent_byte_count <= tbs_certificate_len,
       "Exponent bytes exceed signed TBS length");

This is a pre-existing gap in verify_rsa_pubkey_in_tbs rather than something introduced by this PR, but worth fixing alongside the test correction.

Code Quality

The new test code is clear and well-structured. Comments explaining the DER byte layout (tbs[offset + 257] = 0x01; // DER INTEGER length) are helpful.

Minor: in test_verify_rsa_pubkey_in_tbs_with_allowed_exponent_3, after overwriting the length and first value byte, the bytes at tbs[offset + 259] and tbs[offset + 260] still hold 0x00 and 0x01 (leftover from the default 65537 encoding). This has no effect on correctness (the function reads only 1 byte for single-byte exponents), but might confuse a future reader inspecting the raw TBS buffer.

Testing

The two negative tests (rejects_exponent_1, rejects_wrong_exponent_bytes) and three positive tests cover the main branches. All five new tests use modulus[0] = 0x90 (leading-zero DER form), meaning the is_no_leading_zero code path is not exercised by the new tests specifically. The pre-existing test_valid_pubkey_without_leading_zero covers this path with the default 65537 exponent, so it is not entirely uncovered.

Suggestions

• Fix tbs_len in all new tests to offset + 261 (or 300u32) so the exponent is within the signed region. (blocking)
• Consider adding a should_fail test for the case where exponent_offset >= tbs_certificate_len, once the function's bounds check is extended. (non-blocking, separate PR)
• Add a test for the no-leading-zero path with a non-default exponent (e.g., exponent=3). (non-blocking suggestion)

Verdict

🔄 Request changes — the tbs_len = 271 value in all new tests leaves the exponent outside the declared authenticated region, undermining the security property these tests are meant to verify. Fixing this requires a one-line change in each test.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

noir-examples/noir-passport/utils/data-check/tbs-pubkey/src/lib.nr:110

• Exponent verification still relies entirely on the prover-supplied exponent_offset and does not validate the DER structure around it (e.g., INTEGER tag/length at exponent_offset - 2) or that it is the INTEGER immediately following the modulus inside the same RSAPublicKey SEQUENCE. This allows a proof to satisfy the exponent check by pointing at a matching byte pattern elsewhere in the signed TBS. Consider deriving the expected exponent position from pubkey_offset/DSC_KEY_SIZE (and the modulus form) and/or asserting the DER tag/length and that exponent_offset matches the derived location.

    // Verify exponent bytes match the authenticated TBS certificate at exponent_offset.
    //
    // All 8 allowed exponents are either 1 byte (exponent=3) or 3 bytes (all others).
    //
    // 3-byte exponents and their expected DER value bytes:
    //   38129  (0x94B1)   -> [0x00, 0x94, 0xB1]  (sign byte needed, MSB >= 128)
    //   56611  (0xDD23)   -> [0x00, 0xDD, 0x23]  (sign byte needed, MSB >= 128)
    //   65537  (0x010001) -> [0x01, 0x00, 0x01]
    //   107903 (0x01A57F) -> [0x01, 0xA5, 0x7F]
    //   109729 (0x01AC61) -> [0x01, 0xAC, 0x61]
    //   122125 (0x01DC0D) -> [0x01, 0xDC, 0x0D]
    //   130689 (0x01FF01) -> [0x01, 0xFF, 0x01]
    //
    // For the 3-byte case, u32 >> 16 naturally gives 0x00 for exponents needing a sign byte,
    // and the expected 3-byte sequence matches the DER value encoding directly.
    if exponent < 0x100 {
        assert(exponent_offset < TBS_CERT_SIZE, "Exponent offset out of bounds");
        // 1-byte exponent (only exponent=3 in the allowed list).
        // exponent_offset must point to the single value byte in TBS.
        assert(tbs_certificate[exponent_offset] == exponent as u8, "Exponent mismatch in TBS");
    } else {
        // 3-byte exponent. exponent_offset points to the first value byte (incl. sign byte).
        assert(exponent_offset + 3 <= TBS_CERT_SIZE, "Exponent bytes out of bounds");
        assert(

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.