fix: reorder wc_curve25519_make_pub/generic to input-before-output by MarkAtwood · Pull Request #10367 · wolfSSL/wolfssl

MarkAtwood · 2026-04-30T19:08:23Z

Refs: ZD-21733

The Problem

Four raw-bytes Curve25519 functions had output parameters before input parameters:

// Before (wrong):
int wc_curve25519_make_pub(int public_size, byte* pub,
                           int private_size, const byte* priv);

Every other wolfCrypt raw-bytes function uses input-before-output ordering:

wc_curve25519_import_private(const byte* priv, word32 privSz, curve25519_key* key)
wc_ed25519_make_public(ed25519_key* key, unsigned char* pubKey, word32 pubKeySz)
wc_curve25519_make_priv(WC_RNG* rng, int keysize, byte* priv)

The same wrong ordering was shared by all four related raw-bytes functions: wc_curve25519_make_pub, wc_curve25519_make_pub_blind, wc_curve25519_generic, and wc_curve25519_generic_blind.

Why It Was Not Caught

Both size parameters are int and both must equal CURVE25519_KEYSIZE (32). Transposing the arguments compiles without warning and passes the runtime size check. The clamping check (RFC 7748 §5) provides a partial accidental guard — an arbitrary 32-byte buffer will usually fail — but is not reliable. There was no test that deliberately passed arguments in the wrong order to verify the error path.

Additionally, the original author got the argument order wrong in six places in the test suite when the function was first introduced (2020-08-05), then had to push a follow-up commit the next day titled "fix order of args to wc_curve25519_make_pub()". The signature was not corrected at that point; only the test was patched.

How Fixed

Reordered parameters to private-before-public on all four functions:

// After (correct):
int wc_curve25519_make_pub(int private_size, const byte* priv,
                           int public_size, byte* pub);

Updated all five internal call sites in wolfcrypt/src/curve25519.c, the test suite, and the Doxygen doc comments and code examples.

Risk to External Callers

This is a breaking API change for any external caller that uses these four functions directly. Mitigating factors:

These are low-level raw-bytes functions; the typical application uses wc_curve25519_make_key() instead
All in-tree callers (5 sites, all in wolfcrypt/src/curve25519.c) have been updated atomically
External callers with the wrong argument order were producing wrong output silently; the fix makes their code fail to compile rather than fail silently

If desired, a deprecated wrapper preserving the old name and signature can be added alongside.

Four raw-bytes Curve25519 functions had output parameters before input parameters, opposite to every other wolfCrypt raw-bytes API: wc_curve25519_make_pub(pubSz, pub, privSz, priv) -- was wrong wc_curve25519_make_pub(privSz, priv, pubSz, pub) -- now correct Same fix applied to make_pub_blind, generic, and generic_blind. Update all five internal call sites in curve25519.c and the test suite. Update doc comments and code examples in dox_comments/. Refs: ZD-21733

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #10367

Scan targets checked: wolfcrypt-bugs, wolfcrypt-src

No new issues found in the changed files. ✅

Copilot

Pull request overview

Reorders the raw-bytes Curve25519 APIs to use input-before-output parameter ordering, aligning these low-level functions with the rest of wolfCrypt’s byte-buffer APIs and reducing the likelihood of accidental argument transposition.

Changes:

Reordered parameters for wc_curve25519_make_pub* and wc_curve25519_generic* declarations/definitions.
Updated in-tree call sites in wolfcrypt/src/curve25519.c to match the new signatures.
Updated API tests and Doxygen documentation/examples to use the new parameter order.

Reviewed changes

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

File	Description
wolfssl/wolfcrypt/curve25519.h	Updates public API prototypes to private/input first, then public/output.
wolfcrypt/src/curve25519.c	Updates function definitions and internal call sites to the new ordering.
tests/api/test_curve25519.c	Updates test calls to the new ordering; opportunity to add a regression test for swapped-args misuse.
doc/dox_comments/header_files/curve25519.h	Updates doc prototypes and examples to match the new ordering (return-code docs need alignment with implementation).

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

Copilot · 2026-04-30T19:15:54Z

+int wc_curve25519_make_pub_blind(int private_size, const byte* priv,
+                                 int public_size, byte* pub, WC_RNG* rng);


Reordering parameters on an exported C symbol changes the calling convention expected by already-compiled binaries while keeping the same symbol name. If this can ship in a shared library without a SONAME/major version bump, it can lead to silent runtime breakage (old callers will pass pub/priv swapped). Consider either providing deprecated compatibility wrappers that preserve the old signature (and introduce a new *_v2 name), or ensuring the shared-library ABI version is bumped for this release.

Suggested change

int wc_curve25519_make_pub_blind(int private_size, const byte* priv,

int public_size, byte* pub, WC_RNG* rng);

int wc_curve25519_make_pub_blind(int public_size, byte* pub,

int private_size, const byte* priv,

WC_RNG* rng);

Copilot · 2026-04-30T19:15:54Z

    \return 0 On successfully generating the public key
    \return ECC_BAD_ARG_E If the key sizes are invalid
    \return BAD_FUNC_ARG If any input parameters are NULL

-    \param public_size Size of the public key buffer (must be 32)
-    \param pub Pointer to buffer to store the public key
    \param private_size Size of the private key (must be 32)
    \param priv Pointer to buffer containing the private key
+    \param public_size Size of the public key buffer (must be 32)
+    \param pub Pointer to buffer to store the public key


The documented return codes say BAD_FUNC_ARG is returned when parameters are NULL, but the implementation returns ECC_BAD_ARG_E for NULL pub/priv in wc_curve25519_make_pub(). Please update the Doxygen \return list to match the actual behavior (or change the implementation, but that would be a behavioral/API change).

Copilot · 2026-04-30T19:15:54Z

    \return 0 On successfully generating the public key
    \return ECC_BAD_ARG_E If the key sizes are invalid
    \return BAD_FUNC_ARG If any input parameters are NULL

-    \param public_size Size of the public key buffer (must be 32)
-    \param pub Pointer to buffer to store the public key
    \param private_size Size of the private key (must be 32)
    \param priv Pointer to buffer containing the private key
+    \param public_size Size of the public key buffer (must be 32)
+    \param pub Pointer to buffer to store the public key
    \param rng Pointer to initialized RNG for blinding


The documented return codes say BAD_FUNC_ARG is returned when parameters are NULL, but the implementation of wc_curve25519_make_pub_blind() returns ECC_BAD_ARG_E for NULL pub/priv. Please update the Doxygen \return list to match the actual behavior.

Copilot · 2026-04-30T19:15:55Z

    \return 0 On successfully computing the result
    \return ECC_BAD_ARG_E If the sizes are invalid
    \return BAD_FUNC_ARG If any input parameters are NULL

-    \param public_size Size of the output buffer (must be 32)
-    \param pub Pointer to buffer to store the result
    \param private_size Size of the scalar (must be 32)
    \param priv Pointer to buffer containing the scalar
+    \param public_size Size of the output buffer (must be 32)
+    \param pub Pointer to buffer to store the result
    \param basepoint_size Size of the basepoint (must be 32)
    \param basepoint Pointer to buffer containing the basepoint


The documented return codes say BAD_FUNC_ARG is returned when parameters are NULL, but the implementation of wc_curve25519_generic() returns ECC_BAD_ARG_E for NULL pub/priv/basepoint. Please update the Doxygen \return list to match the actual behavior.

Copilot · 2026-04-30T19:15:55Z

    \return 0 On successfully computing the result
    \return ECC_BAD_ARG_E If the sizes are invalid
    \return BAD_FUNC_ARG If any input parameters are NULL

-    \param public_size Size of the output buffer (must be 32)
-    \param pub Pointer to buffer to store the result
    \param private_size Size of the scalar (must be 32)
    \param priv Pointer to buffer containing the scalar
+    \param public_size Size of the output buffer (must be 32)
+    \param pub Pointer to buffer to store the result
    \param basepoint_size Size of the basepoint (must be 32)
    \param basepoint Pointer to buffer containing the basepoint
    \param rng Pointer to initialized RNG for blinding


The documented return codes say BAD_FUNC_ARG is returned when parameters are NULL, but the implementation of wc_curve25519_generic_blind() returns ECC_BAD_ARG_E for NULL pub/priv/basepoint. Please update the Doxygen \return list to match the actual behavior.

Copilot · 2026-04-30T19:15:55Z

+    ExpectIntEQ(wc_curve25519_make_pub((int)sizeof(key.k), key.k,
+        (int)sizeof(out), out), 0);
    /* test bad cases */


To prevent regressions of the original issue (pub/priv swapped still compiles because both sizes are 32 and both pointers are byte*), add a negative test that intentionally calls wc_curve25519_make_pub() with the arguments swapped and verifies it returns an error (using a deliberately non-clamped buffer for the would-be private key).

…g order The parameter reorder (private-before-public) broke the four call sites in wrapper/rust/wolfssl-wolfcrypt/src/curve25519.rs.

github-actions · 2026-05-01T01:00:22Z

MemBrowse Memory Report

No memory changes detected for:

MarkAtwood requested review from Copilot, douzzer, wolfSSL-Bot and wolfSSL-Fenrir-bot April 30, 2026 19:08

wolfSSL-Fenrir-bot reviewed Apr 30, 2026

View reviewed changes

Copilot started reviewing on behalf of MarkAtwood April 30, 2026 19:11 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

MarkAtwood assigned douzzer Apr 30, 2026

fix: update Rust wrapper call sites for new wc_curve25519_make_pub ar…

4ab2364

…g order The parameter reorder (private-before-public) broke the four call sites in wrapper/rust/wolfssl-wolfcrypt/src/curve25519.rs.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: reorder wc_curve25519_make_pub/generic to input-before-output#10367

fix: reorder wc_curve25519_make_pub/generic to input-before-output#10367
MarkAtwood wants to merge 2 commits intowolfSSL:masterfrom
MarkAtwood:fix/curve25519-make-pub-param-order

MarkAtwood commented Apr 30, 2026

Uh oh!

wolfSSL-Fenrir-bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

github-actions Bot commented May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		int wc_curve25519_make_pub_blind(int private_size, const byte* priv,
		int public_size, byte* pub, WC_RNG* rng);

Conversation

MarkAtwood commented Apr 30, 2026

The Problem

Why It Was Not Caught

How Fixed

Risk to External Callers

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #10367

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 1, 2026

MemBrowse Memory Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants