Skip to content

Conversation

@v1bri
Copy link

@v1bri v1bri commented Nov 5, 2025

Introduce test harness capable of running ACVP test vectors on the Cryptotest firmware. Test outputs can be written to a result file for uploading to the NIST demo server or another verification authority. Also supports comparing results against a golden reference.

This initial version supports HMAC-SHA2, with test vectors and expected outputs that have been verified against the NIST demo server.

@v1bri v1bri requested a review from a team as a code owner November 5, 2025 05:55
@v1bri v1bri requested review from cfrantz and removed request for a team November 5, 2025 05:55
Copy link
Contributor

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, this looks like a great addition to the crypto testing. I've left a couple of comments and suggestions/nits but the implementation looks good to me, and I'm happy to approve when CI is passing.

With regards to getting past CI, there is some guidance on the format of commit messages. These are mostly just suggestions, but there are two aspects that are generally enforced:

  1. The commit message should contain a prefix describing the area/scope of the changes. This is entirely up to you, but my suggestion would be to git commit --amend and change the first line to be

    [cryptotest] ACVP test harness with HMAC-SHA2-256 vectors

    It would also be great if you could do the same with the PR title :)

  2. We require that commits are signed-off to indicate agreement with the Contributor License Agreement (CLA) - see CONTRIBUTING.md for more information. To do that you can run:

    git commit --amend --signoff --no-edit
    git push -f

Copy link
Contributor

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick changes.

It would be great if you could add the [cryptotest] prefix to your commit message (see point 1 here), but other than that I'm happy.

Edit: It also looks like CI is complaining about trailing whitespaces. I think this should normally be handled by ./bazelisk.sh run format, but it looks like we don't have this for JSON files. You can run ./ci/scripts/whitespace.sh master to find trailing whitespace introduced in the diff from master, but given we already know the issue you can run:

./util/fix_trailing_whitespace.py sw/host/cryptotest/testvectors/acvp/hmac_sha256_expected.json

to fix the problem (alternatively, just manually fix the missing newline at the end of the file).

Copy link
Member

@nasahlpa nasahlpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Brian for the contribution!

All 600 test vectors are passing & the code looks good to me!

Once CI passes, we can merge this.

@v1bri v1bri changed the title ACVP test harness with HMAC-SHA2-256 vectors [cryptotest] ACVP test harness with HMAC-SHA2-256 vectors Nov 5, 2025
@AlexJones0
Copy link
Contributor

(I think it needs another ./bazelisk.sh run format after the clippy lint fixes 😅)

Introduce test harness capable of running ACVP test vectors on the
Cryptotest firmware. Test outputs can be written to a result file for
uploading to the NIST demo server or another verification authority.
Also supports comparing results against a golden reference.

This initial version supports HMAC-SHA2, with test vectors and expected
outputs that have been verified against the NIST demo server.

Signed-off-by: Brian Orr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants