Skip to content

Commit

Permalink
Fix PQC and hybrid certificate regressions
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Frauschi committed May 23, 2024
1 parent 24f581f commit 9a58301
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 33 deletions.
32 changes: 4 additions & 28 deletions src/ssl_load.c
Original file line number Diff line number Diff line change
Expand Up @@ -1622,9 +1622,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
DecodedCert* cert, int checkKeySz)
{
int ret = 0;
#ifdef WOLF_PRIVATE_KEY_ID
byte keyType = 0;
#endif
int keySz = 0;
#ifndef NO_RSA
word32 idx;
Expand All @@ -1637,9 +1635,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
case RSAPSSk:
#endif
case RSAk:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = rsa_sa_algo;
#endif
/* Determine RSA key size by parsing public key */
idx = 0;
ret = wc_RsaPublicKeyDecode_ex(cert->publicKey, &idx,
Expand All @@ -1652,9 +1648,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
#endif /* !NO_RSA */
#ifdef HAVE_ECC
case ECDSAk:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = ecc_dsa_sa_algo;
#endif
/* Determine ECC key size based on curve */
#ifdef WOLFSSL_CUSTOM_CURVES
if ((cert->pkCurveOID == 0) && (cert->pkCurveSize != 0)) {
Expand All @@ -1676,9 +1670,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
#endif /* HAVE_ECC */
#if defined(WOLFSSL_SM2) && defined(WOLFSSL_SM3)
case SM2k:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = sm2_sa_algo;
#endif
/* Determine ECC key size based on curve */
keySz = WOLFSSL_SM2_KEY_BITS / 8;
if (checkKeySz) {
Expand All @@ -1690,9 +1682,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
#endif /* HAVE_ED25519 */
#ifdef HAVE_ED25519
case ED25519k:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = ed25519_sa_algo;
#endif
/* ED25519 is fixed key size */
keySz = ED25519_KEY_SIZE;
if (checkKeySz) {
Expand All @@ -1703,9 +1693,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
#endif /* HAVE_ED25519 */
#ifdef HAVE_ED448
case ED448k:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = ed448_sa_algo;
#endif
/* ED448 is fixed key size */
keySz = ED448_KEY_SIZE;
if (checkKeySz) {
Expand All @@ -1717,9 +1705,7 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
#if defined(HAVE_PQC)
#if defined(HAVE_FALCON)
case FALCON_LEVEL1k:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = falcon_level1_sa_algo;
#endif
/* Falcon is fixed key size */
keySz = FALCON_LEVEL1_KEY_SIZE;
if (checkKeySz) {
Expand All @@ -1729,11 +1715,9 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
}
break;
case FALCON_LEVEL5k:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = falcon_level5_sa_algo;
#endif
/* Falcon is fixed key size */
keySz = FALCON_MAX_KEY_SIZE;
keySz = FALCON_LEVEL5_KEY_SIZE;
if (checkKeySz) {
ret = CHECK_KEY_SZ(ssl ? ssl->options.minFalconKeySz :
ctx->minFalconKeySz, FALCON_MAX_KEY_SIZE, keySz,
Expand All @@ -1743,35 +1727,29 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
#endif /* HAVE_FALCON */
#if defined(HAVE_DILITHIUM)
case DILITHIUM_LEVEL2k:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = dilithium_level2_sa_algo;
#endif
/* Dilithium is fixed key size */
keySz = DILITHIUM_MAX_KEY_SIZE;
keySz = DILITHIUM_LEVEL2_KEY_SIZE;
if (checkKeySz) {
ret = CHECK_KEY_SZ(ssl ? ssl->options.minDilithiumKeySz :
ctx->minDilithiumKeySz, DILITHIUM_MAX_KEY_SIZE, keySz,
DILITHIUM_KEY_SIZE_E);
}
break;
case DILITHIUM_LEVEL3k:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = dilithium_level3_sa_algo;
#endif
/* Dilithium is fixed key size */
keySz = DILITHIUM_MAX_KEY_SIZE;
keySz = DILITHIUM_LEVEL3_KEY_SIZE;
if (checkKeySz) {
ret = CHECK_KEY_SZ(ssl ? ssl->options.minDilithiumKeySz :
ctx->minDilithiumKeySz, DILITHIUM_MAX_KEY_SIZE, keySz,
DILITHIUM_KEY_SIZE_E);
}
break;
case DILITHIUM_LEVEL5k:
#ifdef WOLF_PRIVATE_KEY_ID
keyType = dilithium_level5_sa_algo;
#endif
/* Dilithium is fixed key size */
keySz = DILITHIUM_MAX_KEY_SIZE;
keySz = DILITHIUM_LEVEL5_KEY_SIZE;
if (checkKeySz) {
ret = CHECK_KEY_SZ(ssl ? ssl->options.minDilithiumKeySz :
ctx->minDilithiumKeySz, DILITHIUM_MAX_KEY_SIZE, keySz,
Expand All @@ -1786,7 +1764,6 @@ static int ProcessBufferCertPublicKey(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
break;
}

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

return ret;
}
Expand Down
7 changes: 5 additions & 2 deletions src/tls13.c
Original file line number Diff line number Diff line change
Expand Up @@ -10088,10 +10088,13 @@ static int DoTls13CertificateVerify(WOLFSSL* ssl, byte* input,
* with their size as 16-bit integeter prior in memory. Hence,
* we can decode both lengths here now. */
word32 tmpIdx = args->idx;
ato32(input + tmpIdx, &args->sigSz);
word16 tmpSz = 0;
ato16(input + tmpIdx, &tmpSz);
args->sigSz = tmpSz;

tmpIdx += OPAQUE16_LEN + args->sigSz;
ato32(input + tmpIdx, &args->altSignatureSz);
ato16(input + tmpIdx, &tmpSz);
args->altSignatureSz = tmpSz;

if (args->sz != (args->sigSz + args->altSignatureSz +
OPAQUE16_LEN + OPAQUE16_LEN)) {
Expand Down
2 changes: 1 addition & 1 deletion wolfcrypt/src/dilithium.c
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ static int parse_private_key(const byte* priv, word32 privSz,

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

/* Now it is a octet_string(concat(priv,pub)) */
Expand Down
2 changes: 1 addition & 1 deletion wolfcrypt/src/falcon.c
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ static int parse_private_key(const byte* priv, word32 privSz,

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

/* Now it is a octet_string(concat(priv,pub)) */
Expand Down
2 changes: 1 addition & 1 deletion wolfcrypt/src/sphincs.c
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ static int parse_private_key(const byte* priv, word32 privSz,

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

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

0 comments on commit 9a58301

Please sign in to comment.