Skip to content

Commit 9a58301

Browse files
FrauschiFrauschi
committed
Fix PQC and hybrid certificate regressions
Due to recent changes in the logic to decode private keys and to parse the TLS1.3 CertificateVerify message, some regressions regarding PQC private keys and hybrid certificates have been introduced: * Decoding PQC private keys fails as the PKCS8 header of a decoded DER file is now already removed before parsing the key. * The key size wasn't properly stored in the context for PQC keys after decoding a certificate (always the maximum size) * The two 16-bit size values in case of a hybrid signature in the CertificateVerify message have been incorrectly decoded as 32-bit values instead of 16-bit values. This resulted in wrong values, leading to segmentation faults. All three regressions are fixed with the changes in this commit. Signed-off-by: Tobias Frauenschläger <t.frauenschlaeger@me.com>
1 parent 24f581f commit 9a58301

5 files changed

Lines changed: 12 additions & 33 deletions

File tree

‎src/ssl_load.c‎

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1622,9 +1622,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
16221622
DecodedCert* cert, int checkKeySz)
16231623
{
16241624
int ret = 0;
1625-
#ifdef WOLF_PRIVATE_KEY_ID
16261625
byte keyType = 0;
1627-
#endif
16281626
int keySz = 0;
16291627
#ifndef NO_RSA
16301628
word32 idx;
@@ -1637,9 +1635,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
16371635
case RSAPSSk:
16381636
#endif
16391637
case RSAk:
1640-
#ifdef WOLF_PRIVATE_KEY_ID
16411638
keyType = rsa_sa_algo;
1642-
#endif
16431639
/* Determine RSA key size by parsing public key */
16441640
idx = 0;
16451641
ret = wc_RsaPublicKeyDecode_ex(cert->publicKey, &idx,
@@ -1652,9 +1648,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
16521648
#endif /* !NO_RSA */
16531649
#ifdef HAVE_ECC
16541650
case ECDSAk:
1655-
#ifdef WOLF_PRIVATE_KEY_ID
16561651
keyType = ecc_dsa_sa_algo;
1657-
#endif
16581652
/* Determine ECC key size based on curve */
16591653
#ifdef WOLFSSL_CUSTOM_CURVES
16601654
if ((cert->pkCurveOID == 0) && (cert->pkCurveSize != 0)) {
@@ -1676,9 +1670,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
16761670
#endif /* HAVE_ECC */
16771671
#if defined(WOLFSSL_SM2) && defined(WOLFSSL_SM3)
16781672
case SM2k:
1679-
#ifdef WOLF_PRIVATE_KEY_ID
16801673
keyType = sm2_sa_algo;
1681-
#endif
16821674
/* Determine ECC key size based on curve */
16831675
keySz = WOLFSSL_SM2_KEY_BITS / 8;
16841676
if (checkKeySz) {
@@ -1690,9 +1682,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
16901682
#endif /* HAVE_ED25519 */
16911683
#ifdef HAVE_ED25519
16921684
case ED25519k:
1693-
#ifdef WOLF_PRIVATE_KEY_ID
16941685
keyType = ed25519_sa_algo;
1695-
#endif
16961686
/* ED25519 is fixed key size */
16971687
keySz = ED25519_KEY_SIZE;
16981688
if (checkKeySz) {
@@ -1703,9 +1693,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
17031693
#endif /* HAVE_ED25519 */
17041694
#ifdef HAVE_ED448
17051695
case ED448k:
1706-
#ifdef WOLF_PRIVATE_KEY_ID
17071696
keyType = ed448_sa_algo;
1708-
#endif
17091697
/* ED448 is fixed key size */
17101698
keySz = ED448_KEY_SIZE;
17111699
if (checkKeySz) {
@@ -1717,9 +1705,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
17171705
#if defined(HAVE_PQC)
17181706
#if defined(HAVE_FALCON)
17191707
case FALCON_LEVEL1k:
1720-
#ifdef WOLF_PRIVATE_KEY_ID
17211708
keyType = falcon_level1_sa_algo;
1722-
#endif
17231709
/* Falcon is fixed key size */
17241710
keySz = FALCON_LEVEL1_KEY_SIZE;
17251711
if (checkKeySz) {
@@ -1729,11 +1715,9 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
17291715
}
17301716
break;
17311717
case FALCON_LEVEL5k:
1732-
#ifdef WOLF_PRIVATE_KEY_ID
17331718
keyType = falcon_level5_sa_algo;
1734-
#endif
17351719
/* Falcon is fixed key size */
1736-
keySz = FALCON_MAX_KEY_SIZE;
1720+
keySz = FALCON_LEVEL5_KEY_SIZE;
17371721
if (checkKeySz) {
17381722
ret = CHECK_KEY_SZ(ssl ? ssl->options.minFalconKeySz :
17391723
ctx->minFalconKeySz, FALCON_MAX_KEY_SIZE, keySz,
@@ -1743,35 +1727,29 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
17431727
#endif /* HAVE_FALCON */
17441728
#if defined(HAVE_DILITHIUM)
17451729
case DILITHIUM_LEVEL2k:
1746-
#ifdef WOLF_PRIVATE_KEY_ID
17471730
keyType = dilithium_level2_sa_algo;
1748-
#endif
17491731
/* Dilithium is fixed key size */
1750-
keySz = DILITHIUM_MAX_KEY_SIZE;
1732+
keySz = DILITHIUM_LEVEL2_KEY_SIZE;
17511733
if (checkKeySz) {
17521734
ret = CHECK_KEY_SZ(ssl ? ssl->options.minDilithiumKeySz :
17531735
ctx->minDilithiumKeySz, DILITHIUM_MAX_KEY_SIZE, keySz,
17541736
DILITHIUM_KEY_SIZE_E);
17551737
}
17561738
break;
17571739
case DILITHIUM_LEVEL3k:
1758-
#ifdef WOLF_PRIVATE_KEY_ID
17591740
keyType = dilithium_level3_sa_algo;
1760-
#endif
17611741
/* Dilithium is fixed key size */
1762-
keySz = DILITHIUM_MAX_KEY_SIZE;
1742+
keySz = DILITHIUM_LEVEL3_KEY_SIZE;
17631743
if (checkKeySz) {
17641744
ret = CHECK_KEY_SZ(ssl ? ssl->options.minDilithiumKeySz :
17651745
ctx->minDilithiumKeySz, DILITHIUM_MAX_KEY_SIZE, keySz,
17661746
DILITHIUM_KEY_SIZE_E);
17671747
}
17681748
break;
17691749
case DILITHIUM_LEVEL5k:
1770-
#ifdef WOLF_PRIVATE_KEY_ID
17711750
keyType = dilithium_level5_sa_algo;
1772-
#endif
17731751
/* Dilithium is fixed key size */
1774-
keySz = DILITHIUM_MAX_KEY_SIZE;
1752+
keySz = DILITHIUM_LEVEL5_KEY_SIZE;
17751753
if (checkKeySz) {
17761754
ret = CHECK_KEY_SZ(ssl ? ssl->options.minDilithiumKeySz :
17771755
ctx->minDilithiumKeySz, DILITHIUM_MAX_KEY_SIZE, keySz,
@@ -1786,7 +1764,6 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
17861764
break;
17871765
}
17881766

1789-
#ifdef WOLF_PRIVATE_KEY_ID
17901767
/* Store the type and key size as there may not be a private key set. */
17911768
if (ssl != NULL) {
17921769
ssl->buffers.keyType = keyType;
@@ -1796,7 +1773,6 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
17961773
ctx->privateKeyType = keyType;
17971774
ctx->privateKeySz = keySz;
17981775
}
1799-
#endif
18001776

18011777
return ret;
18021778
}

‎src/tls13.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10088,10 +10088,13 @@ static int DoTls13CertificateVerify(WOLFSSL* ssl, byte* input,
1008810088
* with their size as 16-bit integeter prior in memory. Hence,
1008910089
* we can decode both lengths here now. */
1009010090
word32 tmpIdx = args->idx;
10091-
ato32(input + tmpIdx, &args->sigSz);
10091+
word16 tmpSz = 0;
10092+
ato16(input + tmpIdx, &tmpSz);
10093+
args->sigSz = tmpSz;
1009210094

1009310095
tmpIdx += OPAQUE16_LEN + args->sigSz;
10094-
ato32(input + tmpIdx, &args->altSignatureSz);
10096+
ato16(input + tmpIdx, &tmpSz);
10097+
args->altSignatureSz = tmpSz;
1009510098

1009610099
if (args->sz != (args->sigSz + args->altSignatureSz +
1009710100
OPAQUE16_LEN + OPAQUE16_LEN)) {

‎wolfcrypt/src/dilithium.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ static int parse_private_key(const byte* priv, word32 privSz,
488488

489489
/* At this point, it is still a PKCS8 private key. */
490490
if ((ret = ToTraditionalInline(priv, &idx, privSz)) < 0) {
491-
return ret;
491+
/* ignore error, did not have PKCS8 header */
492492
}
493493

494494
/* Now it is a octet_string(concat(priv,pub)) */

‎wolfcrypt/src/falcon.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ static int parse_private_key(const byte* priv, word32 privSz,
469469

470470
/* At this point, it is still a PKCS8 private key. */
471471
if ((ret = ToTraditionalInline(priv, &idx, privSz)) < 0) {
472-
return ret;
472+
/* ignore error, did not have PKCS8 header */
473473
}
474474

475475
/* Now it is a octet_string(concat(priv,pub)) */

‎wolfcrypt/src/sphincs.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ static int parse_private_key(const byte* priv, word32 privSz,
431431

432432
/* At this point, it is still a PKCS8 private key. */
433433
if ((ret = ToTraditionalInline(priv, &idx, privSz)) < 0) {
434-
return ret;
434+
/* ignore error, did not have PKCS8 header */
435435
}
436436

437437
/* Now it is a octet_string(concat(priv,pub)) */

0 commit comments

Comments
 (0)