ML-DSA: Support deterministic signing#88
Conversation
|
@mjdemilliano is an approved contrinutor |
There was a problem hiding this comment.
Pull request overview
Adds deterministic ML-DSA signature generation support by allowing callers to supply an explicit 32-byte seed, wiring the new wolfSSL APIs through the Python binding and adding test coverage for deterministic behavior.
Changes:
- Add
MlDsaPrivate.sign_with_seed()to generate deterministic signatures using a caller-provided seed (optionally with a context). - Extend the CFFI
cdefwithwc_dilithium_sign_msg_with_seed/wc_dilithium_sign_ctx_msg_with_seed. - Add a unit test validating deterministic re-signing and basic seed-length checking.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| wolfcrypt/ciphers.py | Introduces deterministic signing API and seed-length constant for ML-DSA private keys. |
| tests/test_mldsa.py | Adds tests for deterministic signing behavior with a fixed seed. |
| scripts/build_ffi.py | Exposes the new wolfSSL deterministic-signing functions to CFFI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert isinstance(seed, bytes) and len(seed) == MlDsaPrivate._SIGNATURE_SEED_LENGTH, \ | ||
| f"Seed for generating a signature must be {MlDsaPrivate._SIGNATURE_SEED_LENGTH} bytes." |
There was a problem hiding this comment.
assert is being used for runtime input validation of seed. Asserts can be disabled with Python optimizations (-O), which would remove this check and change the exception behavior for invalid seeds. Prefer raising TypeError/ValueError (or WolfCryptError if that’s the library convention) with a clear message instead.
| assert isinstance(seed, bytes) and len(seed) == MlDsaPrivate._SIGNATURE_SEED_LENGTH, \ | |
| f"Seed for generating a signature must be {MlDsaPrivate._SIGNATURE_SEED_LENGTH} bytes." | |
| if not isinstance(seed, bytes): | |
| raise TypeError("Seed for generating a signature must be bytes.") | |
| if len(seed) != MlDsaPrivate._SIGNATURE_SEED_LENGTH: | |
| raise ValueError( | |
| f"Seed for generating a signature must be " | |
| f"{MlDsaPrivate._SIGNATURE_SEED_LENGTH} bytes." | |
| ) |
| if ctx is not None: | ||
| ctx_bytestype = t2b(ctx) | ||
| ret = _lib.wc_dilithium_sign_ctx_msg_with_seed( | ||
| _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_with_seed takes ctxLen as a byte (0–255), but len(ctx_bytestype) is passed without bounds checking. If ctx is longer than 255 bytes this will truncate/overflow at the C boundary and sign using an unintended context length. Validate len(ctx_bytestype) <= 255 (and raise a deterministic exception) before calling into _lib.
| from wolfcrypt.ciphers import MlDsaPrivate, MlDsaPublic, MlDsaType | ||
| from wolfcrypt.random import Random | ||
|
|
||
| ML_DSA_SIGNATURE_SEED_LENGTH = 32 |
There was a problem hiding this comment.
This duplicates the seed-length constant that is already defined on MlDsaPrivate (_SIGNATURE_SEED_LENGTH). To avoid tests drifting if the library constant ever changes, consider referencing MlDsaPrivate._SIGNATURE_SEED_LENGTH directly instead of hard-coding 32 here.
| ML_DSA_SIGNATURE_SEED_LENGTH = 32 | |
| ML_DSA_SIGNATURE_SEED_LENGTH = MlDsaPrivate._SIGNATURE_SEED_LENGTH |
| # re-generate from the same seed: | ||
| signature_from_same_seed = mldsa_priv.sign_with_seed(message, signature_seed) | ||
| assert signature == signature_from_same_seed | ||
|
|
There was a problem hiding this comment.
sign_with_seed(..., ctx=...) introduces a separate code path (calls wc_dilithium_sign_ctx_msg_with_seed), but this test suite only exercises the ctx is None branch. Add a test that passes a non-None ctx (and ideally covers boundary conditions like empty ctx and max-length ctx) to ensure the new path works and stays covered.
| # Exercise the non-None ctx code path, including boundary conditions. | |
| empty_ctx = b"" | |
| signature_empty_ctx = mldsa_priv.sign_with_seed( | |
| message, signature_seed, ctx=empty_ctx | |
| ) | |
| assert len(signature_empty_ctx) == mldsa_priv.sig_size | |
| assert mldsa_pub.verify(signature_empty_ctx, message) | |
| assert signature_empty_ctx == mldsa_priv.sign_with_seed( | |
| message, signature_seed, ctx=empty_ctx | |
| ) | |
| ctx = b"ml-dsa ctx" | |
| signature_with_ctx = mldsa_priv.sign_with_seed(message, signature_seed, ctx=ctx) | |
| assert len(signature_with_ctx) == mldsa_priv.sig_size | |
| assert mldsa_pub.verify(signature_with_ctx, message) | |
| assert signature_with_ctx == mldsa_priv.sign_with_seed( | |
| message, signature_seed, ctx=ctx | |
| ) | |
| max_ctx = b"x" * 255 | |
| signature_max_ctx = mldsa_priv.sign_with_seed( | |
| message, signature_seed, ctx=max_ctx | |
| ) | |
| assert len(signature_max_ctx) == mldsa_priv.sig_size | |
| assert mldsa_pub.verify(signature_max_ctx, message) | |
| assert signature_max_ctx == mldsa_priv.sign_with_seed( | |
| message, signature_seed, ctx=max_ctx | |
| ) |
danielinux
left a comment
There was a problem hiding this comment.
Please address copilot's commends
No description provided.