Skip to content

Fix compiling when WOLFSSH_NO_NISTP256_MLKEM768_SHA256 is defined.#887

Open
tjko wants to merge 1 commit intowolfSSL:masterfrom
tjko:mlkem_fix
Open

Fix compiling when WOLFSSH_NO_NISTP256_MLKEM768_SHA256 is defined.#887
tjko wants to merge 1 commit intowolfSSL:masterfrom
tjko:mlkem_fix

Conversation

@tjko
Copy link
Contributor

@tjko tjko commented Mar 5, 2026

See #886

Copilot AI review requested due to automatic review settings March 5, 2026 06:59
@wolfSSL-Bot
Copy link

Can one of the admins verify this patch?

Copy link

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

Fixes a compile-time guard so KEXKEM message IDs remain available when only one of the ML-KEM768 SHA256 KEX variants is enabled (e.g., NIST P-256 disabled but Curve25519 still enabled), addressing build failures reported in #886.

Changes:

  • Update #ifndef WOLFSSH_NO_NISTP256_MLKEM768_SHA256 to a combined preprocessor condition that accounts for both NISTP256 and Curve25519 ML-KEM768 SHA256 KEX options.
  • Apply the same guard update for both MSGID_KEXKEM_INIT and MSGID_KEXKEM_REPLY.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tjko
Copy link
Contributor Author

tjko commented Mar 5, 2026

Btw, seems like this check should also include WOLFSSH_NO_NISTP384_MLKEM1024_SHA384?

@tjko tjko requested a review from Copilot March 5, 2026 16:40
Copy link

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@ejohnstown
Copy link
Contributor

Could you remove the two guard checks and then squash the PR?

Copy link

@Frauschi Frauschi left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!

@ejohnstown
Copy link
Contributor

ejohnstown commented Mar 6, 2026

This uncovered another bug. I have a fix for it. I'll need to get it merged before this.

@tjko
Copy link
Contributor Author

tjko commented Mar 6, 2026

This uncovered another bug. I have a fix for it. I'll need to get it merged before this.

I think I ran into that same issue (?) when trying to only have "mlkem768x25519-sha256" support enabled:

1969-12-31 16:00:16 [DEBUG] Decoding MSGID_KEXDH_INIT
1969-12-31 16:00:16 [DEBUG] Entering SendKexDhReply()
1969-12-31 16:00:16 [DEBUG] Using Ed25519 Host key
1969-12-31 16:00:16 [DEBUG] Entering KeyAgreeEcdhMlKem_server()
1969-12-31 16:00:17 [DEBUG] Leaving KeyAgreeEcdhMlKem_server(), ret = 0
1969-12-31 16:00:17 [DEBUG] Entering SignHEd25519()
1969-12-31 16:00:17 [DEBUG] Leaving SignHEd25519(), ret = 0
1969-12-31 16:00:17 [ERROR] cannot use FIPS KDF with ML-KEM

(update: looks like #889 fixes the issue I noticed)

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.

Compiling failing when WOLFSSH_NO_NISTP256_MLKEM768_SHA256 is defined.

5 participants