Skip to content

Commit b6324ad

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 b6324ad

8 files changed

Lines changed: 707 additions & 7 deletions

File tree

‎tests/api/test_asn.c‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2481,3 +2481,63 @@ 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+
word32 idx;
2500+
byte privOnlyDer[256];
2501+
int privOnlyDerSz;
2502+
byte fullPub[256];
2503+
word32 fullPubSz = sizeof(fullPub);
2504+
byte derivedPub[256];
2505+
word32 derivedPubSz = sizeof(derivedPub);
2506+
2507+
XMEMSET(&fullKey, 0, sizeof(fullKey));
2508+
XMEMSET(&privOnlyKey, 0, sizeof(privOnlyKey));
2509+
2510+
ExpectIntEQ(wc_ecc_init(&fullKey), 0);
2511+
idx = 0;
2512+
ExpectIntEQ(wc_EccPrivateKeyDecode(ecc_clikey_der_256, &idx, &fullKey,
2513+
sizeof_ecc_clikey_der_256), 0);
2514+
ExpectIntEQ(fullKey.type, ECC_PRIVATEKEY);
2515+
PRIVATE_KEY_UNLOCK();
2516+
ExpectIntEQ(wc_ecc_export_x963(&fullKey, fullPub, &fullPubSz), 0);
2517+
PRIVATE_KEY_LOCK();
2518+
2519+
/* Re-encode as a private-key-only SEC1 DER (no public point). */
2520+
ExpectIntGT(privOnlyDerSz = wc_EccPrivateKeyToDer(&fullKey, privOnlyDer,
2521+
sizeof(privOnlyDer)), 0);
2522+
2523+
ExpectIntEQ(wc_ecc_init(&privOnlyKey), 0);
2524+
idx = 0;
2525+
ExpectIntEQ(wc_EccPrivateKeyDecode(privOnlyDer, &idx, &privOnlyKey,
2526+
(word32)privOnlyDerSz), 0);
2527+
2528+
/* The public point should have been derived automatically, making the
2529+
* key fully usable rather than left as ECC_PRIVATEKEY_ONLY. */
2530+
ExpectIntEQ(privOnlyKey.type, ECC_PRIVATEKEY);
2531+
PRIVATE_KEY_UNLOCK();
2532+
ExpectIntEQ(wc_ecc_export_x963(&privOnlyKey, derivedPub, &derivedPubSz),
2533+
0);
2534+
PRIVATE_KEY_LOCK();
2535+
ExpectIntEQ(derivedPubSz, fullPubSz);
2536+
ExpectBufEQ(derivedPub, fullPub, fullPubSz);
2537+
2538+
wc_ecc_free(&privOnlyKey);
2539+
wc_ecc_free(&fullKey);
2540+
#endif /* !NO_ASN && HAVE_ECC && !NO_ECC_MAKE_PUB && USE_CERT_BUFFERS_256 &&
2541+
* !HAVE_FIPS */
2542+
return EXPECT_RESULT();
2543+
}

‎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: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32719,6 +32719,31 @@ int wc_EccPrivateKeyDecode(const byte* input, word32* inOutIdx, ecc_key* key,
3271932719
key, curve_id);
3272032720
}
3272132721

32722+
/* Based on ecc.c's HAVE_ECC_MAKE_PUB condition, which is local to
32723+
* that file. This guard is intentionally a superset: QNX_CAAM and
32724+
* IMXRT1170_CAAM are added because those ports store the private
32725+
* scalar as an opaque hardware "black key" where a software point
32726+
* multiply would be meaningless or unsafe. */
32727+
#if !defined(NO_ECC_MAKE_PUB) && !defined(WOLFSSL_ATECC508A) && \
32728+
!defined(WOLFSSL_ATECC608A) && !defined(WOLFSSL_MICROCHIP_TA100) && \
32729+
!defined(WOLFSSL_CRYPTOCELL) && !defined(WOLFSSL_SILABS_SE_ACCEL) && \
32730+
!defined(WOLFSSL_KCAPI_ECC) && !defined(WOLFSSL_QNX_CAAM) && \
32731+
!defined(WOLFSSL_IMXRT1170_CAAM)
32732+
if ((ret == 0) && (key->type == ECC_PRIVATEKEY_ONLY)) {
32733+
/* SEC1 allows omitting the public point; derive it. Best-effort:
32734+
* failure must not fail decoding of an otherwise valid key. */
32735+
int pubRet;
32736+
#ifdef ECC_TIMING_RESISTANT
32737+
pubRet = wc_ecc_make_pub_ex(key, NULL, key->rng);
32738+
#else
32739+
pubRet = wc_ecc_make_pub_ex(key, NULL, NULL);
32740+
#endif
32741+
if (pubRet != 0) {
32742+
WOLFSSL_MSG("Best-effort ECC public key derivation failed");
32743+
}
32744+
}
32745+
#endif
32746+
3272232747
FREE_ASNGETDATA(dataASN, key != NULL ? key->heap : NULL);
3272332748
return ret;
3272432749
}

‎wolfcrypt/src/asn_orig.c‎

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

7710+
/* Based on ecc.c's HAVE_ECC_MAKE_PUB condition, which is local to
7711+
* that file. This guard is intentionally a superset: QNX_CAAM and
7712+
* IMXRT1170_CAAM are added because those ports store the private
7713+
* scalar as an opaque hardware "black key" where a software point
7714+
* multiply would be meaningless or unsafe. */
7715+
#if !defined(NO_ECC_MAKE_PUB) && !defined(WOLFSSL_ATECC508A) && \
7716+
!defined(WOLFSSL_ATECC608A) && !defined(WOLFSSL_MICROCHIP_TA100) && \
7717+
!defined(WOLFSSL_CRYPTOCELL) && !defined(WOLFSSL_SILABS_SE_ACCEL) && \
7718+
!defined(WOLFSSL_KCAPI_ECC) && !defined(WOLFSSL_QNX_CAAM) && \
7719+
!defined(WOLFSSL_IMXRT1170_CAAM)
7720+
if ((ret == 0) && (key->type == ECC_PRIVATEKEY_ONLY)) {
7721+
/* SEC1 allows omitting the public point; derive it. Best-effort:
7722+
* failure must not fail decoding of an otherwise valid key. */
7723+
int pubRet;
7724+
#ifdef ECC_TIMING_RESISTANT
7725+
pubRet = wc_ecc_make_pub_ex(key, NULL, key->rng);
7726+
#else
7727+
pubRet = wc_ecc_make_pub_ex(key, NULL, NULL);
7728+
#endif
7729+
if (pubRet != 0) {
7730+
WOLFSSL_MSG("Best-effort ECC public key derivation failed");
7731+
}
7732+
}
7733+
#endif
7734+
77107735
WC_FREE_VAR_EX(priv, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
77117736
WC_FREE_VAR_EX(pub, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
77127737

0 commit comments

Comments
 (0)