Repository navigation
Various small fixes and improvements - #1468
mkannwischer wants to merge 7 commits into
Conversation
CBMC Results (ML-DSA-65, REDUCE-RAM)
Full Results (415 configurations)
|
CBMC Results (ML-DSA-87, REDUCE-RAM)
Full Results (415 configurations)
|
CBMC Results (ML-DSA-44, REDUCE-RAM)
Full Results (415 configurations)
|
CBMC Results (ML-DSA-44)
Full Results (415 configurations)
|
CBMC Results (ML-DSA-65)
Full Results (415 configurations)
|
CBMC Results (ML-DSA-87)
Full Results (415 configurations)
|
74ad5fa to
0ec538c
Compare
| ensures(ctxlen <= 255 || return_value == 0) | ||
| ensures(hashalg == MLD_PREHASH_NONE || ph != NULL || return_value == 0) |
There was a problem hiding this comment.
This is a bit difficult to read.
Can we express this as implications? E.g.
ensures(ctxlen > 255 ==> return_value == 0)`
| requires(phlen <= MLD_MAX_BUFFER_SIZE) | ||
| requires(ctxlen == 0 || memory_no_alias(ctx, ctxlen)) | ||
| requires(hashalg == MLD_PREHASH_NONE || memory_no_alias(ph, phlen)) | ||
| requires(hashalg == MLD_PREHASH_NONE || ph == NULL || |
There was a problem hiding this comment.
This is inconsistent with other public API which isn't checking for NULL pointers either.
There was a problem hiding this comment.
The contract change was just matching what the code does, but I agree that the code is currently not doing what our conventions suggest. I'll change it.
| #ifndef MLD_DEV_RISCV32_FASTMUL_H | ||
| #define MLD_DEV_RISCV32_FASTMUL_H | ||
|
|
||
| #if !defined(MLD_SYS_RISCV32_M) |
There was a problem hiding this comment.
Hm, we don't have this for the other backends, though... we should be consistent, whatever we do.
There was a problem hiding this comment.
I'm not aware of a case before that a backend can accidentally be assembled for an unsupported architecture instead of failing to assemble at all. If we do have more cases like that, we should guard it equally.
The x86_64 FIPS202 backend has a similar guard, but I think that's actually not needed:
hanno-becker
left a comment
There was a problem hiding this comment.
Some questions regarding consistency of the changes; see comments.
0ec538c to
7997e09
Compare
Unsupported values previously failed far downstream with unrelated errors. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
…tion The public header declared the function in keygen-only builds, where it is not defined. Use the guard of the definition. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
Relax the precondition on ctxlen so that the CBMC proof covers the documented error return for an overlong context string. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
Per API-CONVENTIONS.md, mldsa-native does not check pointers for NULL, and for HashML-DSA ph is paired with a non-zero length. Drop the NULL check and the corresponding documented error. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
mld_sign_pk_from_sk() is not built with MLD_CONFIG_CORE_API_ONLY, but its helpers were. With MLD_CONFIG_INTERNAL_API_QUALIFIER=static they were unused and -Werror builds failed. Guard them by their actual users. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
Selecting the backend through MLD_CONFIG_ARITH_BACKEND_FILE bypassed the check in native/meta.h; on RV64 it built but computed wrong results. The CBMC proofs of the C wrappers run on the host and now define MLD_SYS_RISCV32_M. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
A custom capability function could not disable the backend. Check MLD_SYS_CAP_ARMV81M_MVE in all three entry points so that they fall back together, as the state is kept bit-interleaved. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
7997e09 to
a238db9
Compare
No description provided.