Skip to content

Ensure ECCSI SSK is in [1, q-1]. Fix multiple issues in TI/Xilinx AES. Update base16/64/blake2b documentation. #11902

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
kareem-wolfssl wants to merge 4 commits into
base: master
Choose a base branch
from
+176 −27
Open

Ensure ECCSI SSK is in [1, q-1]. Fix multiple issues in TI/Xilinx AES. Update base16/64/blake2b documentation. #11902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
75ee1ed
Update base16/64/blake2b documentation.
kareem-wolfssl Oct 26, 2026
c5066d2
Fix some return code issues in TI AES support.
kareem-wolfssl Oct 27, 2026
42a7ee8
Ensure SSK is in [1, q-1].
kareem-wolfssl Oct 27, 2026
2777b74
Code review feedback
kareem-wolfssl Oct 27, 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
4 changes: 3 additions & 1 deletion doc/dox_comments/header_files/blake2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ int wc_Blake2bUpdate(Blake2b* b2b, const byte* data, word32 sz);
each piece of input data desired.

\return 0 Returned upon successfully computing the Blake2b hash
\return -1 Returned if there is a failure while parsing the Blake2b hash
\return BAD_FUNC_ARG Returned if b2b or final is NULL, or the digest
size is 0 or larger than BLAKE2B_OUTBYTES
\return MEMORY_E Returned if memory allocation fails

\param b2b pointer to the Blake2b structure to update
\param final pointer to a buffer in which to store the blake2b hash.
Expand Down
21 changes: 12 additions & 9 deletions doc/dox_comments/header_files/coding.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
written to the output buffer in the variable outLen.

\return 0 Returned upon successfully decoding the Base64 encoded input
\return BAD_FUNC_ARG Returned if the output buffer is too small to
\return BUFFER_E Returned if the output buffer is too small to
store the decoded input
\return ASN_INPUT_E Returned if a character in the input buffer falls
outside of the Base64 range ([A-Za-z0-9+/=]) or if there is an invalid
line ending in the Base64 encoded input
outside of the Base64 range ([A-Za-z0-9+/=]) and is not a space, CR or
LF, or if there is an invalid line ending in the Base64 encoded input

\param in pointer to the input buffer to decode
\param inLen length of the input buffer to decode
Expand Down Expand Up @@ -171,8 +171,10 @@ int Base64_Encode_NoNl(const byte* in, word32 inLen, byte* out,
to the output buffer in the variable outLen.

\return 0 Returned upon successfully decoding the Base16 encoded input
\return BAD_FUNC_ARG Returned if the output buffer is too small to store
the decoded input or if the input length is not a multiple of two
\return BAD_FUNC_ARG Returned if the input length is odd, except a
single hex digit, which decodes to one byte holding its value
\return BUFFER_E Returned if the output buffer is too small to store
the decoded input
\return ASN_INPUT_E Returned if a character in the input buffer falls
outside of the Base16 range ([0-9A-F])

Expand Down Expand Up @@ -244,11 +246,12 @@ int Base16_Encode(const byte* in, word32 inLen, byte* out, word32* outLen);
timing attacks are not a concern.

\return 0 On successfully decoding the Base64 encoded input.
\return BAD_FUNC_ARG If the output buffer is too small to store the
decoded input.
\return BAD_FUNC_ARG If out or outLen is NULL.
\return ASN_INPUT_E If a character in the input buffer falls outside
of the Base64 range or if there is an invalid line ending.
\return BUFFER_E If running out of buffer while decoding.
of the Base64 range and is not a space, CR or LF, or if there is an
invalid line ending.
\return BUFFER_E If the output buffer is too small to store the
decoded input.

\param in pointer to the input buffer to decode
\param inLen length of the input buffer to decode
Expand Down
68 changes: 68 additions & 0 deletions tests/api/test_eccsi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,3 +860,71 @@ int test_wc_Eccsi_FeatureCoverage(void)
#endif
return EXPECT_RESULT();
}

