Skip to content

Conversation

@tamasan238
Copy link
Member

Description

Added below to solve trouble allocating memory for the MlKemKey structure in C#.
Since the size of the MlKemKey structure depends on the build options, these are needed.

  • wolfCrypt/wc_mlkem.c: wc_MlKemKey_New(), wc_MlKemKey_Delete()
  • wolfCrypt/wc_dilithium.c: wc_dilithium_new(), wc_dilithium_delete()

This PR is related to another PR I'll add in the comments.

I'm not familiar with adding code to wolfCrypt.
Please feel free to point out any inappropriate parts.

Testing

Tested.

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@devin-ai-integration
Copy link
Contributor

🛟 Devin Lifeguard found 1 likely issues in this PR

  • declare-const-pointers snippet snippet: Change the signatures (and corresponding header declarations) of wc_dilithium_new() and wc_MlKemKey_New() to use const void* heap instead of void* heap.

@tamasan238
please take a look at the above issues which Devin flagged. Devin will not fix these issues automatically.

@tamasan238
Copy link
Member Author

Add ML-KEM/ML-DSA support for C# wrapper
#9040

Add C# examples using ML-KEM/ML-DSA
wolfSSL/wolfssl-examples#514

@JacobBarthelmeh
Copy link
Contributor

@tamasan238 update the "assignees" to be wolfssl-bot when ready for re-review / merge.

@JacobBarthelmeh JacobBarthelmeh requested a review from Copilot July 30, 2025 16:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds memory allocation and deallocation functions for ML-KEM and ML-DSA (Dilithium) key structures to support C# bindings where the structure size depends on build options.

  • Adds wc_MlKemKey_New() and wc_MlKemKey_Delete() functions for ML-KEM
  • Adds wc_dilithium_new() and wc_dilithium_delete() functions for ML-DSA
  • Includes test coverage for the new allocation/deallocation functions

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
wolfssl/wolfcrypt/mlkem.h Adds function declarations for ML-KEM key allocation/deallocation
wolfssl/wolfcrypt/dilithium.h Adds function declarations for Dilithium key allocation/deallocation
wolfcrypt/src/wc_mlkem.c Implements ML-KEM key allocation and deallocation functions
wolfcrypt/src/dilithium.c Implements Dilithium key allocation and deallocation functions
wolfcrypt/test/test.c Adds basic tests for the new allocation/deallocation functions

@tamasan238
Copy link
Member Author

Thank you for your review!
I'll correct each item and contact again.

@tamasan238 tamasan238 force-pushed the for-pr-1 branch 2 times, most recently from e07d591 to 14b40ec Compare August 2, 2025 11:25
@tamasan238
Copy link
Member Author

  • Added macro guard using WC_NO_CONSTRUCTORS
  • Removed not-returned ret
  • (Did not change the documentation style in dilithium.c)
    • These are same as other APIs in same file.
    • If changes are necessary, I think it is better to change them all at once in other PR.

I have confirmed that some tests have failed.
I'm investigating the cause, but I don't yet know the reason.

If you have any information, I would appreciate your help.

@kojiws
Copy link
Contributor

kojiws commented Aug 5, 2025

Retest this please

@kojiws
Copy link
Contributor

kojiws commented Aug 5, 2025

@tamasan238

Retry tests removed most errors, but still one is left.
Probably, ./configure --enable-sp-math-all=521 --enable-keygen --enable-cryptonly --disable-rsa --disable-dh --enable-sp --enable-ecc --enable-opensslextra && make test can reproduce the error.

Log:

Testing DEFAULT: --enable-sp-math-all=521 --enable-keygen --enable-cryptonly --disable-rsa --disable-dh --enable-sp --enable-ecc  --enable-opensslextra
make[3]: warning: -j3 forced in submake: resetting jobserver mode.
make[3]: warning: -j3 forced in submake: resetting jobserver mode.
make[4]: warning: -j3 forced in submake: resetting jobserver mode.
�[1;32mConfigure RESULT = 0�[0m

make[2]: warning: -j3 forced in submake: resetting jobserver mode.
/usr/bin/ld: wolfcrypt/test/test.o: in function `mlkem_test':
test.c:(.text+0x12eeb): undefined reference to `wc_MlKemKey_New'
/usr/bin/ld: test.c:(.text+0x12f06): undefined reference to `wc_MlKemKey_Delete'
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:7118: wolfcrypt/test/testwolfcrypt] Error 1

@tamasan238
Copy link
Member Author

@kojiws Thank you so much!!
I'll investigate.

@tamasan238
Copy link
Member Author

I couldn't reproduce that error in my environment.
I'll retest and see how it goes.

image

Jenkins retest this please

@tamasan238
Copy link
Member Author

retest this please

@tamasan238
Copy link
Member Author

The error was “These APIs cannot be found when using liboqs.”
I enclosed the part of the test code that calls the relevant API with #ifdef WOLFSSL_WC_MLKEM.

@tamasan238
Copy link
Member Author

retest this please

@tamasan238
Copy link
Member Author

I finally passed all the tests. @kojiws Thank you!!

@JacobBarthelmeh JacobBarthelmeh merged commit e0913c4 into wolfSSL:master Aug 25, 2025
253 of 254 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants