Skip to content

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

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.

Sign up for GitHub

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

Open
MarkAtwood wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b258ed8
fix: reorder wc_curve25519_make_pub to input-before-output
MarkAtwood Apr 30, 2026
4ab2364
fix: update Rust wrapper call sites for new wc_curve25519_make_pub ar…
MarkAtwood Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 25 additions & 25 deletions doc/dox_comments/header_files/curve25519.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,19 +780,19 @@ int wc_curve25519_size(curve25519_key* 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
Comment on lines 779 to +786
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

_Example_
\code
byte priv[CURVE25519_KEYSIZE];
byte pub[CURVE25519_KEYSIZE];

// initialize priv with private key
int ret = wc_curve25519_make_pub(sizeof(pub), pub, sizeof(priv),
priv);
int ret = wc_curve25519_make_pub(sizeof(priv), priv, sizeof(pub),
pub);
if (ret != 0) {
// error generating public key
}
Expand All @@ -801,8 +801,8 @@ int wc_curve25519_size(curve25519_key* key);
\sa wc_curve25519_make_key
\sa wc_curve25519_make_pub_blind
*/
int wc_curve25519_make_pub(int public_size, byte* pub, int private_size,
const byte* priv);
int wc_curve25519_make_pub(int private_size, const byte* priv,
int public_size, byte* pub);

/*!
\ingroup Curve25519
Expand All @@ -814,10 +814,10 @@ int wc_curve25519_make_pub(int public_size, byte* pub, int private_size,
\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
Comment on lines 813 to 821
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

_Example_
Expand All @@ -828,8 +828,8 @@ int wc_curve25519_make_pub(int public_size, byte* pub, int private_size,

wc_InitRng(&rng);
// initialize priv with private key
int ret = wc_curve25519_make_pub_blind(sizeof(pub), pub,
sizeof(priv), priv, &rng);
int ret = wc_curve25519_make_pub_blind(sizeof(priv), priv,
sizeof(pub), pub, &rng);
if (ret != 0) {
// error generating public key
}
Expand All @@ -838,8 +838,8 @@ int wc_curve25519_make_pub(int public_size, byte* pub, int private_size,
\sa wc_curve25519_make_pub
\sa wc_curve25519_generic_blind
*/
int wc_curve25519_make_pub_blind(int public_size, byte* pub,
int private_size, const byte* priv,
int wc_curve25519_make_pub_blind(int private_size, const byte* priv,
int public_size, byte* pub,
WC_RNG* rng);

/*!
Expand All @@ -853,10 +853,10 @@ int wc_curve25519_make_pub_blind(int public_size, byte* pub,
\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
Comment on lines 852 to 861
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

Expand All @@ -867,8 +867,8 @@ int wc_curve25519_make_pub_blind(int public_size, byte* pub,
byte result[CURVE25519_KEYSIZE];

// initialize scalar and basepoint
int ret = wc_curve25519_generic(sizeof(result), result,
sizeof(scalar), scalar,
int ret = wc_curve25519_generic(sizeof(scalar), scalar,
sizeof(result), result,
sizeof(basepoint), basepoint);
if (ret != 0) {
// error computing result
Expand All @@ -878,9 +878,9 @@ int wc_curve25519_make_pub_blind(int public_size, byte* pub,
\sa wc_curve25519_shared_secret
\sa wc_curve25519_generic_blind
*/
int wc_curve25519_generic(int public_size, byte* pub, int private_size,
const byte* priv, int basepoint_size,
const byte* basepoint);
int wc_curve25519_generic(int private_size, const byte* priv,
int public_size, byte* pub,
int basepoint_size, const byte* basepoint);

/*!
\ingroup Curve25519
Expand All @@ -892,10 +892,10 @@ int wc_curve25519_generic(int public_size, byte* pub, int private_size,
\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
Comment on lines 891 to 901
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
Expand All @@ -909,17 +909,17 @@ int wc_curve25519_generic(int public_size, byte* pub, int private_size,

wc_InitRng(&rng);
// initialize scalar and basepoint
int ret = wc_curve25519_generic_blind(sizeof(result), result,
sizeof(scalar), scalar,
int ret = wc_curve25519_generic_blind(sizeof(scalar), scalar,
sizeof(result), result,
sizeof(basepoint), basepoint,
&rng);
\endcode

\sa wc_curve25519_generic
\sa wc_curve25519_make_pub_blind
*/
int wc_curve25519_generic_blind(int public_size, byte* pub,
int private_size, const byte* priv,
int wc_curve25519_generic_blind(int private_size, const byte* priv,
int public_size, byte* pub,
int basepoint_size, const byte* basepoint,
WC_RNG* rng);

Expand Down
26 changes: 13 additions & 13 deletions tests/api/test_curve25519.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,25 +370,25 @@ int test_wc_curve25519_make_pub(void)
ExpectIntEQ(wc_InitRng(&rng), 0);
ExpectIntEQ(wc_curve25519_make_key(&rng, CURVE25519_KEYSIZE, &key), 0);

ExpectIntEQ(wc_curve25519_make_pub((int)sizeof(out), out,
(int)sizeof(key.k), key.k), 0);
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof(key.k), key.k,
(int)sizeof(out), out), 0);
/* test bad cases */
Comment on lines +373 to 375
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof(key.k) - 1, key.k,
(int)sizeof out, out), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof out, out, (int)sizeof(key.k),
NULL), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof out - 1, out,
(int)sizeof(key.k), key.k), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof out, NULL,
(int)sizeof(key.k), key.k), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
(int)sizeof(out), out), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof(key.k), NULL,
(int)sizeof(out), out), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof(key.k), key.k,
(int)sizeof(out) - 1, out), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof(key.k), key.k,
(int)sizeof(out), NULL), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
/* verify clamping test */
key.k[0] |= ~248;
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof out, out, (int)sizeof(key.k),
key.k), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof(key.k), key.k,
(int)sizeof(out), out), WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
key.k[0] &= 248;
/* repeat the expected-to-succeed test. */
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof out, out, (int)sizeof(key.k),
key.k), 0);
ExpectIntEQ(wc_curve25519_make_pub((int)sizeof(key.k), key.k,
(int)sizeof(out), out), 0);

