-
Notifications
You must be signed in to change notification settings - Fork 907
[crypto] Add missing tests to the testplan #28636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nasahlpa, I've left a few comments but this looks like a nice improvement to me. It would also be good if @engdoreis could check this, particularly with regards to the specified SiVal stages.
As a question, do you currently see the //sw/device/tests/crypto/cryptotest:rsa_kat passing locally? I've tried running rsa_kat_fpga_cw340_sival_rom_ext but I get an error:
thread 'main' panicked at sw/host/tests/crypto/rsa_kat/src/main.rs:111:53:
called `Result::unwrap()` on an `Err` value: CapacityError: insufficient capacity
...
I haven't had time to debug the problem fully, but it looks generally like the RSA 4096 wycheproof test vectors have n of a length that is parsed to 513 bytes whereas the cryptotest UJSON definitions define a RSA_CMD_MAX_N_BYTES of 512. Not sure why this is happening - perhaps there is an extra byte/nibble that needs to be stripped somewhere, or some parsing logic has gone slightly wrong?
| desc: '''Check that KMAC works correctly. | ||
|
|
||
| Run KMAC and compare the output against | ||
| the wycheproof test vector. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: while we use the wycheproof test vectors here, there are also some example NIST KMAC test vectors (not part of CAVP) for KMAC/KMACXOF here that could potentially be used. It looks like DV is already making use of these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, we should think of adding additional test vectors for KMAC.
Add the AES-GCM and RSA tests to the `crypto_kat_test_suite` target. Signed-off-by: Pascal Nasahl <[email protected]>
The testplan located in `sw/device/lib/crypto/data` is missing some of the cryptotests that we have. Add them to the testplan. Signed-off-by: Pascal Nasahl <[email protected]>
|
Thanks @AlexJones0 for the careful review! I have fixed the failing RSA test you have discovered in PR #28655. |
This PR consists of two PRs:
crypto_kat_test_suitetarget