int test_wc_Eccsi_DecodeSskRange(void)
{
EXPECT_DECLS;
#ifdef WOLFCRYPT_HAVE_ECCSI
EccsiKey key;
mp_int ssk;
ecc_point* pvt = NULL;
/* P-256 order q */
static const byte q[32] = {
0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xBC, 0xE6, 0xFA, 0xAD, 0xA7, 0x17, 0x9E, 0x84,
0xF3, 0xB9, 0xCA, 0xC2, 0xFC, 0x63, 0x25, 0x51
};
byte buf[32 * 3];

XMEMSET(&key, 0, sizeof(key));
ExpectIntEQ(wc_InitEccsiKey(&key, HEAP_HINT, INVALID_DEVID), 0);
ExpectIntEQ(mp_init(&ssk), MP_OKAY);
ExpectNotNull(pvt = wc_ecc_new_point());

XMEMSET(buf, 0, sizeof(buf));
buf[31] = 1;
ExpectIntEQ(wc_DecodeEccsiSsk(&key, buf, 32, &ssk), 0);
ExpectIntEQ(mp_cmp_d(&ssk, 1), MP_EQ);
ExpectIntEQ(wc_DecodeEccsiPair(&key, buf, sizeof(buf), &ssk, pvt), 0);
ExpectIntEQ(wc_SetEccsiPair(&key, &ssk, pvt), 0);

XMEMCPY(buf, q, sizeof(q));
buf[31]--;
ExpectIntEQ(wc_DecodeEccsiSsk(&key, buf, 32, &ssk), 0);

buf[31]++;
ExpectIntEQ(wc_DecodeEccsiSsk(&key, buf, 32, &ssk),
WC_NO_ERR_TRACE(ECC_PRIV_KEY_E));
ExpectIntEQ(wc_DecodeEccsiPair(&key, buf, sizeof(buf), &ssk, pvt),
WC_NO_ERR_TRACE(ECC_PRIV_KEY_E));
ExpectIntEQ(mp_read_unsigned_bin(&ssk, q, sizeof(q)), MP_OKAY);
ExpectIntEQ(wc_SetEccsiPair(&key, &ssk, pvt),
WC_NO_ERR_TRACE(ECC_PRIV_KEY_E));

XMEMSET(buf, 0xFF, 32);
ExpectIntEQ(wc_DecodeEccsiSsk(&key, buf, 32, &ssk),
WC_NO_ERR_TRACE(ECC_PRIV_KEY_E));

XMEMSET(buf, 0, 32);
ExpectIntEQ(wc_DecodeEccsiSsk(&key, buf, 32, &ssk),
WC_NO_ERR_TRACE(ECC_PRIV_KEY_E));
ExpectIntEQ(wc_DecodeEccsiPair(&key, buf, sizeof(buf), &ssk, pvt),
WC_NO_ERR_TRACE(ECC_PRIV_KEY_E));
mp_zero(&ssk);
ExpectIntEQ(wc_SetEccsiPair(&key, &ssk, pvt),
WC_NO_ERR_TRACE(ECC_PRIV_KEY_E));
#if !defined(WOLFSSL_SP_MATH) && (!defined(WOLFSSL_SP_MATH_ALL) || \
defined(WOLFSSL_SP_INT_NEGATIVE))
ExpectIntEQ(mp_read_radix(&ssk, "-1", MP_RADIX_HEX), MP_OKAY);
ExpectIntEQ(mp_isneg(&ssk), MP_YES);
ExpectIntEQ(wc_SetEccsiPair(&key, &ssk, pvt),
WC_NO_ERR_TRACE(ECC_PRIV_KEY_E));
#endif

wc_ecc_del_point(pvt);
mp_free(&ssk);
wc_FreeEccsiKey(&key);
#endif
return EXPECT_RESULT();
}
4 changes: 3 additions & 1 deletion tests/api/test_eccsi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@

int test_wc_Eccsi_DecisionCoverage(void);
int test_wc_Eccsi_FeatureCoverage(void);
int test_wc_Eccsi_DecodeSskRange(void);

#define TEST_ECCSI_DECLS \
TEST_DECL_GROUP("eccsi", test_wc_Eccsi_DecisionCoverage), \
TEST_DECL_GROUP("eccsi", test_wc_Eccsi_FeatureCoverage)
TEST_DECL_GROUP("eccsi", test_wc_Eccsi_FeatureCoverage), \
TEST_DECL_GROUP("eccsi", test_wc_Eccsi_DecodeSskRange)

#endif /* WOLFCRYPT_TEST_ECCSI_H */
39 changes: 39 additions & 0 deletions wolfcrypt/src/eccsi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,33 @@ static int eccsi_load_order(EccsiKey* key)
return err;
}

/* Check SSK is in [1, q-1]. */
static int eccsi_check_ssk(const EccsiKey* key, const mp_int* ssk)
{
int err = 0;
WC_DECLARE_VAR(order, mp_int, 1, key->heap);

if (mp_iszero(ssk) || mp_isneg(ssk)) {
return ECC_PRIV_KEY_E;
}

Comment on lines +208 to +213

@claude claude Bot Oct 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 (optional) Callers of wc_SetEccsiPair can still install a negative SSK, which is outside the [1, q-1] range this change is meant to enforce. eccsi_check_ssk only rejects zero and values where mp_cmp(ssk, order) != MP_LT. A negative mp_int compares less than q, so it passes. Fix: in eccsi_check_ssk, reject mp_iszero(ssk) || mp_isneg(ssk) before the order comparison. mp_isneg is defined for sp_int, tfm and integer, so the check builds under every math backend.

Why this was flagged

wc_SetEccsiPair (eccsi.c:1785) takes a caller-supplied const mp_int* ssk. When the math library supports sign (WOLFSSL_SP_INT_NEGATIVE, e.g. with OPENSSL_EXTRA, or the tfm/integer backends), the mp_int can be negative. eccsi_check_ssk (eccsi.c:205) checks mp_iszero, then mp_cmp((mp_int*)ssk, order) != MP_LT. A negative value is MP_LT the positive order, so the check returns 0. mp_copy then stores it in key->ssk and signing uses it. The decode paths read unsigned bytes and cannot produce a negative value, so wc_SetEccsiPair is the only entry that gets past the new range check. The PR states the goal is to ensure SSK is in [1, q-1], and this input is outside that range yet accepted.

Verification: The new eccsi_check_ssk (wolfcrypt/src/eccsi.c:205-229) has only two tests. With WOLFSSL_SP_INT_NEGATIVE, _sp_cmp (wolfcrypt/src/sp_int.c:5821-5826) compares signs first. So any negative ssk compares MP_LT against the positive order and passes. For example, ssk = -q passes the check but acts as a zero signing key. The severity is low because the base branch did no range check at all.

WC_ALLOC_VAR_EX(order, mp_int, 1, key->heap, DYNAMIC_TYPE_ECC,
err = MEMORY_E);
if (err == 0) {
err = mp_init(order);
}
if (err == 0) {
err = mp_read_radix(order, key->ecc.dp->order, MP_RADIX_HEX);
if ((err == 0) && (mp_cmp(ssk, order) != MP_LT)) {
err = ECC_PRIV_KEY_E;
}
mp_clear(order);
}
WC_FREE_VAR_EX(order, key->heap, DYNAMIC_TYPE_ECC);

return err;
}

