Skip to content
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

Incorrect Order of z and d in NIST tests #1853

Closed
abhinav-thales opened this issue Jul 23, 2024 · 1 comment · Fixed by #1854
Closed

Incorrect Order of z and d in NIST tests #1853

abhinav-thales opened this issue Jul 23, 2024 · 1 comment · Fixed by #1854

Comments

@abhinav-thales
Copy link
Contributor

In the NIST tests for ML-KEM, the random number stream is constructed as [z || d || m] i.e. the first random byte returned by randombytes() would be z followed by d and m.

However, in the code

64 bytes random is generated together https://github.com/open-quantum-safe/liboqs/blob/main/src/kem/ml_kem/pqcrystals-kyber-standard_ml-kem-512-ipd_ref/kem.c#L54
and then passed to derand api for keypair generation.

In the derand api, first 32 byte is used for keypair generation https://github.com/open-quantum-safe/liboqs/blob/main/src/kem/ml_kem/pqcrystals-kyber-standard_ml-kem-512-ipd_ref/kem.c#L29
which makes the first 32 byte as 'd' as per Algo 12 of FIPS-203.

The next 32 bytes are appended to end of private key https://github.com/open-quantum-safe/liboqs/blob/main/src/kem/ml_kem/pqcrystals-kyber-standard_ml-kem-512-ipd_ref/kem.c#L33
which makes it 'z' as per Algo 15.

So, the random byte stream construction should be [d || z || m] so that randombytes are output in the order of d->z->m

Currently, it works for NIST Intermediate Vectors as the value of z and d are same for all KEM version. Incase, they are different the tests would fail.

A simple switch of z and d at https://github.com/open-quantum-safe/liboqs/blob/main/tests/test_kem_vectors.sh#L26 should solve this IMO.

@bhess
Copy link
Member

bhess commented Jul 23, 2024

Thank you @abhinav-thales for catching this! I think your analysis is correct, which would indeed lead to issues if z and d are not the same. I'll follow up with a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants