Add API support for External-MU Signature[ML-DSA]#2366
Conversation
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
|
Hi @bhess , I was trying to implement the external mu OQS APIs based on approach 1 suggested here by you. However, in my (limited) understanding it seems difficult adding a "new variant" without having support from the upstream repo(see one error below). Tried adding the new variants "ml_dsa_xx_extmu" in 'copy_from_upstream.yml' under ml-dsa umbrella leading to 'FileNotFoundError: [Errno 2] No such file or directory: 'repos/mldsa-native/integration/liboqs/ML-DSA-44-EXTMU_META.yml'. Additionally, the extmu and the existing variants shall be using the same set of source files, which could lead to linking errors due to "same definition" if both the variant are enabled. Please let me know your opinion or please guide me incase I am in the wrong direction. Thanks. |
|
Hi @abhi-dev-engg, thanks for working on enabling externalMu. In my view, the preferred approach is to add the external-mu API definition directly to the upstream YAML file (for example, in ML-DSA-44_META.yml, signature_signature_extmu/signature_verify_extmu). By doing this, we can then adapt the copy-from-upstream script to automatically generate the code for the -externalmu variant. This keeps the integration easier to maintain if upstream APIs change. Let me know if that makes sense to you! |
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
|
Hi @bhess , thank you for your reply. I agree with your approach. would try to do the same and yes it requires "some" changes in copy_from_upstream.py for this to work. Let me spend some time over this and get back. |
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Hi @bhess , the first version is ready now. Pls have a look at your convenience. |
xuganyu96
left a comment
There was a problem hiding this comment.
Thank you for the contribution.
In addition to the individual code change comments I've made, I have a bigger concern with the strategy used to implement the external-mu API. From what I have read, your strategy for implementing external mu is to treat it as a separate implementation of ML-DSA whose sign and verify behave differently from the non-external-mu implementations of ML-DSA. This feels inconsistent with how liboqs intends "different implementations of the same scheme" to behave. The expectation is that the same function across different implementations of the same scheme (e.g. x86 impl of ML-DSA's sign versus aarch64's impl, versus the portable impl) should have identical external behavior, such as "passing all of the same KAT". This is not true for the "sign" and "verify" function under the external-mu implementations.
Because external-mu sign/verify have different external behaviors from non-external-mu sign-verify, I feel strongly that they should be separate functions instead of separate implementations. I am thinking of something like this:
// list of OQS API
OQS_SIG *OQS_SIG_ml_dsa_44_new(void);
OQS_API OQS_STATUS OQS_SIG_ml_dsa_44_keypair(uint8_t *public_key, uint8_t *secret_key);
OQS_API OQS_STATUS OQS_SIG_ml_dsa_44_sign(uint8_t *signature, size_t *signature_len, const uint8_t *message, size_t message_len, const uint8_t *secret_key);
OQS_API OQS_STATUS OQS_SIG_ml_dsa_44_verify(const uint8_t *message, size_t message_len, const uint8_t *signature, size_t signature_len, const uint8_t *public_key);
OQS_API OQS_STATUS OQS_SIG_ml_dsa_44_sign_with_ctx_str(uint8_t *signature, size_t *signature_len, const uint8_t *message, size_t message_len, const uint8_t *ctx, size_t ctxlen, const uint8_t *secret_key);
OQS_API OQS_STATUS OQS_SIG_ml_dsa_44_verify_with_ctx_str(const uint8_t *message, size_t message_len, const uint8_t *signature, size_t signature_len, const uint8_t *ctx, size_t ctxlen, const uint8_t *public_key);
/* ext-mu functions: replace (msg, msglen) with mu
* return OQS_ERROR if no implementation of this scheme can do external-mu
*/
OQS_API OQS_STATUS OQS_SIG_ml_dsa_44_sign_extmu(uint8_t *signature, size_t *signature_len, const uint8_t *mu, const uint8_t *secret_key);
OQS_API OQS_STATUS OQS_SIG_ml_dsa_44_verify_extmu(const uint8_t *mu, const uint8_t *signature, size_t signature_len, const uint8_t *public_key);| spec-version: ML-DSA | ||
| primary-upstream: | ||
| source: https://github.com/pq-code-package/mldsa-native/commit/f48f164cefb07f4ffa519ddda7cee670b8ee3517 | ||
| source: https://github.com/abhi-dev-engg/mldsa-native/commit/b3198696c21f4b28508ad2231e130768f622076a |
There was a problem hiding this comment.
I am not comfortable with moving the source of implementation from official PQCA repository to a personal fork.
I can see that you are trying to change the schema of the upstream META.yml file. If a change in the schema of META.yml files is necessary, I would recommend opening a separate pull request to mldsa-native.
Alternatively, you can set only the _extmu implementations to upstream from your fork, which means that in copy_from_upstream.yml there will be a separate upstream (e.g. mldsa-native-fork) from the official PQCA upstream.
There was a problem hiding this comment.
I would be creating a PR to merge this into mldsa-native once this PR is reviewed and then replacing this with the official git repo as currently done. I needed a way to test out this feature hence this change.
My bad, I should have mentioned it earlier. The PR description is updated with this info.
|
Thanks @abhi-dev-engg. I’ll take a more detailed look.
This would be Option 2 from the associated issue #2254. |
|
@abhi-dev-engg any updates by when you are planning to complete this ? I read through this and it is almost done and was waiting for mL-DSA merge which is now done. Sorry for bothering and thanks in advance |
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
sure, will try to get this done, would start on it by Fri. |
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
Signed-off-by: Abhi S <saxena_abhinav@icloud.com>
|
@Mastmeetu PR is upto date. only rebasing it against main was needed. |
Thank you so much @abhi-dev-engg for the quick work. Hi @bhess @xuganyu96 , this PR was reviewed by you earlier and approved for correctness. The pending was upstream changes which are included now. Could you please have a look and approve ? Thanks in advance! |
@abhi-dev-engg Please rebase on top of main. Thank you. |
@xuganyu96 done |
|
Confirming if this feature would be part of next release ? |
Planned for 0.17.0 release. |
Was assuming this release as you asked for a rebase. Sigh |
0.16 was already late compared to the normal release cadence. It also has a large number of major API/algorithm changes that piled on enormous amount of work for downstream projects like oqs-provider. Hence the decision to draw this admittedly arbitrary line on what to include. I understand that this pull request has been there for a while and will prioritize it as soon as 0.16 release lands. When this pull request lands, we can work out how you can check out a specific commit so you don't have to wait for 0.17. |
|
@xuganyu96 i understand. No worries at all. Thank you for your time and response |
Add External API support for "External MU" Signature generation and verification.
The following changes were done as a part of this PR: