Skip to content

Commit b72a8ed

Browse files
author
Emma Stensland
committed
Fix ML-DSA missing public key guards and memory allocation, add ECC pubkey derivation logs
1 parent 4c68523 commit b72a8ed

8 files changed

Lines changed: 803 additions & 181 deletions

File tree

‎tests/api/test_asn.c‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2481,3 +2481,81 @@ int test_wc_AsnFeatureCoverage(void)
24812481
#endif /* !NO_ASN && HAVE_ECC && USE_CERT_BUFFERS_256 && !HAVE_FIPS */
24822482
return EXPECT_RESULT();
24832483
}
2484+
2485+
/* wc_EccPrivateKeyDecode should derive and cache the public point (best
2486+
* effort) when it decodes a SEC1 private key whose optional public point
2487+
* was omitted, so the key comes out fully usable. */
2488+
int test_wc_EccPrivateKeyDecode_derive_pub(void)
2489+
{
2490+
EXPECT_DECLS;
2491+
#if !defined(NO_ASN) && defined(HAVE_ECC) && !defined(NO_ECC_MAKE_PUB) && \
2492+
defined(USE_CERT_BUFFERS_256) && !defined(HAVE_FIPS) && \
2493+
!defined(WOLFSSL_ATECC508A) && !defined(WOLFSSL_ATECC608A) && \
2494+
!defined(WOLFSSL_MICROCHIP_TA100) && !defined(WOLFSSL_CRYPTOCELL) && \
2495+
!defined(WOLFSSL_SILABS_SE_ACCEL) && !defined(WOLFSSL_KCAPI_ECC) && \
2496+
!defined(WOLFSSL_QNX_CAAM) && !defined(WOLFSSL_IMXRT1170_CAAM)
2497+
ecc_key fullKey;
2498+
ecc_key privOnlyKey;
2499+
WC_RNG rng;
2500+
word32 idx;
2501+
byte privOnlyDer[256];
2502+
int privOnlyDerSz;
2503+
byte fullPub[256];
2504+
word32 fullPubSz = sizeof(fullPub);
2505+
byte derivedPub[256];
2506+
word32 derivedPubSz = sizeof(derivedPub);
2507+
2508+
XMEMSET(&fullKey, 0, sizeof(fullKey));
2509+
XMEMSET(&privOnlyKey, 0, sizeof(privOnlyKey));
2510+
2511+
ExpectIntEQ(wc_InitRng(&rng), 0);
2512+
2513+
ExpectIntEQ(wc_ecc_init(&fullKey), 0);
2514+
idx = 0;
2515+
ExpectIntEQ(wc_EccPrivateKeyDecode(ecc_clikey_der_256, &idx, &fullKey,
2516+
sizeof_ecc_clikey_der_256), 0);
2517+
ExpectIntEQ(fullKey.type, ECC_PRIVATEKEY);
2518+
PRIVATE_KEY_UNLOCK();
2519+
ExpectIntEQ(wc_ecc_export_x963(&fullKey, fullPub, &fullPubSz), 0);
2520+
PRIVATE_KEY_LOCK();
2521+
2522+
/* Re-encode as a private-key-only SEC1 DER (no public point). */
2523+
ExpectIntGT(privOnlyDerSz = wc_EccPrivateKeyToDer(&fullKey, privOnlyDer,
2524+
sizeof(privOnlyDer)), 0);
2525+
2526+
#ifdef ECC_TIMING_RESISTANT
2527+
/* Without an RNG set, decode must not derive unblinded: the key is
2528+
* left as ECC_PRIVATEKEY_ONLY, same as before auto-derivation existed. */
2529+
ExpectIntEQ(wc_ecc_init(&privOnlyKey), 0);
2530+
idx = 0;
2531+
ExpectIntEQ(wc_EccPrivateKeyDecode(privOnlyDer, &idx, &privOnlyKey,
2532+
(word32)privOnlyDerSz), 0);
2533+
ExpectIntEQ(privOnlyKey.type, ECC_PRIVATEKEY_ONLY);
2534+
wc_ecc_free(&privOnlyKey);
2535+
#endif
2536+
2537+
/* Opting into blinding (as every other blinded ECC op in this library
2538+
* requires) makes decode derive and cache the public point. */
2539+
ExpectIntEQ(wc_ecc_init(&privOnlyKey), 0);
2540+
ExpectIntEQ(wc_ecc_set_rng(&privOnlyKey, &rng), 0);
2541+
idx = 0;
2542+
ExpectIntEQ(wc_EccPrivateKeyDecode(privOnlyDer, &idx, &privOnlyKey,
2543+
(word32)privOnlyDerSz), 0);
2544+
2545+
/* The public point should have been derived automatically, making the
2546+
* key fully usable rather than left as ECC_PRIVATEKEY_ONLY. */
2547+
ExpectIntEQ(privOnlyKey.type, ECC_PRIVATEKEY);
2548+
PRIVATE_KEY_UNLOCK();
2549+
ExpectIntEQ(wc_ecc_export_x963(&privOnlyKey, derivedPub, &derivedPubSz),
2550+
0);
2551+
PRIVATE_KEY_LOCK();
2552+
ExpectIntEQ(derivedPubSz, fullPubSz);
2553+
ExpectBufEQ(derivedPub, fullPub, fullPubSz);
2554+
2555+
wc_ecc_free(&privOnlyKey);
2556+
wc_ecc_free(&fullKey);
2557+
wc_FreeRng(&rng);
2558+
#endif /* !NO_ASN && HAVE_ECC && !NO_ECC_MAKE_PUB && USE_CERT_BUFFERS_256 &&
2559+
* !HAVE_FIPS */
2560+
return EXPECT_RESULT();
2561+
}