DoExpectIntEQ(wc_FreeRng(&rng), 0);
wc_curve25519_free(&key);
Expand Down
40 changes: 20 additions & 20 deletions wolfcrypt/src/curve25519.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ static WC_INLINE void curve25519_copy_point(byte* out, const byte* point,
* return value is propagated from curve25519() (0 on success), or
* ECC_BAD_ARG_E, and the byte vectors are little endian.
*/
int wc_curve25519_make_pub(int public_size, byte* pub, int private_size,
const byte* priv)
int wc_curve25519_make_pub(int private_size, const byte* priv,
int public_size, byte* pub)
{
int ret;
#ifdef FREESCALE_LTC_ECC
Expand Down Expand Up @@ -204,8 +204,8 @@ int wc_curve25519_make_pub(int public_size, byte* pub, int private_size,

ret = wc_InitRng(&rng);
if (ret == 0) {
ret = wc_curve25519_make_pub_blind(public_size, pub, private_size,
priv, &rng);
ret = wc_curve25519_make_pub_blind(private_size, priv, public_size,
pub, &rng);

wc_FreeRng(&rng);
}
Expand Down Expand Up @@ -283,8 +283,8 @@ static int curve25519_smul_blind(byte* rp, const byte* n, const byte* p,
}
#endif

int wc_curve25519_make_pub_blind(int public_size, byte* pub, int private_size,
const byte* priv, WC_RNG* rng)
int wc_curve25519_make_pub_blind(int private_size, const byte* priv,
int public_size, byte* pub, WC_RNG* rng)
{
int ret;
#ifdef FREESCALE_LTC_ECC
Expand Down Expand Up @@ -333,8 +333,8 @@ int wc_curve25519_make_pub_blind(int public_size, byte* pub, int private_size,
* return value is propagated from curve25519() (0 on success),
* and the byte vectors are little endian.
*/
int wc_curve25519_generic(int public_size, byte* pub,
int private_size, const byte* priv,
int wc_curve25519_generic(int private_size, const byte* priv,
int public_size, byte* pub,
int basepoint_size, const byte* basepoint)
{
#ifdef FREESCALE_LTC_ECC
Expand Down Expand Up @@ -373,7 +373,7 @@ int wc_curve25519_generic(int public_size, byte* pub,

ret = wc_InitRng(&rng);
if (ret == 0) {
ret = wc_curve25519_generic_blind(public_size, pub, private_size, priv,
ret = wc_curve25519_generic_blind(private_size, priv, public_size, pub,
basepoint_size, basepoint, &rng);

wc_FreeRng(&rng);
Expand All @@ -391,8 +391,8 @@ int wc_curve25519_generic(int public_size, byte* pub,
* return value is propagated from curve25519() (0 on success),
* and the byte vectors are little endian.
*/
int wc_curve25519_generic_blind(int public_size, byte* pub,
int private_size, const byte* priv,
int wc_curve25519_generic_blind(int private_size, const byte* priv,
int public_size, byte* pub,
int basepoint_size, const byte* basepoint,
WC_RNG* rng)
{
Expand Down Expand Up @@ -579,14 +579,14 @@ int wc_curve25519_make_key(WC_RNG* rng, int keysize, curve25519_key* key)
if (ret == 0) {
key->privSet = 1;
#ifdef WOLFSSL_CURVE25519_BLINDING
ret = wc_curve25519_make_pub_blind((int)sizeof(key->p.point),
key->p.point, (int)sizeof(key->k), key->k, rng);
ret = wc_curve25519_make_pub_blind((int)sizeof(key->k),
key->k, (int)sizeof(key->p.point), key->p.point, rng);
if (ret == 0) {
ret = wc_curve25519_set_rng(key, rng);
}
#else
ret = wc_curve25519_make_pub((int)sizeof(key->p.point),
key->p.point, (int)sizeof(key->k), key->k);
ret = wc_curve25519_make_pub((int)sizeof(key->k),
key->k, (int)sizeof(key->p.point), key->p.point);
#endif
key->pubSet = (ret == 0);
}
Expand Down Expand Up @@ -805,12 +805,12 @@ int wc_curve25519_export_public_ex(curve25519_key* key, byte* out,
/* calculate public if missing */
if (!key->pubSet) {
#ifdef WOLFSSL_CURVE25519_BLINDING
ret = wc_curve25519_make_pub_blind((int)sizeof(key->p.point),
key->p.point, (int)sizeof(key->k),
key->k, key->rng);
ret = wc_curve25519_make_pub_blind((int)sizeof(key->k),
key->k, (int)sizeof(key->p.point),
key->p.point, key->rng);
#else
ret = wc_curve25519_make_pub((int)sizeof(key->p.point), key->p.point,
(int)sizeof(key->k), key->k);
ret = wc_curve25519_make_pub((int)sizeof(key->k), key->k,
(int)sizeof(key->p.point), key->p.point);
#endif
key->pubSet = (ret == 0);
}
Expand Down
16 changes: 8 additions & 8 deletions wolfssl/wolfcrypt/curve25519.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,22 +155,22 @@ enum {
};

WOLFSSL_API
int wc_curve25519_make_pub(int public_size, byte* pub, int private_size,
const byte* priv);
int wc_curve25519_make_pub(int private_size, const byte* priv,
int public_size, byte* pub);
#ifdef WOLFSSL_CURVE25519_BLINDING
WOLFSSL_API
int wc_curve25519_make_pub_blind(int public_size, byte* pub, int private_size,
const byte* priv, WC_RNG* rng);
int wc_curve25519_make_pub_blind(int private_size, const byte* priv,
int public_size, byte* pub, WC_RNG* rng);
Comment on lines +162 to +163
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
#endif

WOLFSSL_API
int wc_curve25519_generic(int public_size, byte* pub,
int private_size, const byte* priv,
int wc_curve25519_generic(int private_size, const byte* priv,
int public_size, byte* pub,
int basepoint_size, const byte* basepoint);
#ifdef WOLFSSL_CURVE25519_BLINDING
WOLFSSL_API
int wc_curve25519_generic_blind(int public_size, byte* pub,
int private_size, const byte* priv,
int wc_curve25519_generic_blind(int private_size, const byte* priv,
int public_size, byte* pub,
int basepoint_size, const byte* basepoint,
WC_RNG* rng);
#endif
Expand Down
Loading