ML-DSA: Add optional context to signing and verification#90
ML-DSA: Add optional context to signing and verification#90mjdemilliano wants to merge 1 commit intowolfSSL:masterfrom
Conversation
|
@mjdemilliano is an approved contributor |
There was a problem hiding this comment.
Pull request overview
This PR adds optional “context” (ctx) support to ML-DSA (Dilithium) signing and verification, enabling domain separation when using the wolfSSL context-aware APIs.
Changes:
- Extend
MlDsaPrivate.sign()and_MlDsaBase.verify()with an optionalctxargument and route to*_ctx_msgwolfCrypt APIs when provided. - Add tests covering sign/verify behavior with correct/incorrect context.
- Extend the CFFI bindings to include
wc_dilithium_sign_ctx_msgandwc_dilithium_verify_ctx_msg.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
wolfcrypt/ciphers.py |
Adds ctx parameter and switches between ctx/non-ctx wolfCrypt functions. |
tests/test_mldsa.py |
Adds coverage for context-aware signing and verification. |
scripts/build_ffi.py |
Declares the new ctx-aware wolfCrypt APIs in the FFI cdef. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ctx is not None: | ||
| ctx_bytestype = t2b(ctx) | ||
| ret = _lib.wc_dilithium_sign_ctx_msg( | ||
| _ffi.from_buffer(ctx_bytestype), | ||
| len(ctx_bytestype), | ||
| _ffi.from_buffer(msg_bytestype), |
There was a problem hiding this comment.
wc_dilithium_sign_ctx_msg() takes ctxLen as a C "byte" in the FFI definition, but this wrapper passes len(ctx_bytestype) without bounds checking. For ctx lengths > 255, cffi will truncate the value and only a prefix of ctx will be used, which is very surprising and can break domain separation; please validate ctx length (or adjust the FFI signature if ctxLen is actually wider).
| if ret < 0: # pragma: no cover | ||
| raise WolfCryptError("wc_dilithium_sign_msg() error (%d)" % ret) |
There was a problem hiding this comment.
When ctx is provided, failures will raise an exception that still references wc_dilithium_sign_msg(), even though wc_dilithium_sign_ctx_msg() was called. Please update the error string to match the actual call path (or include ctx/no-ctx in the message).
| int wc_dilithium_export_public(dilithium_key* key, byte* out, word32* outLen); | ||
| int wc_dilithium_import_public(const byte* in, word32 inLen, dilithium_key* key); | ||
| int wc_dilithium_sign_msg(const byte* msg, word32 msgLen, byte* sig, word32* sigLen, dilithium_key* key, WC_RNG* rng); | ||
| int wc_dilithium_sign_ctx_msg(const byte* ctx, byte ctxLen, const byte* msg, word32 msgLen, byte* sig, word32* sigLen, dilithium_key* key, WC_RNG* rng); |
There was a problem hiding this comment.
The new FFI declarations use different types for ctxLen: wc_dilithium_sign_ctx_msg declares ctxLen as byte, but wc_dilithium_verify_ctx_msg declares ctxLen as word32. Please confirm the correct signature in the wolfSSL headers and make these consistent; if ctxLen is actually word32, the current byte declaration will truncate lengths >255 at the ABI boundary.
| int wc_dilithium_sign_ctx_msg(const byte* ctx, byte ctxLen, const byte* msg, word32 msgLen, byte* sig, word32* sigLen, dilithium_key* key, WC_RNG* rng); | |
| int wc_dilithium_sign_ctx_msg(const byte* ctx, word32 ctxLen, const byte* msg, word32 msgLen, byte* sig, word32* sigLen, dilithium_key* key, WC_RNG* rng); |
| wrong_message = b"This is a wrong message for ML-DSA signature" | ||
| assert not mldsa_pub.verify(signature, wrong_message) | ||
|
|
||
| # Verify with ctx for signature generated without |
There was a problem hiding this comment.
Minor test comment clarity: "Verify with ctx for signature generated without" reads incomplete; consider updating to something like "...generated without context" so the intent is unambiguous.
| # Verify with ctx for signature generated without | |
| # Verify with ctx for signature generated without context |
danielinux
left a comment
danielinux
left a comment
There was a problem hiding this comment.
Please address copilot's commends
No description provided.