‎tests/api/test_asn.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ int test_ToTraditional_ex_negative(void);
4444
int test_ToTraditional_ex_mldsa_bad_params(void);
4545
int test_wc_AsnDecisionCoverage(void);
4646
int test_wc_AsnFeatureCoverage(void);
47+
int test_wc_EccPrivateKeyDecode_derive_pub(void);
4748

4849
#define TEST_ASN_DECLS \
4950
TEST_DECL_GROUP("asn", test_SetAsymKeyDer), \
@@ -65,6 +66,7 @@ int test_wc_AsnFeatureCoverage(void);
6566
TEST_DECL_GROUP("asn", test_ToTraditional_ex_negative), \
6667
TEST_DECL_GROUP("asn", test_ToTraditional_ex_mldsa_bad_params), \
6768
TEST_DECL_GROUP("asn", test_wc_AsnDecisionCoverage), \
68-
TEST_DECL_GROUP("asn", test_wc_AsnFeatureCoverage)
69+
TEST_DECL_GROUP("asn", test_wc_AsnFeatureCoverage), \
70+
TEST_DECL_GROUP("asn", test_wc_EccPrivateKeyDecode_derive_pub)
6971

7072
#endif /* WOLFCRYPT_TEST_ASN_H */

‎tests/api/test_mldsa.c‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7763,6 +7763,179 @@ int test_mldsa_make_key_from_seed(void)
77637763
return EXPECT_RESULT();
77647764
}
77657765

