MLDSA-composite#5451
Conversation
6befa28 to
4edf6ce
Compare
3bf999d to
763cf9d
Compare
763cf9d to
d4d4afe
Compare
There was a problem hiding this comment.
Not a full review at all, these comments are just what immediately jumped out at me from a brief scan.
A more broad observation is that this PR is way too large. At the very least the ECC private key OID encoding patch and the ML-DSA context string argument should both be split out into distinct PRs, with tests of their own independent of ML-DSA-composite.
Also looking how some now very complex policies are being dumped into a string here, I think something like #5021 is basically required.
[Note added by @falko-strenzke: The active and current PR for the signature options mentioned above is #5489]
| }) | ||
|
|
||
| #if defined(BOTAN_HAS_RSA) | ||
| BOTAN_REGISTER_PERF_TEST_TMPL(MLDSA44_RSA2048_PKCS15_SHA256, PerfTest_MLDSA_Composite); |
There was a problem hiding this comment.
This approach is very unlike how the rest of the perf tests are presented where there is a single entry point which then iterates over a list of possible parameters (eg botan speed ECDH) IMO it should follow that style rather than presenting one perflet per parameter set.
There was a problem hiding this comment.
MLDSA44_RSA2048_PKCS15_SHA256 is not a parameter, it's an algorithm.
Currently, all the different algorithms defined in draft-ietf-lamps-pq-composite-sigs-15 are
modelled as such, that is, algorithms, in Botan.
I don't really see an alternative to this.
If it is desired to turn them into parameters of an abstract MLDSA-composite algorithm, that seems
also possible.
It doesn't reflect the to-be-RFCs modelling, though.
Furthermore, the availability of the individual parameter sets is determined by compiled support for
the traditional algorithm components.
The case that none are available could be caught at compile time, but I am not sure if the
conditional availability of parameter sets might break assumptions in some places.
I guess this is an unprecedented case.
Apart from these considerations, the approach to model X.509 algorithms as parameters is already
taken for ML-DSA and SLH-DSA.
What I would refrain from in any case is assigning a default parameter when none is given.
The choice seems too deliberate for the composites in my view, and would all the more have to vary
based on compiled support for the traditional components.
There was a problem hiding this comment.
@reneme @randombit How do we proceed with the issue of parameterization that I outline above? Should I keep the current model "every MLDSA-composite parameter set is an algorithm" or should I refactor it to the model "MLDSA-composite is an algorithm with the various parameter sets"?
I would favor a quick decision here because I will soon start with the implementation of MLKEM-composite and ideally would create it in the right spirit to begin with.
|
|
||
| explicit MLDSA_Signing_Parameters(std::string_view param_str) : m_rndmzd(true), m_ctx({}) { | ||
| const char* error_tmpl = "Parameter string '{}' is not a valid ML-DSA or Dilithium parameter specification"; | ||
| if(param_str.empty()) { |
There was a problem hiding this comment.
This function seems like an excellent argument for #5021 ...
[Note added by @falko-strenzke: The active and current PR for the signature options mentioned above is #5489]
There was a problem hiding this comment.
Extending the public key API regarding the parameterization is a great idea. If you remember, I made a proposal some time ago. In my view, the API extension should not continue the usage of string function parameters. Instead, we should have a typed framework, that ideally already verifies parameter choices at compile time. At the same time, the legacy string-based API will most likely need continued support for a longer time. If we can agree on a reasonable approach here, I would try to see what we can do in the scope of the current BSI project, which runs until October of this year.
For ML-DSA context this is straightforward. I put it on my task list. @randombit |
|
@randombit |
5b296a6 to
a22193f
Compare
a22193f to
9da975e
Compare
| * ``dilithium_aes``, that uses AES instead of Keccak-based primitives. | ||
| This mode is deprecated and will be removed in a future release. | ||
|
|
||
| ML-DSA (draft-ietf-lamps-pq-composite-sigs-15) |
There was a problem hiding this comment.
| ML-DSA (draft-ietf-lamps-pq-composite-sigs-15) | |
| ML-DSA-composite (draft-ietf-lamps-pq-composite-sigs-15) |
b0bb319 to
272eb63
Compare
faf8b51 to
9ca6421
Compare
Implementation of MLDSA composite defined in draft-ietf-lamps-pq-composite-sigs-15.
The various composite algorithms are modelled as algorithms in Botan.
This PR also is dependend on #5531 (context parameter encoding for ML-DSA).
Our internal review is still pending, this is why I mark it as draft. But early feedback on the overall architecture is welcome. Once we have agreed on the principal design, I can start with the MLKEM composite implementation. The code will be equivalent to a great degree.
(Also note that creating the branch in the upstream repo was not intended ...)
Open Tasks:
each algorithm combination (parameter set) is one algorithm in Botan (currently implemented)