#ifdef WOLFCRYPT_ECCSI_CLIENT
/*
* Parameters, as a hex strings in the ECC object, loaded into mp_ints in key.
Expand Down Expand Up @@ -1126,6 +1153,7 @@ int wc_EncodeEccsiSsk(const EccsiKey* key, mp_int* ssk, byte* data, word32* sz)
* @return 0 on success.
* @return BAD_FUNC_ARG when key, data or ssk is NULL.
* @return BUFFER_E when size of data is not equal to the expected size.
* @return ECC_PRIV_KEY_E when SSK is not in [1, q-1].
* @return MP_MEM or MEMORY_E when dynamic memory allocation fails.
*/
int wc_DecodeEccsiSsk(const EccsiKey* key, const byte* data, word32 sz,
Expand All @@ -1143,6 +1171,9 @@ int wc_DecodeEccsiSsk(const EccsiKey* key, const byte* data, word32 sz,
if (err == 0) {
err = mp_read_unsigned_bin(ssk, data, (word32)key->ecc.dp->size);
}
if (err == 0) {
err = eccsi_check_ssk(key, ssk);
}

return err;
}
Expand Down Expand Up @@ -1202,6 +1233,7 @@ int wc_EncodeEccsiPvt(const EccsiKey* key, ecc_point* pvt, byte* data,
* @return BAD_FUNC_ARG when key, data, ssk or pvt is NULL.
* @return LENGTH_ONLY_E when data is NULL - sz is set.
* @return BUFFER_E when size of data is not equal to the expected size.
* @return ECC_PRIV_KEY_E when SSK is not in [1, q-1].
* @return MP_MEM or MEMORY_E when dynamic memory allocation fails.
*/
int wc_DecodeEccsiPair(const EccsiKey* key, const byte* data, word32 sz,
Expand All @@ -1220,6 +1252,9 @@ int wc_DecodeEccsiPair(const EccsiKey* key, const byte* data, word32 sz,
/* Read the SSK value from key size bytes. */
err = mp_read_unsigned_bin(ssk, data, (word32)key->ecc.dp->size);
}
if (err == 0) {
err = eccsi_check_ssk(key, ssk);
}
if (err == 0) {
data += key->ecc.dp->size;
/* Read the PVT's x value from key size bytes. */
Expand Down Expand Up @@ -1736,6 +1771,7 @@ int wc_SetEccsiHash(EccsiKey* key, const byte* hash, byte hashSz)
* @param [in] pvt Public Validation Token (PVT) as an ECC point.
* @return 0 on success.
* @return BAD_FUNC_ARG when key, ssk or pvt is NULL.
* @return ECC_PRIV_KEY_E when SSK is not in [1, q-1].
* @return MP math errors when copy fails
*/
int wc_SetEccsiPair(EccsiKey* key, const mp_int* ssk, const ecc_point* pvt)
Expand All @@ -1746,6 +1782,9 @@ int wc_SetEccsiPair(EccsiKey* key, const mp_int* ssk, const ecc_point* pvt)
err = BAD_FUNC_ARG;
}

if (err == 0) {
err = eccsi_check_ssk(key, ssk);
}
if (err == 0) {
err = mp_copy(ssk, &key->ssk);
}
Expand Down
57 changes: 43 additions & 14 deletions wolfcrypt/src/port/ti/ti-aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ int wc_AesGetKeySize(Aes* aes, word32* keySize)
static int AesAlign16(Aes* aes, byte* out, const byte* in, word32 sz,
word32 dir, word32 mode)
{
bool ok;

/* Processed aligned chunk to HW AES */
wolfSSL_TI_lockCCM();
ROM_AESReset(AES_BASE);
Expand All @@ -160,9 +162,12 @@ static int AesAlign16(Aes* aes, byte* out, const byte* in, word32 sz,
/* if input and output same will overwrite input iv */
XMEMCPY(aes->tmp, in + sz - WC_AES_BLOCK_SIZE, WC_AES_BLOCK_SIZE);
}
ROM_AESDataProcess(AES_BASE, (uint32_t *)in, (uint32_t *)out, sz);
ok = ROM_AESDataProcess(AES_BASE, (uint32_t *)in, (uint32_t *)out, sz);
wolfSSL_TI_unlockCCM();

if (ok == false)
return WC_HW_E;

/* store iv for next call */
if (mode == AES_CFG_MODE_CBC) {
if (dir == AES_CFG_DIR_ENCRYPT)
Expand All @@ -182,7 +187,7 @@ static int AesAlign16(Aes* aes, byte* out, const byte* in, word32 sz,
} while ((int)sz > 0);
}

return true;
return 0;
}

static int AesProcess(Aes* aes, byte* out, const byte* in, word32 sz,
Expand All @@ -191,6 +196,7 @@ static int AesProcess(Aes* aes, byte* out, const byte* in, word32 sz,
const byte *in_p; byte *out_p;
word32 size;
byte buff[TI_BUFFSIZE];
int ret;

if ((aes == NULL) || (in == NULL) || (out == NULL))
return BAD_FUNC_ARG;
Expand All @@ -209,7 +215,9 @@ static int AesProcess(Aes* aes, byte* out, const byte* in, word32 sz,
out_p = buff;
}

AesAlign16(aes, out_p, in_p, size, dir, mode);
ret = AesAlign16(aes, out_p, in_p, size, dir, mode);
if (ret != 0)
return ret;

if (!IS_ALIGN16(out)) {
XMEMCPY(out, buff, size);
Expand Down Expand Up @@ -415,7 +423,7 @@ static int AesAuthArgCheck(Aes* aes, byte* out, const byte* in, word32 inSz,
return 0;
}

static void AesAuthSetIv(Aes *aes, const byte *nonce, word32 len, word32 L,
static int AesAuthSetIv(Aes *aes, const byte *nonce, word32 len, word32 L,
int mode)
{
if (mode == AES_CFG_MODE_CCM) {
Expand Down Expand Up @@ -465,7 +473,11 @@ static void AesAuthSetIv(Aes *aes, const byte *nonce, word32 len, word32 L,
ROM_AESReset(AES_BASE);
ROM_AESConfigSet(AES_BASE, (aes->keylen-8) | AES_CFG_DIR_ENCRYPT | AES_CFG_MODE_ECB);
ROM_AESKey1Set(AES_BASE, aes->key, (aes->keylen-8));
ROM_AESDataProcess(AES_BASE, zeros, subkey, sizeof zeros);
if (ROM_AESDataProcess(AES_BASE, zeros, subkey, sizeof zeros) ==
false) {
wolfSSL_TI_unlockCCM();
return WC_HW_E;
}

/* GHASH */
ROM_AESReset(AES_BASE);
Expand All @@ -489,6 +501,8 @@ static void AesAuthSetIv(Aes *aes, const byte *nonce, word32 len, word32 L,
wolfSSL_TI_unlockCCM();
}
}

return 0;
}

/* Allocates a 16 byte aligned, whole block sized temporary for sz bytes and,
Expand Down Expand Up @@ -587,7 +601,10 @@ static int AesAuthEncrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
return BAD_FUNC_ARG;
}

AesAuthSetIv(aes, nonce, nonceSz, L, mode);
ret = AesAuthSetIv(aes, nonce, nonceSz, L, mode);
if (ret != 0) {
return ret;
}

if (inSz == 0 && authInSz == 0) {
#ifdef HAVE_AESCCM
Expand All @@ -606,10 +623,15 @@ static int AesAuthEncrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
ROM_AESReset(AES_BASE);
ROM_AESConfigSet(AES_BASE, (aes->keylen-8) | AES_CFG_DIR_ENCRYPT | AES_CFG_MODE_ECB);
ROM_AESKey1Set(AES_BASE, aes->key, (aes->keylen-8));
ROM_AESDataProcess(AES_BASE, aes->reg, tmpTag, WC_AES_BLOCK_SIZE);
if (ROM_AESDataProcess(AES_BASE, aes->reg, tmpTag,
WC_AES_BLOCK_SIZE) == false) {
ret = WC_HW_E;
}
wolfSSL_TI_unlockCCM();
XMEMCPY(authTag, tmpTag, authTagSz);
return 0;
if (ret == 0) {
XMEMCPY(authTag, tmpTag, authTagSz);
}
return ret;
}

/* Bounce each buffer that is not 16 byte aligned, and each buffer whose
Expand Down Expand Up @@ -663,7 +685,7 @@ static int AesAuthEncrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
if (out != NULL)
ForceZero(out, inSz);
ForceZero(authTag, authTagSz);
ret = AES_GCM_AUTH_E;
ret = (mode == AES_CFG_MODE_CCM) ? AES_CCM_AUTH_E : AES_GCM_AUTH_E;
}
else {
if (out_save != NULL)
Expand Down Expand Up @@ -708,7 +730,10 @@ static int AesAuthDecrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
return BAD_FUNC_ARG;
}

AesAuthSetIv(aes, nonce, nonceSz, L, mode);
ret = AesAuthSetIv(aes, nonce, nonceSz, L, mode);
if (ret != 0) {
return ret;
}

if (inSz == 0 && authInSz == 0) {
#ifdef HAVE_AESCCM
Expand All @@ -729,10 +754,14 @@ static int AesAuthDecrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
ROM_AESReset(AES_BASE);
ROM_AESConfigSet(AES_BASE, (aes->keylen-8) | AES_CFG_DIR_ENCRYPT | AES_CFG_MODE_ECB);
ROM_AESKey1Set(AES_BASE, aes->key, (aes->keylen-8));
ROM_AESDataProcess(AES_BASE, aes->reg, tmpTag, WC_AES_BLOCK_SIZE);
if (ROM_AESDataProcess(AES_BASE, aes->reg, tmpTag,
WC_AES_BLOCK_SIZE) == false) {
ret = WC_HW_E;
}
wolfSSL_TI_unlockCCM();

if (ConstantCompare(authTag, (byte*)tmpTag, authTagSz) != 0) {
if ((ret == 0) &&
(ConstantCompare(authTag, (byte*)tmpTag, authTagSz) != 0)) {
ret = AES_GCM_AUTH_E;
}
return ret;
Expand Down Expand Up @@ -788,7 +817,7 @@ static int AesAuthDecrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
/* out is NULL for the GMAC case, where inSz is zero. */
if (out != NULL)
ForceZero(out, inSz);
ret = AES_GCM_AUTH_E;
ret = (mode == AES_CFG_MODE_CCM) ? AES_CCM_AUTH_E : AES_GCM_AUTH_E;
}
else {
if (out_save != NULL)
Expand Down
Loading
Loading