7766+
int test_mldsa_make_public_key(void)
7767+
{
7768+
EXPECT_DECLS;
7769+
#if defined(WOLFSSL_HAVE_MLDSA) && defined(WOLFSSL_MLDSA_PRIVATE_KEY) && \
7770+
!defined(WOLFSSL_MLDSA_ASSIGN_KEY) && !defined(WOLFSSL_MLDSA_NO_MAKE_KEY)
7771+
wc_MlDsaKey* key;
7772+
7773+
key = (wc_MlDsaKey*)XMALLOC(sizeof(*key), NULL, DYNAMIC_TYPE_TMP_BUFFER);
7774+
ExpectNotNull(key);
7775+
if (key != NULL) {
7776+
XMEMSET(key, 0, sizeof(*key));
7777+
}
7778+
7779+
/* NULL key. */
7780+
ExpectIntEQ(wc_MlDsaKey_MakePublicKey(NULL),
7781+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
7782+
7783+
#ifndef WOLFSSL_NO_ML_DSA_44
7784+
ExpectIntEQ(wc_MlDsaKey_Init(key, NULL, INVALID_DEVID), 0);
7785+
ExpectIntEQ(wc_MlDsaKey_SetParams(key, WC_ML_DSA_44), 0);
7786+
7787+
/* Private key not set yet. */
7788+
ExpectIntEQ(wc_MlDsaKey_MakePublicKey(key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
7789+
7790+
/* Import a known private-only key (no public key attached) and derive
7791+
* the public key from it. */
7792+
ExpectIntEQ(wc_MlDsaKey_ImportPrivRaw(key, bench_mldsa_44_key,
7793+
sizeof_bench_mldsa_44_key), 0);
7794+
ExpectIntEQ(key->pubKeySet, 0);
7795+
7796+
ExpectIntEQ(wc_MlDsaKey_MakePublicKey(key), 0);
7797+
ExpectIntEQ(key->pubKeySet, 1);
7798+
ExpectIntEQ(XMEMCMP(key->p, bench_mldsa_44_pubkey,
7799+
sizeof_bench_mldsa_44_pubkey), 0);
7800+
7801+
/* No-op when the public key is already set. */
7802+
ExpectIntEQ(wc_MlDsaKey_MakePublicKey(key), 0);
7803+
7804+
wc_MlDsaKey_Free(key);
7805+
#endif /* !WOLFSSL_NO_ML_DSA_44 */
7806+
7807+
#ifndef WOLFSSL_NO_ML_DSA_65
7808+
ExpectIntEQ(wc_MlDsaKey_Init(key, NULL, INVALID_DEVID), 0);
7809+
ExpectIntEQ(wc_MlDsaKey_SetParams(key, WC_ML_DSA_65), 0);
7810+
7811+
ExpectIntEQ(wc_MlDsaKey_ImportPrivRaw(key, bench_mldsa_65_key,
7812+
sizeof_bench_mldsa_65_key), 0);
7813+
ExpectIntEQ(key->pubKeySet, 0);
7814+
7815+
ExpectIntEQ(wc_MlDsaKey_MakePublicKey(key), 0);
7816+
ExpectIntEQ(key->pubKeySet, 1);
7817+
ExpectIntEQ(XMEMCMP(key->p, bench_mldsa_65_pubkey,
7818+
sizeof_bench_mldsa_65_pubkey), 0);
7819+
7820+
wc_MlDsaKey_Free(key);
7821+
#endif /* !WOLFSSL_NO_ML_DSA_65 */
7822+
7823+
#ifndef WOLFSSL_NO_ML_DSA_87
7824+
ExpectIntEQ(wc_MlDsaKey_Init(key, NULL, INVALID_DEVID), 0);
7825+
ExpectIntEQ(wc_MlDsaKey_SetParams(key, WC_ML_DSA_87), 0);
7826+
7827+
ExpectIntEQ(wc_MlDsaKey_ImportPrivRaw(key, bench_mldsa_87_key,
7828+
sizeof_bench_mldsa_87_key), 0);
7829+
ExpectIntEQ(key->pubKeySet, 0);
7830+
7831+
ExpectIntEQ(wc_MlDsaKey_MakePublicKey(key), 0);
7832+
ExpectIntEQ(key->pubKeySet, 1);
7833+
ExpectIntEQ(XMEMCMP(key->p, bench_mldsa_87_pubkey,
7834+
sizeof_bench_mldsa_87_pubkey), 0);
7835+
7836+
wc_MlDsaKey_Free(key);
7837+
#endif /* !WOLFSSL_NO_ML_DSA_87 */
7838+
7839+
XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
7840+
#endif
7841+
return EXPECT_RESULT();
7842+
}
7843+
7844+
int test_mldsa_private_key_decode_derives_public_key(void)
7845+
{
7846+
EXPECT_DECLS;
7847+
#if defined(WOLFSSL_HAVE_MLDSA) && !defined(WOLFSSL_MLDSA_NO_ASN1) && \
7848+
!defined(WOLFSSL_MLDSA_ASSIGN_KEY) && !defined(WOLFSSL_MLDSA_NO_MAKE_KEY) && \
7849+
!defined(WOLFSSL_MLDSA_NO_SIGN) && !defined(WOLFSSL_MLDSA_NO_VERIFY) && \
7850+
defined(WOLFSSL_MLDSA_PUBLIC_KEY)
7851+
wc_MlDsaKey* privKey;
7852+
wc_MlDsaKey* pubKey;
7853+
word32 idx;
7854+
7855+
privKey = (wc_MlDsaKey*)XMALLOC(sizeof(*privKey), NULL,
7856+
DYNAMIC_TYPE_TMP_BUFFER);
7857+
ExpectNotNull(privKey);
7858+
pubKey = (wc_MlDsaKey*)XMALLOC(sizeof(*pubKey), NULL,
7859+
DYNAMIC_TYPE_TMP_BUFFER);
7860+
ExpectNotNull(pubKey);
7861+
if (privKey != NULL) {
7862+
XMEMSET(privKey, 0, sizeof(*privKey));
7863+
}
7864+
if (pubKey != NULL) {
7865+
XMEMSET(pubKey, 0, sizeof(*pubKey));
7866+
}
7867+
7868+
#ifndef WOLFSSL_NO_ML_DSA_44
7869+
ExpectIntEQ(wc_MlDsaKey_Init(privKey, NULL, INVALID_DEVID), 0);
7870+
ExpectIntEQ(wc_MlDsaKey_SetParams(privKey, WC_ML_DSA_44), 0);
7871+
7872+
/* mldsa44_priv_only holds a private-key-only DER (no embedded public
7873+
* point). wc_MlDsaKey_PrivateKeyDecode should derive and cache the
7874+
* public key as a best-effort side effect of decoding it. */
7875+
idx = 0;
7876+
ExpectIntEQ(wc_MlDsaKey_PrivateKeyDecode(privKey, mldsa44_priv_only,
7877+
sizeof_mldsa44_priv_only, &idx), 0);
7878+
ExpectIntEQ(privKey->pubKeySet, 1);
7879+
7880+
/* Confirm the derived public key matches the known public key for the
7881+
* same key pair. */
7882+
ExpectIntEQ(wc_MlDsaKey_Init(pubKey, NULL, INVALID_DEVID), 0);
7883+
ExpectIntEQ(wc_MlDsaKey_SetParams(pubKey, WC_ML_DSA_44), 0);
7884+
idx = 0;
7885+
ExpectIntEQ(wc_MlDsaKey_PublicKeyDecode(pubKey, mldsa44_pub_spki,
7886+
sizeof_mldsa44_pub_spki, &idx), 0);
7887+
ExpectIntEQ(XMEMCMP(privKey->p, pubKey->p, WC_MLDSA_44_PUB_KEY_SIZE), 0);
7888+
7889+
wc_MlDsaKey_Free(privKey);
7890+
wc_MlDsaKey_Free(pubKey);
7891+
#endif /* !WOLFSSL_NO_ML_DSA_44 */
7892+
7893+
#ifndef WOLFSSL_NO_ML_DSA_65
7894+
ExpectIntEQ(wc_MlDsaKey_Init(privKey, NULL, INVALID_DEVID), 0);
7895+
ExpectIntEQ(wc_MlDsaKey_SetParams(privKey, WC_ML_DSA_65), 0);
7896+
7897+
idx = 0;
7898+
ExpectIntEQ(wc_MlDsaKey_PrivateKeyDecode(privKey, mldsa65_priv_only,
7899+
sizeof_mldsa65_priv_only, &idx), 0);
7900+
ExpectIntEQ(privKey->pubKeySet, 1);
7901+
7902+
ExpectIntEQ(wc_MlDsaKey_Init(pubKey, NULL, INVALID_DEVID), 0);
7903+
ExpectIntEQ(wc_MlDsaKey_SetParams(pubKey, WC_ML_DSA_65), 0);
7904+
idx = 0;
7905+
ExpectIntEQ(wc_MlDsaKey_PublicKeyDecode(pubKey, mldsa65_pub_spki,
7906+
sizeof_mldsa65_pub_spki, &idx), 0);
7907+
ExpectIntEQ(XMEMCMP(privKey->p, pubKey->p, WC_MLDSA_65_PUB_KEY_SIZE), 0);
7908+
7909+
wc_MlDsaKey_Free(privKey);
7910+
wc_MlDsaKey_Free(pubKey);
7911+
#endif /* !WOLFSSL_NO_ML_DSA_65 */
7912+
7913+
#ifndef WOLFSSL_NO_ML_DSA_87
7914+
ExpectIntEQ(wc_MlDsaKey_Init(privKey, NULL, INVALID_DEVID), 0);
7915+
ExpectIntEQ(wc_MlDsaKey_SetParams(privKey, WC_ML_DSA_87), 0);
7916+
7917+
idx = 0;
7918+
ExpectIntEQ(wc_MlDsaKey_PrivateKeyDecode(privKey, mldsa87_priv_only,
7919+
sizeof_mldsa87_priv_only, &idx), 0);
7920+
ExpectIntEQ(privKey->pubKeySet, 1);
7921+
7922+
ExpectIntEQ(wc_MlDsaKey_Init(pubKey, NULL, INVALID_DEVID), 0);
7923+
ExpectIntEQ(wc_MlDsaKey_SetParams(pubKey, WC_ML_DSA_87), 0);
7924+
idx = 0;
7925+
ExpectIntEQ(wc_MlDsaKey_PublicKeyDecode(pubKey, mldsa87_pub_spki,
7926+
sizeof_mldsa87_pub_spki, &idx), 0);
7927+
ExpectIntEQ(XMEMCMP(privKey->p, pubKey->p, WC_MLDSA_87_PUB_KEY_SIZE), 0);
7928+
7929+
wc_MlDsaKey_Free(privKey);
7930+
wc_MlDsaKey_Free(pubKey);
7931+
#endif /* !WOLFSSL_NO_ML_DSA_87 */
7932+
7933+
XFREE(privKey, NULL, DYNAMIC_TYPE_TMP_BUFFER);
7934+
XFREE(pubKey, NULL, DYNAMIC_TYPE_TMP_BUFFER);
7935+
#endif
7936+
return EXPECT_RESULT();
7937+
}
7938+
77667939
int test_mldsa_sig_kats(void)
77677940
{
77687941
EXPECT_DECLS;

‎tests/api/test_mldsa.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ int test_mldsa_public_der_decode(void);
3838
int test_mldsa_der(void);
3939
int test_mldsa_oneasymkey_version(void);
4040
int test_mldsa_make_key_from_seed(void);
41+
int test_mldsa_make_public_key(void);
42+
int test_mldsa_private_key_decode_derives_public_key(void);
4143
int test_mldsa_sig_kats(void);
4244
int test_mldsa_sign_ctx_kats(void);
4345
int test_mldsa_verify_ctx_kats(void);
@@ -75,6 +77,8 @@ int test_mldsa_legacy_shim(void);
7577
TEST_DECL_GROUP("mldsa", test_mldsa_der), \
7678
TEST_DECL_GROUP("mldsa", test_mldsa_oneasymkey_version), \
7779
TEST_DECL_GROUP("mldsa", test_mldsa_make_key_from_seed), \
80+
TEST_DECL_GROUP("mldsa", test_mldsa_make_public_key), \
81+
TEST_DECL_GROUP("mldsa", test_mldsa_private_key_decode_derives_public_key), \
7882
TEST_DECL_GROUP("mldsa", test_mldsa_sig_kats), \
7983
TEST_DECL_GROUP("mldsa", test_mldsa_sign_ctx_kats), \
8084
TEST_DECL_GROUP("mldsa", test_mldsa_verify_ctx_kats), \

‎wolfcrypt/src/asn.c‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32719,6 +32719,30 @@ int wc_EccPrivateKeyDecode(const byte* input, word32* inOutIdx, ecc_key* key,
3271932719
key, curve_id);
3272032720
}
3272132721

32722+
/* Superset of ecc.c's HAVE_ECC_MAKE_PUB: also excludes QNX_CAAM and
32723+
* IMXRT1170_CAAM, whose private scalar is an opaque hardware key. */
32724+
#if !defined(NO_ECC_MAKE_PUB) && !defined(WOLFSSL_ATECC508A) && \
32725+
!defined(WOLFSSL_ATECC608A) && !defined(WOLFSSL_MICROCHIP_TA100) && \
32726+
!defined(WOLFSSL_CRYPTOCELL) && !defined(WOLFSSL_SILABS_SE_ACCEL) && \
32727+
!defined(WOLFSSL_KCAPI_ECC) && !defined(WOLFSSL_QNX_CAAM) && \
32728+
!defined(WOLFSSL_IMXRT1170_CAAM)
32729+
/* SEC1 allows omitting the public point; derive it best-effort. Under
32730+
* ECC_TIMING_RESISTANT, only if the caller already opted into blinding
32731+
* via wc_ecc_set_rng(), as every other blinded op in this library does. */
32732+
#ifdef ECC_TIMING_RESISTANT
32733+
if ((ret == 0) && (key->type == ECC_PRIVATEKEY_ONLY) &&
32734+
(key->rng != NULL)) {
32735+
#else
32736+
if ((ret == 0) && (key->type == ECC_PRIVATEKEY_ONLY)) {
32737+
#endif
32738+
int pubRet = wc_ecc_make_pub_ex(key, NULL, key->rng);
32739+
if (pubRet != 0) {
32740+
WOLFSSL_MSG_EX("Best-effort ECC public key derivation failed: %d",
32741+
pubRet);
32742+
}
32743+
}
32744+
#endif
32745+
3272232746
FREE_ASNGETDATA(dataASN, key != NULL ? key->heap : NULL);
3272332747
return ret;
3272432748
}

‎wolfcrypt/src/asn_orig.c‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7707,6 +7707,30 @@ int wc_EccPrivateKeyDecode(const byte* input, word32* inOutIdx, ecc_key* key,
77077707
(word32)pubSz, key, curve_id);
77087708
}
77097709

7710+
/* Superset of ecc.c's HAVE_ECC_MAKE_PUB: also excludes QNX_CAAM and
7711+
* IMXRT1170_CAAM, whose private scalar is an opaque hardware key. */
7712+
#if !defined(NO_ECC_MAKE_PUB) && !defined(WOLFSSL_ATECC508A) && \
7713+
!defined(WOLFSSL_ATECC608A) && !defined(WOLFSSL_MICROCHIP_TA100) && \
7714+
!defined(WOLFSSL_CRYPTOCELL) && !defined(WOLFSSL_SILABS_SE_ACCEL) && \
7715+
!defined(WOLFSSL_KCAPI_ECC) && !defined(WOLFSSL_QNX_CAAM) && \
7716+
!defined(WOLFSSL_IMXRT1170_CAAM)
7717+
/* SEC1 allows omitting the public point; derive it best-effort. Under
7718+
* ECC_TIMING_RESISTANT, only if the caller already opted into blinding
7719+
* via wc_ecc_set_rng(), as every other blinded op in this library does. */
7720+
#ifdef ECC_TIMING_RESISTANT
7721+
if ((ret == 0) && (key->type == ECC_PRIVATEKEY_ONLY) &&
7722+
(key->rng != NULL)) {
7723+
#else
7724+
if ((ret == 0) && (key->type == ECC_PRIVATEKEY_ONLY)) {
7725+
#endif
7726+
int pubRet = wc_ecc_make_pub_ex(key, NULL, key->rng);
7727+
if (pubRet != 0) {
7728+
WOLFSSL_MSG_EX("Best-effort ECC public key derivation failed: %d",
7729+
pubRet);
7730+
}
7731+
}
7732+
#endif
7733+
77107734
WC_FREE_VAR_EX(priv, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
77117735
WC_FREE_VAR_EX(pub, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
77127736

0 commit comments

Comments
 (0)