Conversation
CBMC Results (ML-KEM-768)Full Results (187 proofs)
|
CBMC Results (ML-KEM-512)Full Results (187 proofs)
|
CBMC Results (ML-KEM-1024)Full Results (187 proofs)
|
724e0b1 to
9a3e91a
Compare
|
Modified the two remaining occurences of _MEMSAFE to _SAFE in order to fit the expected format for aarch proofs. |
25cad96 to
3e258d8
Compare
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @L-series - I left a couple of comments.
This PR actually takes another approach that I had in mind: I thought that after running each HOL-Light proof we check for the expected theorems to be present.
The approach implemented here is more like a linter. But I support the addition!
The -a flag is highly appreciated - thank you!
scripts/check-theorems
Outdated
| [[ $ARCH == "x86_64" && $routine == "mlkem_rej_uniform" ]] && suffixes=("CORRECT") | ||
|
|
||
| for sfx in "${suffixes[@]}"; do | ||
| if ! grep -q "_${sfx}" "$PROOFS_DIR/${routine}.ml"; then |
There was a problem hiding this comment.
can you actually grep for {suffix} = prove or {suffix} = time prove?
There was a problem hiding this comment.
Done, however please note that I modified some of the proofs as they included double whitespaces before the = prove. I opted for this as it seems cleaner than having a more complicated regexp. As an aside, perhaps there is some tool to apply standard formatting onto the *.ml files?
This commit introduces two new flags to the hol_light command of the tests script. The first allows user specification of which architecture to run/list the proofs for and the second allows for entering the nix cross-compilation devshell for that architecture. If either are not passed, the behavior is unchanged. Signed-off-by: Andreas Hatziiliou <andreas.hatziiliou@savoirfairelinux.com>
We add a check to the linting script called check-theorems that ensures that all HOL-Light proofs provide the expected set of theorems depending on the architecture. Signed-off-by: Andreas Hatziiliou <andreas.hatziiliou@savoirfairelinux.com>
Thank you for the review! Indeed, the idea I had in mind was that one should avoid using compute on proofs which do not contain the required theorems. However, I see now that there is still value running the HOL CI even if the routine is only partially proven. |
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @L-series for the changes! I'm happy with the overall approach now.
Two comments still need to be addressed. Thank you!
| if [[ $arch == "x86_64" && ${routine} == "mlkem_rej_uniform" ]]; then | ||
| suffixes=("CORRECT") | ||
| elif [[ $arch == "x86_64" ]]; then | ||
| suffixes+=("SUBROUTINE_CORRECT" "NOIBT_SUBROUTINE_CORRECT" "NOIBT_SUBROUTINE_SAFE") |
There was a problem hiding this comment.
I tried removing the SUBROUTINE_SAFE proof from the mlkem_poly_compress_d4.ml proof and the script did not catch it. Is that because NOIBT_SUBROUTINE_SAFE matches the SUBROUTINE_SAFE suffix? That should be fixed.
We can actually check the full theorem name, not just the suffix. It should be possible to derive the theorem names from the file names - we should enforce that and adjust theorem names where needed.
| if [[ $arch == "x86_64" && ${routine} == "mlkem_rej_uniform" ]]; then | ||
| suffixes=("CORRECT") |
There was a problem hiding this comment.
Why is there this exceptions? There are memory safety proofs for this routine?
| (* ========================================================================= *) | ||
|
|
||
| let MLKEM_REJ_UNIFORM_MEMSAFE = prove | ||
| let MLKEM_REJ_UNIFORM_SAFE = prove |
There was a problem hiding this comment.
This was intentionally named this way and I would prefer to keep it like that because we do not have any constant time proofs at this time.
Can you implement it as: if mlkem_rej_uniform then MEMSAFE else SAFE? That would be better, I believe.
|
@L-series, gentle ping. Could you please adjust this PR so we can get this merged? |
This commit aims to resolve the two following issues:
testsscript to allow cross-platform proof checking #1585Note that the
check-theoremsscript will currently fail the HOL-Light CI because for AArch, the naming convention for the memory safety proof (usually _SAFE) is not respected inmlkem_rej_uniform(it uses _MEMSAFE). @hanno-becker please advise if I should modify this.