add more test coverage to crypto_rejects

[meilu-git]

No description provided.

[Copilot]

Pull request overview

This PR expands the TBOR PartInit negative-path integration tests for mach_seed_envelope handling, aiming to increase coverage across AEAD authentication failures, fixed-length validation, and session/partition recovery behaviors.

Changes:

• Adds targeted tampering tests for envelope components (IV/AAD/tag) and boundary-length rejects.
• Adds robustness tests for repeatability/isolation of rejects and successful PartInit following prior rejects.
• Adds additional behavioral tests around session closure/replay and parameter inputs (e.g., POTA thumbprint, all-zero seed).

Comments suppressed due to low confidence (3)

ddi/tbor/types/tests/commands/part_init/crypto_rejects.rs:255

• The AEAD envelope header is 8 bytes (magic+alg+rsv+aad_len). With HEADER_LEN = 4, this test mutates header/reserved bytes rather than the AAD region, so it doesn't validate AAD authentication as intended.

    // HEADER(4) ‖ IV(12) ‖ AAD(32) ‖ CT(32) ‖ TAG(16)
    const HEADER_LEN: usize = 4;
    const IV_LEN: usize = 12;
    let aad_index = HEADER_LEN + IV_LEN;
    envelope[aad_index] ^= 0x01;

ddi/tbor/types/tests/commands/part_init/crypto_rejects.rs:408

• The AEAD envelope header length is 8 bytes in this repo. Starting the tamper loop at 4 means the test includes header fields (alg/rsv/aad_len) that aren't part of the IV/AAD/CT/TAG regions and may exercise different failure modes than intended.

    const HEADER_LEN: usize = 4;

ddi/tbor/types/tests/commands/part_init/crypto_rejects.rs:560

• The AEAD envelope header is 8 bytes; using HEADER_LEN = 4 makes ciphertext_index and the IV/AAD mutation indices point into the header/reserved/aad_len fields instead of the intended regions.

    const HEADER_LEN: usize = 4;
    const IV_LEN: usize = 12;
    const AAD_LEN: usize = 32;

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (5)

ddi/tbor/types/tests/commands/part_init/crypto_rejects.rs:254

• HEADER_LEN is hard-coded as 4, but the AEAD envelope header in this repo is 8 bytes (azihsm_crypto::aead_envelope::HEADER_LEN). With HEADER_LEN = 4, aad_index points into the IV instead of the AAD, so this test is not actually exercising AAD tampering.

    // Envelope:
    // HEADER(4) ‖ IV(12) ‖ AAD(32) ‖ CT(32) ‖ TAG(16)
    const HEADER_LEN: usize = 4;
    const IV_LEN: usize = 12;
    let aad_index = HEADER_LEN + IV_LEN;

ddi/tbor/types/tests/commands/part_init/crypto_rejects.rs:402

• The doc comment and HEADER_LEN assume a 4-byte envelope header, but the AEAD envelope header is 8 bytes. Starting at offset 4 causes this loop to tamper the header (alg/rsv/aad_len) and shifts all offsets, so the test doesn't match its stated intent ("every byte after the envelope header").

/// Envelope layout:
/// HEADER(4) ‖ IV(12) ‖ AAD(32) ‖ CT(32) ‖ TAG(16)
///
/// The header is intentionally excluded because malformed header fields may
/// follow a separate structural-decoding path and return a status other than

ddi/tbor/types/tests/commands/part_init/crypto_rejects.rs:559

• HEADER_LEN is set to 4, but the AEAD envelope header is 8 bytes. With the wrong header length, the indices labeled "iv", "aad", and "ciphertext" actually fall into the header/IV/AAD respectively, so this test is not mutating the intended components.

    const HEADER_LEN: usize = 4;
    const IV_LEN: usize = 12;
    const AAD_LEN: usize = 32;

ddi/tbor/types/tests/commands/part_init/crypto_rejects.rs:462

• This test treats the envelope header length as 4 bytes, but the AEAD envelope header is 8 bytes. As written, it doesn't include the actual "header-only" length in the matrix and the header-length assertion can be incorrect if the header constant ever changes.

    let valid_len = valid_envelope.len();
    assert!(
        valid_len > 4,
        "valid envelope length must exceed header length"
    );

ddi/tbor/types/tests/commands/part_init/crypto_rejects.rs:801

• This test only asserts that an error occurs and then prints it. Given the PartInit spec documents SessionNotFound for an unknown session_id, not asserting the returned status makes this test much less effective at catching regressions in session resolution.

    let err = ctx
        .tbor(&req)
        .expect_err("unknown request session id must be rejected");

    eprintln!("unknown session rejection: {err:#?}");
}

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

ddi/tbor/types/tests/commands/part_init/crypto_rejects.rs:49

• try_read_from_bytes(..) returns a Result; calling .ok().expect(..) discards the actual error, making failures harder to diagnose. You can expect on the Result directly and keep the error context.

    req.part_policy =
        <PartPolicy as zerocopy::TryFromBytes>::try_read_from_bytes(&known_good_part_policy())
            .ok()
            .expect("known-good policy parses");