diff --git a/src/dtls13.c b/src/dtls13.c index 995d144119..0de419def8 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -2369,7 +2369,6 @@ static Dtls13Epoch* Dtls13NewEpochSlot(WOLFSSL* ssl) w64wrapper oldestNumber; int i; - /* FIXME: add max function */ oldestNumber = w64From32((word32)-1, (word32)-1); oldest = NULL; @@ -2380,8 +2379,10 @@ static Dtls13Epoch* Dtls13NewEpochSlot(WOLFSSL* ssl) if (!w64Equal(e->epochNumber, ssl->dtls13Epoch) && !w64Equal(e->epochNumber, ssl->dtls13PeerEpoch) && - w64LT(e->epochNumber, oldestNumber)) + w64LT(e->epochNumber, oldestNumber)) { oldest = e; + oldestNumber = e->epochNumber; + } } if (oldest == NULL) @@ -2720,17 +2721,27 @@ static int Dtls13RtxIsTrackedByRn(const Dtls13RtxRecord* r, w64wrapper epoch, static int Dtls13KeyUpdateAckReceived(WOLFSSL* ssl) { int ret; + /* Validate on a local copy so ssl->dtls13Epoch is left untouched when a + * check fails. */ + w64wrapper newEpoch = ssl->dtls13Epoch; ret = DeriveTls13Keys(ssl, update_traffic_key, ENCRYPT_SIDE_ONLY, 1); if (ret != 0) return ret; - w64Increment(&ssl->dtls13Epoch); + w64Increment(&newEpoch); /* Epoch wrapped up */ - if (w64IsZero(ssl->dtls13Epoch)) + if (w64IsZero(newEpoch)) + return BAD_STATE_E; + + /* RFC 9147 Section 4.2.1: the epoch must not exceed 2^48-1. */ + if (w64GT(newEpoch, + w64From32(DTLS13_EPOCH_MAX_HI32, DTLS13_EPOCH_MAX_LO32))) return BAD_STATE_E; + ssl->dtls13Epoch = newEpoch; + return Dtls13SetEpochKeys(ssl, ssl->dtls13Epoch, ENCRYPT_SIDE_ONLY); } diff --git a/src/internal.c b/src/internal.c index dcd8fa8ab6..ccea4c0271 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3282,6 +3282,7 @@ static void FreeCiphersSide(Ciphers *cipher, void* heap) cipher->aria = NULL; #endif #ifdef HAVE_CAMELLIA + wc_CamelliaFree(cipher->cam); XFREE(cipher->cam, heap, DYNAMIC_TYPE_CIPHER); cipher->cam = NULL; #endif diff --git a/src/pk.c b/src/pk.c index 131e1367e0..bfc039e5d0 100644 --- a/src/pk.c +++ b/src/pk.c @@ -4977,8 +4977,7 @@ static int _DH_compute_key(unsigned char* key, const WOLFSSL_BIGNUM* otherPub, * correctly. */ if (keySz < padded_keySz) { - XMEMMOVE(key, key + (padded_keySz - keySz), - padded_keySz - keySz); + XMEMMOVE(key + (padded_keySz - keySz), key, keySz); XMEMSET(key, 0, padded_keySz - keySz); keySz = padded_keySz; } diff --git a/src/ssl.c b/src/ssl.c index efbbf3074e..395352835c 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -3319,8 +3319,8 @@ int wolfSSL_CTX_set1_groups(WOLFSSL_CTX* ctx, int* groups, int i; int _groups[WOLFSSL_MAX_GROUP_COUNT]; WOLFSSL_ENTER("wolfSSL_CTX_set1_groups"); - if (count == 0) { - WOLFSSL_MSG("Group count is zero"); + if (count <= 0) { + WOLFSSL_MSG("Group count is not positive"); return WOLFSSL_FAILURE; } if (count > WOLFSSL_MAX_GROUP_COUNT) { @@ -3358,8 +3358,8 @@ int wolfSSL_set1_groups(WOLFSSL* ssl, int* groups, int count) int i; int _groups[WOLFSSL_MAX_GROUP_COUNT]; WOLFSSL_ENTER("wolfSSL_CTX_set1_groups"); - if (count == 0) { - WOLFSSL_MSG("Group count is zero"); + if (count <= 0) { + WOLFSSL_MSG("Group count is not positive"); return WOLFSSL_FAILURE; } if (count > WOLFSSL_MAX_GROUP_COUNT) { @@ -13025,7 +13025,10 @@ size_t wolfSSL_get_peer_finished(const WOLFSSL *ssl, void *buf, size_t count) long wolfSSL_get_verify_result(const WOLFSSL *ssl) { if (ssl == NULL) { - return WOLFSSL_FAILURE; + /* Return a non-zero error so the OpenSSL-idiomatic + * "!= X509_V_OK" check does not mistake a NULL ssl for a + * successful verification (X509_V_OK is 0). */ + return WOLFSSL_X509_V_ERR_APPLICATION_VERIFICATION; } return (long)ssl->peerVerifyRet; @@ -16199,9 +16202,24 @@ int wolfSSL_CTX_set_servername_arg(WOLFSSL_CTX* ctx, void* arg) int wolfSSL_CRYPTO_memcmp(const void *a, const void *b, size_t size) { + int ret = 0; + int chunk; + const byte* pa = (const byte*)a; + const byte* pb = (const byte*)b; + if (!a || !b) return -1; - return ConstantCompare((const byte*)a, (const byte*)b, (int)size); + /* ConstantCompare takes an int length. Compare in chunks of at most + * INT_MAX so a size that does not fit in an int is not narrowed into a + * negative or truncated length, which could wrongly report equality. */ + while (size > 0) { + chunk = (size > (size_t)INT_MAX) ? INT_MAX : (int)size; + ret |= ConstantCompare(pa, pb, chunk); + pa += chunk; + pb += chunk; + size -= (size_t)chunk; + } + return ret; } unsigned long wolfSSL_ERR_peek_last_error(void) diff --git a/src/tls.c b/src/tls.c index 62118d0678..2defa82509 100644 --- a/src/tls.c +++ b/src/tls.c @@ -391,15 +391,17 @@ ProtocolVersion MakeTLSv1_3(void) * ctx SSL/TLS context object. * groups Array of groups. * count Number of groups in array. - * returns BAD_FUNC_ARG when ctx or groups is NULL, not using TLS v1.3 or - * count is greater than WOLFSSL_MAX_GROUP_COUNT and WOLFSSL_SUCCESS on success. + * returns BAD_FUNC_ARG when ctx or groups is NULL, not using TLS v1.3, count is + * not positive or count is greater than WOLFSSL_MAX_GROUP_COUNT and + * WOLFSSL_SUCCESS on success. */ int wolfSSL_CTX_set_groups(WOLFSSL_CTX* ctx, int* groups, int count) { int ret, i; WOLFSSL_ENTER("wolfSSL_CTX_set_groups"); - if (ctx == NULL || groups == NULL || count > WOLFSSL_MAX_GROUP_COUNT) + if (ctx == NULL || groups == NULL || count <= 0 || + count > WOLFSSL_MAX_GROUP_COUNT) return BAD_FUNC_ARG; if (!IsTLS_ex(ctx->method->version)) return BAD_FUNC_ARG; @@ -436,15 +438,17 @@ int wolfSSL_CTX_set_groups(WOLFSSL_CTX* ctx, int* groups, int count) * ssl SSL/TLS object. * groups Array of groups. * count Number of groups in array. - * returns BAD_FUNC_ARG when ssl or groups is NULL, not using TLS v1.3 or - * count is greater than WOLFSSL_MAX_GROUP_COUNT and WOLFSSL_SUCCESS on success. + * returns BAD_FUNC_ARG when ssl or groups is NULL, not using TLS v1.3, count is + * not positive or count is greater than WOLFSSL_MAX_GROUP_COUNT and + * WOLFSSL_SUCCESS on success. */ int wolfSSL_set_groups(WOLFSSL* ssl, int* groups, int count) { int ret, i; WOLFSSL_ENTER("wolfSSL_set_groups"); - if (ssl == NULL || groups == NULL || count > WOLFSSL_MAX_GROUP_COUNT) + if (ssl == NULL || groups == NULL || count <= 0 || + count > WOLFSSL_MAX_GROUP_COUNT) return BAD_FUNC_ARG; if (!IsTLS_ex(ssl->version)) return BAD_FUNC_ARG; @@ -2065,6 +2069,15 @@ static int TLSX_ALPN_ParseAndSet(WOLFSSL *ssl, const byte *input, word16 length, byte sel_len = 0; TLSX *extension = NULL; + /* RFC 7301 Section 3.1: a ServerHello ALPN extension MUST contain + * exactly one protocol name. The first name's length byte plus its + * payload must therefore span the whole list. */ + if ((word16)(input[offset] + OPAQUE8_LEN) != size) { + SendAlert(ssl, alert_fatal, illegal_parameter); + WOLFSSL_ERROR_VERBOSE(BUFFER_ERROR); + return BUFFER_ERROR; + } + r = ALPN_find_match(ssl, &extension, &sel, &sel_len, input + offset, size); if (r != 0) return r; @@ -3251,9 +3264,27 @@ static int TLSX_MFL_Parse(WOLFSSL* ssl, const byte* input, word16 length, #ifdef WOLFSSL_OLD_UNSUPPORTED_EXTENSION (void) isRequest; #else - if (!isRequest) + if (!isRequest) { + TLSX* extension; + if (TLSX_CheckUnsupportedExtension(ssl, TLSX_MAX_FRAGMENT_LENGTH)) return TLSX_HandleUnsupportedExtension(ssl); + + /* RFC 6066 Section 4: the server's response value must match the + * value the client requested. The request may have been configured on + * the WOLFSSL object or inherited from the WOLFSSL_CTX. */ + extension = TLSX_Find(ssl->extensions, TLSX_MAX_FRAGMENT_LENGTH); + if (extension == NULL) { + extension = TLSX_Find(ssl->ctx->extensions, + TLSX_MAX_FRAGMENT_LENGTH); + } + if (extension == NULL || extension->data == NULL || + ((byte*)extension->data)[0] != *input) { + SendAlert(ssl, alert_fatal, illegal_parameter); + WOLFSSL_ERROR_VERBOSE(UNKNOWN_MAX_FRAG_LEN_E); + return UNKNOWN_MAX_FRAG_LEN_E; + } + } #endif switch (*input) { diff --git a/src/tls13.c b/src/tls13.c index 8d7efb9df4..063b59b65e 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6100,11 +6100,24 @@ static int DoTls13CertificateRequest(WOLFSSL* ssl, const byte* input, return BUFFER_ERROR; /* INVALID_PARAMETER does not map to illegal_parameter in the central * alert path, so emit the alert explicitly before returning. */ - if (ssl->options.connectState < FINISHED_DONE && len > 0) { + if (ssl->options.connectState < FINISHED_DONE) { + /* RFC 8446 Section 4.3.2: in the handshake the context is zero + * length. */ + if (len > 0) { + SendAlert(ssl, alert_fatal, illegal_parameter); + WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); + return INVALID_PARAMETER; + } + } +#ifdef WOLFSSL_POST_HANDSHAKE_AUTH + else if (len == 0) { + /* RFC 8446 Section 4.3.2: a post-handshake CertificateRequest context + * MUST be non-empty and unique for the connection. */ SendAlert(ssl, alert_fatal, illegal_parameter); WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); return INVALID_PARAMETER; } +#endif #ifdef WOLFSSL_POST_HANDSHAKE_AUTH /* Remember the request context bytes; the CertReqCtx allocation and @@ -6113,6 +6126,18 @@ static int DoTls13CertificateRequest(WOLFSSL* ssl, const byte* input, */ reqCtxLen = len; reqCtxData = input + *inOutIdx; + /* Reject a context that duplicates one still pending on the connection. */ + if (ssl->options.connectState >= FINISHED_DONE) { + CertReqCtx* dup; + for (dup = ssl->certReqCtx; dup != NULL; dup = dup->next) { + if (dup->len == reqCtxLen && + XMEMCMP(&dup->ctx, reqCtxData, reqCtxLen) == 0) { + SendAlert(ssl, alert_fatal, illegal_parameter); + WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); + return INVALID_PARAMETER; + } + } + } #endif *inOutIdx += len; @@ -6134,6 +6159,14 @@ static int DoTls13CertificateRequest(WOLFSSL* ssl, const byte* input, } *inOutIdx += len; + /* RFC 8446 Section 4.3.2: the signature_algorithms extension MUST be + * present in a CertificateRequest. */ + if (peerSuites.hashSigAlgoSz == 0) { + SendAlert(ssl, alert_fatal, missing_extension); + WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); + return INVALID_PARAMETER; + } + #ifdef WOLFSSL_CERT_SETUP_CB if ((ret = CertSetupCbWrapper(ssl)) != 0) return ret; @@ -12172,8 +12205,16 @@ int SendTls13KeyUpdate(WOLFSSL* ssl) WOLFSSL_ENTER("SendTls13KeyUpdate"); #ifdef WOLFSSL_DTLS13 - if (ssl->options.dtls) + if (ssl->options.dtls) { + /* RFC 9147 Section 4.2.1: do not send a KeyUpdate that would advance + * the sending epoch beyond 2^48-1. */ + if (w64GTE(ssl->dtls13Epoch, + w64From32(DTLS13_EPOCH_MAX_HI32, DTLS13_EPOCH_MAX_LO32))) { + WOLFSSL_MSG("DTLS 1.3 sending epoch at maximum; refusing KeyUpdate"); + return BAD_STATE_E; + } i = Dtls13GetRlHeaderLength(ssl, 1) + DTLS_HANDSHAKE_HEADER_SZ; + } #endif /* WOLFSSL_DTLS13 */ outputSz = OPAQUE8_LEN + MAX_MSG_EXTRA; @@ -12307,7 +12348,18 @@ static int DoTls13KeyUpdate(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { - w64Increment(&ssl->dtls13PeerEpoch); + w64wrapper newEpoch = ssl->dtls13PeerEpoch; + w64Increment(&newEpoch); + + /* RFC 9147 Section 4.2.1: the epoch must not exceed 2^48-1. Reject a + * peer KeyUpdate that would advance the receiving epoch past the + * limit. Validate on a local copy so ssl->dtls13PeerEpoch is left + * untouched when the check fails. */ + if (w64GT(newEpoch, + w64From32(DTLS13_EPOCH_MAX_HI32, DTLS13_EPOCH_MAX_LO32))) + return BAD_STATE_E; + + ssl->dtls13PeerEpoch = newEpoch; ret = Dtls13SetEpochKeys(ssl, ssl->dtls13PeerEpoch, DECRYPT_SIDE_ONLY); if (ret != 0) diff --git a/tests/api.c b/tests/api.c index a2873fd747..183649f15c 100644 --- a/tests/api.c +++ b/tests/api.c @@ -19069,7 +19069,10 @@ static int test_wolfSSL_verify_result(void) WOLFSSL_CTX* ctx = NULL; long result = 0xDEADBEEF; - ExpectIntEQ(WC_NO_ERR_TRACE(WOLFSSL_FAILURE), wolfSSL_get_verify_result(ssl)); + /* A NULL ssl returns a non-zero verify error (not X509_V_OK) so the + * OpenSSL-idiomatic "!= X509_V_OK" check is not fooled. See F-4594. */ + ExpectIntEQ(WOLFSSL_X509_V_ERR_APPLICATION_VERIFICATION, + wolfSSL_get_verify_result(ssl)); ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method())); ExpectNotNull(ssl = SSL_new(ctx)); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 794808f38b..fb5f8a3233 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -5948,6 +5948,11 @@ enum { DTLS13_EPOCH_TRAFFIC0 = 3 }; +/* RFC 9147 Section 4.2.1: the DTLS 1.3 epoch is a 48-bit value and must not + * exceed 2^48-1. Expressed as the high/low 32-bit halves of a w64wrapper. */ +#define DTLS13_EPOCH_MAX_HI32 0x0000FFFFU +#define DTLS13_EPOCH_MAX_LO32 0xFFFFFFFFU + /* 64-bit epoch + 64-bit sequence number */ #define DTLS13_RN_SIZE (OPAQUE64_LEN + OPAQUE64_LEN) /* Maximum number of ACK records allowed in an ACK message */ diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 2281bb2f26..f46d0009af 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -2732,6 +2732,7 @@ enum { WOLFSSL_X509_V_ERR_PATH_LENGTH_EXCEEDED = 25, WOLFSSL_X509_V_ERR_CERT_REJECTED = 28, WOLFSSL_X509_V_ERR_SUBJECT_ISSUER_MISMATCH = 29, + WOLFSSL_X509_V_ERR_APPLICATION_VERIFICATION = 50, WOLFSSL_X509_V_ERR_HOSTNAME_MISMATCH = 62, WOLFSSL_X509_V_ERR_IP_ADDRESS_MISMATCH = 64, WOLFSSL_X509_V_ERR_INVALID_CA = 79,