Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add wolfcrypt ML-DSA (dilithium) support. #507

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

philljj
Copy link
Contributor

@philljj philljj commented Sep 21, 2024

Description

Adds wolfcrypt ML-DSA (wolfcrypt/src/dilithium.c) support to wolfBoot.

Config

Added sim ML-DSA example:

  • config/examples/sim-ml-dsa.config

Use ML_DSA_LEVEL=<num> to configure the ML-DSA parameter set. See the config example and updated PQ docs for explanation of parameters.

Docs

Updated docs/PQ.md to describe ML-DSA and all three NIST approved PQ sig algs we have implemented.

Testing

  • added renode-nrf52 ML-DSA-44 test to github workflow
  • built nrf52
  • built and booted sim

Note: wolfssl needs to be on master to pickup the supporting PRs. The ML-DSA renode test checks out master and pulls. If we want to update the lib/wolfssl submodule I can change this.

Prereqs

Requires this PR to build:

Requires this PR to conform to FIPS 204:

@philljj philljj self-assigned this Sep 21, 2024
src/image.c Outdated
if (ret != 0) {
wolfBoot_printf("error: wc_MlDsaKey_SetParams(%d)" \
" returned %d\n", ML_DSA_LEVEL, ret);
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer a single return flow to make sure wc_MlDsaKey_Free(&ml_dsa); gets called. Refactor function with if (ret == 0) logic and only have a single return at bottom.

ifeq ($(SIGN),ML_DSA)
OBJS_REAL+=$(WOLFDIR)/wolfcrypt/src/dilithium.o

CFLAGS += -D"WOLFSSL_DILITHIUM_FIPS204_DRAFT" \
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer these build options reside in tools/keytools/user_settings.h. Please move this and others if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll move these, agree will be tidier


if (ret == 0) {
/* Now import pub key. */
ret = wc_MlDsaKey_ImportPubRaw(&ml_dsa, pubkey, pub_len);
Copy link
Contributor

Choose a reason for hiding this comment

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

davidgarske@Davids-MacBook-Pro-2 wolfBoot-alt % make
        [CC-sim] src/string.o
        [CC-sim] src/image.o
src/image.c:542:15: error: use of undeclared identifier 'out'
  542 |         ret = wc_MlDsaKey_ImportPubRaw(&ml_dsa, pubkey, pub_len);
      |               ^
lib/wolfssl/wolfssl/wolfcrypt/dilithium.h:753:32: note: expanded from macro 'wc_MlDsaKey_ImportPubRaw'
  753 |     wc_dilithium_import_public(out, outLen, key)
      |                                ^
src/image.c:542:15: error: use of undeclared identifier 'outLen'
lib/wolfssl/wolfssl/wolfcrypt/dilithium.h:753:37: note: expanded from macro 'wc_MlDsaKey_ImportPubRaw'
  753 |     wc_dilithium_import_public(out, outLen, key)
      |                                     ^
2 errors generated.
make: *** [src/image.o] Error 1

Copy link
Contributor

Choose a reason for hiding this comment

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

cp config/examples/sim-ml-dsa.config .config
make distclean
make

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dgarske dgarske removed their assignment Sep 25, 2024
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.

3 participants