Test Improvements for ML-KEM

[abhinav-thales]

This PR does the following:
- enable ML-KEM ACVP tests on Windows
Since ML-KEM final specification is released, it would be good to have the tests enabled on Windows
- testcases for ML-KEM rejection key
Add negative testcase as per Algorithm 18 of ML-KEM final specification when the calculated encrypted text does not match the passed ciphertext
- harden checks in vector_kem for RNG lengths and correct error message in decaps

[SWilson4]

Not sure if you're working on Windows @abhinav-thales, but this script might be helpful to get past the formatting check.

[SWilson4]

@abhinav-thales I triggered CI on the latest commit; please see the minor build failures.

[bhess]

Thank you for this additional test @abhinav-thales !
See the to comments in the code.
The new test is specific to ML-KEM while the existing tests apply to all KEMs. I think it would make sense to structure the code a bit differently, for example moving the new test to separate function.

[abhinav-thales]

Thank you for this additional test @abhinav-thales ! See the to comments in the code. The new test is specific to ML-KEM while the existing tests apply to all KEMs. I think it would make sense to structure the code a bit differently, for example moving the new test to separate function.

Thank you @bhess for the review. Have moved the added testcase to a separate function now.

[bhess]

Thanks for the updates! One more general comment about the test:
The main use case of the rejection key in ML-KEM seems to be if active attackers modify the ciphertext, where the decapsulation should output the rejection key. A test for this would modify the ciphertext and check if the rejection keys match, while your test modifies the secret key. Could you also add a test case that modifies the ciphertext?

[abhinav-thales]

Thanks for the updates! One more general comment about the test: The main use case of the rejection key in ML-KEM seems to be if active attackers modify the ciphertext, where the decapsulation should output the rejection key. A test for this would modify the ciphertext and check if the rejection keys match, while your test modifies the secret key. Could you also add a test case that modifies the ciphertext?

added the testcase now.

[bhess]

Thanks @abhinav-thales for adding the additional test! Looks good to me.

[baentsch]

Thanks for the contribution, @abhinav-thales . Please see the single comments and mark/#ifdef explicitly the algorithm-specific code, e.g., using OQS_ENABLE_KEM_ML_KEM, to allow the system to keep its (admittedly only internal) differentiation in generalized and algorithm-specific code.

[abhinav-thales]

Thanks for the contribution, @abhinav-thales . Please see the single comments and mark/#ifdef explicitly the algorithm-specific code, e.g., using OQS_ENABLE_KEM_ML_KEM, to allow the system to keep its (admittedly only internal) differentiation in generalized and algorithm-specific code.

Hi @baentsch , thanks for the review. The ML-KEM tests are now under conditional check of ML-KEM configuration.

[abhinav-thales]

@abhinav-thales I triggered CI on the latest commit; please see the minor build failures.

Hi @SWilson4 , can you trigger the workflow on the latest commit as well please ?

[abhinav-thales]

Hi @SWilson4 @baentsch the review comments are addressed now. Could you please help with triggering CI pipeline if the fixes are fine. Thanks.

[SWilson4]

Looks good to me. Thanks for making the requested changes!

[abhinav-thales]

Hi @baentsch , If the changes meet your expectations, could you please approve them? If there are any additional adjustments needed, kindly let me know. Thank you !

[baentsch]

Thanks for re-requesting review @abhinav-thales : I was under the mistaken assumption that by marking all comments resolved things turn green, sorry. Now explicitly marked so. Thanks on behalf of the FOSS community for your contributions to improve the library for everyone!