From e13861bcde8015bb99ddb034224afb66e2fb89b8 Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 1 Feb 2022 11:28:25 -0800 Subject: [PATCH 1/2] Fix for mutual authentication to prevent mismatch of certificate and sig algo. Work from Sean P. ZD 13571 --- src/tls13.c | 53 ++++++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/tls13.c b/src/tls13.c index ecdaa7029..93c2efff3 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6538,6 +6538,8 @@ static int DoTls13CertificateVerify(WOLFSSL* ssl, byte* input, case TLS_ASYNC_BUILD: { + int validSigAlgo = 0; + /* Signature algorithm. */ if ((args->idx - args->begin) + ENUM_LEN + ENUM_LEN > totalSz) { ERROR_OUT(BUFFER_ERROR, exit_dcv); @@ -6563,53 +6565,50 @@ static int DoTls13CertificateVerify(WOLFSSL* ssl, byte* input, /* Check for public key of required type. */ #ifdef HAVE_ED25519 - if (args->sigAlgo == ed25519_sa_algo && - !ssl->peerEd25519KeyPresent) { - WOLFSSL_MSG("Peer sent ED25519 sig but not ED25519 cert"); - ret = SIG_VERIFY_E; - goto exit_dcv; + if (args->sigAlgo == ed25519_sa_algo) { + WOLFSSL_MSG("Peer sent ED25519 sig"); + validSigAlgo = ssl->peerEd25519KeyPresent; } #endif #ifdef HAVE_ED448 - if (args->sigAlgo == ed448_sa_algo && !ssl->peerEd448KeyPresent) { - WOLFSSL_MSG("Peer sent ED448 sig but not ED448 cert"); - ret = SIG_VERIFY_E; - goto exit_dcv; + if (args->sigAlgo == ed448_sa_algo) { + WOLFSSL_MSG("Peer sent ED448 sig"); + validSigAlgo = ssl->peerEd448KeyPresent; } #endif #ifdef HAVE_ECC - if (args->sigAlgo == ecc_dsa_sa_algo && - !ssl->peerEccDsaKeyPresent) { - WOLFSSL_MSG("Peer sent ECC sig but not ECC cert"); - ret = SIG_VERIFY_E; - goto exit_dcv; + if (args->sigAlgo == ecc_dsa_sa_algo) { + WOLFSSL_MSG("Peer sent ECC sig"); + validSigAlgo = ssl->peerEccDsaKeyPresent; } #endif #ifdef HAVE_PQC - if (args->sigAlgo == falcon_level1_sa_algo && !ssl->peerFalconKeyPresent) { - WOLFSSL_MSG("Peer sent Falcon Level 1 sig but different cert"); - ret = SIG_VERIFY_E; - goto exit_dcv; + if (args->sigAlgo == falcon_level1_sa_algo) { + WOLFSSL_MSG("Peer sent Falcon Level 1 sig"); + validSigAlgo = ssl->peerFalconKeyPresent; } - if (args->sigAlgo == falcon_level5_sa_algo && !ssl->peerFalconKeyPresent) { - WOLFSSL_MSG("Peer sent Falcon Level 5 sig but different cert"); - ret = SIG_VERIFY_E; - goto exit_dcv; + if (args->sigAlgo == falcon_level5_sa_algo) { + WOLFSSL_MSG("Peer sent Falcon Level 5 sig"); + validSigAlgo = ssl->peerFalconKeyPresent; } #endif #ifndef NO_RSA if (args->sigAlgo == rsa_sa_algo) { - WOLFSSL_MSG("Peer sent PKCS#1.5 algo but not in certificate"); + WOLFSSL_MSG("Peer sent PKCS#1.5 algo - not valid TLS 1.3"); ERROR_OUT(INVALID_PARAMETER, exit_dcv); } - if (args->sigAlgo == rsa_pss_sa_algo && - (ssl->peerRsaKey == NULL || !ssl->peerRsaKeyPresent)) { - WOLFSSL_MSG("Peer sent RSA sig but not RSA cert"); + if (args->sigAlgo == rsa_pss_sa_algo) { + WOLFSSL_MSG("Peer sent RSA sig"); + validSigAlgo = (ssl->peerRsaKey != NULL) && + ssl->peerRsaKeyPresent; + } + #endif + if (!validSigAlgo) { + WOLFSSL_MSG("Sig algo doesn't correspond to certficate"); ret = SIG_VERIFY_E; goto exit_dcv; } - #endif sig->buffer = (byte*)XMALLOC(args->sz, ssl->heap, DYNAMIC_TYPE_SIGNATURE); From 08047b2d959ee5e21a4a2c672308f45fec61f059 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 4 Feb 2022 10:57:35 -0800 Subject: [PATCH 2/2] Add checking to make sure key is present in all cases. Explicitly set `validSigAlgo` to zero with comment to clarify the default assumption. --- src/tls13.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/tls13.c b/src/tls13.c index 93c2efff3..16a8c9ef9 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6538,7 +6538,7 @@ static int DoTls13CertificateVerify(WOLFSSL* ssl, byte* input, case TLS_ASYNC_BUILD: { - int validSigAlgo = 0; + int validSigAlgo; /* Signature algorithm. */ if ((args->idx - args->begin) + ENUM_LEN + ENUM_LEN > totalSz) { @@ -6564,35 +6564,41 @@ static int DoTls13CertificateVerify(WOLFSSL* ssl, byte* input, } /* Check for public key of required type. */ + /* Assume invalid unless signature algo matches the key provided */ + validSigAlgo = 0; #ifdef HAVE_ED25519 if (args->sigAlgo == ed25519_sa_algo) { WOLFSSL_MSG("Peer sent ED25519 sig"); - validSigAlgo = ssl->peerEd25519KeyPresent; + validSigAlgo = (ssl->peerEd25519Key != NULL) && + ssl->peerEd25519KeyPresent; } #endif #ifdef HAVE_ED448 if (args->sigAlgo == ed448_sa_algo) { WOLFSSL_MSG("Peer sent ED448 sig"); - validSigAlgo = ssl->peerEd448KeyPresent; + validSigAlgo = (ssl->peerEd448Key != NULL) && + ssl->peerEd448KeyPresent; } #endif #ifdef HAVE_ECC if (args->sigAlgo == ecc_dsa_sa_algo) { WOLFSSL_MSG("Peer sent ECC sig"); - validSigAlgo = ssl->peerEccDsaKeyPresent; + validSigAlgo = (ssl->peerEccDsaKey != NULL) && + ssl->peerEccDsaKeyPresent; } #endif #ifdef HAVE_PQC if (args->sigAlgo == falcon_level1_sa_algo) { WOLFSSL_MSG("Peer sent Falcon Level 1 sig"); - validSigAlgo = ssl->peerFalconKeyPresent; + validSigAlgo = (ssl->peerFalconKey != NULL) && + ssl->peerFalconKeyPresent; } if (args->sigAlgo == falcon_level5_sa_algo) { WOLFSSL_MSG("Peer sent Falcon Level 5 sig"); - validSigAlgo = ssl->peerFalconKeyPresent; + validSigAlgo = (ssl->peerFalconKey != NULL) && + ssl->peerFalconKeyPresent; } #endif - #ifndef NO_RSA if (args->sigAlgo == rsa_sa_algo) { WOLFSSL_MSG("Peer sent PKCS#1.5 algo - not valid TLS 1.3"); @@ -6606,8 +6612,7 @@ static int DoTls13CertificateVerify(WOLFSSL* ssl, byte* input, #endif if (!validSigAlgo) { WOLFSSL_MSG("Sig algo doesn't correspond to certficate"); - ret = SIG_VERIFY_E; - goto exit_dcv; + ERROR_OUT(SIG_VERIFY_E, exit_dcv); } sig->buffer = (byte*)XMALLOC(args->sz, ssl->heap,