From 310ffd004592bb5eeda70ba28f8deb907ce39574 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Tue, 27 Nov 2018 17:27:31 +1000 Subject: [PATCH] Check for TLS 1.3 version in the method for extenstions. During parsing of ClientHello, ServerHello and HelloRetryRequest, the SSL object version may not be set to the negotiated version. --- src/tls.c | 110 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 69 insertions(+), 41 deletions(-) diff --git a/src/tls.c b/src/tls.c index f31dd0102..fd7fc553f 100644 --- a/src/tls.c +++ b/src/tls.c @@ -6837,7 +6837,7 @@ static int TLSX_KeyShare_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType) { int ret; - KeyShareEntry *keyShareEntry; + KeyShareEntry *keyShareEntry = NULL; word16 group; if (msgType == client_hello) { @@ -6897,7 +6897,7 @@ static int TLSX_KeyShare_Parse(WOLFSSL* ssl, byte* input, word16 length, return BUFFER_ERROR; /* Not in list sent if there isn't a private key. */ - if (keyShareEntry->key == NULL) + if (keyShareEntry == NULL || keyShareEntry->key == NULL) return BAD_KEY_SHARE_DATA; /* Process the entry to calculate the secret. */ @@ -9789,11 +9789,15 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("SNI extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && msgType != client_hello && msgType != encrypted_extensions) { return EXT_NOT_ALLOWED; } + else if (!IsAtLeastTLSv1_3(ssl->version) && + msgType == encrypted_extensions) { + return EXT_NOT_ALLOWED; + } #endif ret = SNI_PARSE(ssl, input + offset, size, isRequest); break; @@ -9802,11 +9806,15 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("Max Fragment Length extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && msgType != client_hello && msgType != encrypted_extensions) { return EXT_NOT_ALLOWED; } + else if (!IsAtLeastTLSv1_3(ssl->version) && + msgType == encrypted_extensions) { + return EXT_NOT_ALLOWED; + } #endif ret = MFL_PARSE(ssl, input + offset, size, isRequest); break; @@ -9815,8 +9823,10 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("Truncated HMAC extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && !ssl->options.downgrade) + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && + !ssl->options.downgrade) { break; + } #endif ret = THM_PARSE(ssl, input + offset, size, isRequest); break; @@ -9825,11 +9835,15 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("Supported Groups extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && msgType != client_hello && msgType != encrypted_extensions) { return EXT_NOT_ALLOWED; } + else if (!IsAtLeastTLSv1_3(ssl->version) && + msgType == encrypted_extensions) { + return EXT_NOT_ALLOWED; + } #endif ret = EC_PARSE(ssl, input + offset, size, isRequest); break; @@ -9838,8 +9852,10 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("Point Formats extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && !ssl->options.downgrade) + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && + !ssl->options.downgrade) { break; + } #endif ret = PF_PARSE(ssl, input + offset, size, isRequest); break; @@ -9848,8 +9864,10 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("Certificate Status Request extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && !ssl->options.downgrade) + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && + !ssl->options.downgrade) { break; + } #endif ret = CSR_PARSE(ssl, input + offset, size, isRequest); break; @@ -9858,7 +9876,7 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("Certificate Status Request v2 extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && msgType != client_hello && msgType != certificate_request && msgType != certificate) { @@ -9873,8 +9891,10 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("Extended Master Secret extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && !ssl->options.downgrade) + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && + !ssl->options.downgrade) { break; + } #endif #ifndef NO_WOLFSSL_SERVER if (isRequest) @@ -9888,8 +9908,10 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("Secure Renegotiation extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && !ssl->options.downgrade) + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && + !ssl->options.downgrade) { break; + } #endif ret = SCR_PARSE(ssl, input + offset, size, isRequest); break; @@ -9910,8 +9932,10 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("Quantum-Safe-Hybrid extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && !ssl->options.downgrade) + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && + !ssl->options.downgrade) { break; + } #endif ret = QSH_PARSE(ssl, input + offset, size, isRequest); break; @@ -9920,11 +9944,15 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, WOLFSSL_MSG("ALPN extension received"); #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && msgType != client_hello && msgType != encrypted_extensions) { return EXT_NOT_ALLOWED; } + else if (!IsAtLeastTLSv1_3(ssl->version) && + msgType == encrypted_extensions) { + return EXT_NOT_ALLOWED; + } #endif ret = ALPN_PARSE(ssl, input + offset, size, isRequest); break; @@ -9936,7 +9964,7 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, break; #ifdef WOLFSSL_TLS13 - if (IsAtLeastTLSv1_3(ssl->version) && + if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && msgType != client_hello && msgType != certificate_request) { return EXT_NOT_ALLOWED; @@ -9952,7 +9980,7 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, if (!IsAtLeastTLSv1_3(ssl->ctx->method->version)) break; - if (IsAtLeastTLSv1_3(ssl->version) && + if ( #ifdef WOLFSSL_TLS13_DRAFT_18 msgType != client_hello #else @@ -9969,14 +9997,14 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, case TLSX_COOKIE: WOLFSSL_MSG("Cookie extension received"); - if (!IsAtLeastTLSv1_3(ssl->version)) + if (!IsAtLeastTLSv1_3(ssl->ctx->method->version)) break; - if (IsAtLeastTLSv1_3(ssl->version) && - msgType != client_hello && + if (msgType != client_hello && msgType != hello_retry_request) { return EXT_NOT_ALLOWED; } + ret = CKE_PARSE(ssl, input + offset, size, msgType); break; @@ -9987,11 +10015,9 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, if (!IsAtLeastTLSv1_3(ssl->ctx->method->version)) break; - if (IsAtLeastTLSv1_3(ssl->version) && - msgType != client_hello && - msgType != server_hello) { + if (msgType != client_hello && msgType != server_hello) return EXT_NOT_ALLOWED; - } + ret = PSK_PARSE(ssl, input + offset, size, msgType); pskDone = 1; break; @@ -9999,13 +10025,12 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, case TLSX_PSK_KEY_EXCHANGE_MODES: WOLFSSL_MSG("PSK Key Exchange Modes extension received"); - if (!IsAtLeastTLSv1_3(ssl->version)) + if (!IsAtLeastTLSv1_3(ssl->ctx->method->version)) break; - if (IsAtLeastTLSv1_3(ssl->version) && - msgType != client_hello) { + if (msgType != client_hello) return EXT_NOT_ALLOWED; - } + ret = PKM_PARSE(ssl, input + offset, size, msgType); break; #endif @@ -10014,13 +10039,16 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, case TLSX_EARLY_DATA: WOLFSSL_MSG("Early Data extension received"); - if (!IsAtLeastTLSv1_3(ssl->version)) + if (!IsAtLeastTLSv1_3(ssl->ctx->method->version)) break; - if (IsAtLeastTLSv1_3(ssl->version) && - msgType != client_hello && - msgType != session_ticket && - msgType != encrypted_extensions) { + if (msgType != client_hello && msgType != session_ticket && + msgType != encrypted_extensions) { + return EXT_NOT_ALLOWED; + } + if (!IsAtLeastTLSv1_3(ssl->version) && + (msgType == session_ticket || + msgType == encrypted_extensions)) { return EXT_NOT_ALLOWED; } ret = EDI_PARSE(ssl, input + offset, size, msgType); @@ -10031,13 +10059,12 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, case TLSX_POST_HANDSHAKE_AUTH: WOLFSSL_MSG("Post Handshake Authentication extension received"); - if (!IsAtLeastTLSv1_3(ssl->version)) + if (!IsAtLeastTLSv1_3(ssl->ctx->method->version)) break; - if (IsAtLeastTLSv1_3(ssl->version) && - msgType != client_hello) { + if (msgType != client_hello) return EXT_NOT_ALLOWED; - } + ret = PHA_PARSE(ssl, input + offset, size, msgType); break; #endif @@ -10046,14 +10073,17 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, case TLSX_SIGNATURE_ALGORITHMS_CERT: WOLFSSL_MSG("Signature Algorithms extension received"); - if (!IsAtLeastTLSv1_3(ssl->version)) + if (!IsAtLeastTLSv1_3(ssl->ctx->method->version)) break; - if (IsAtLeastTLSv1_3(ssl->version) && - msgType != client_hello && + if (msgType != client_hello && msgType != certificate_request) { return EXT_NOT_ALLOWED; } + if (!IsAtLeastTLSv1_3(ssl->version) && + msgType == certificate_request) { + return EXT_NOT_ALLOWED; + } ret = SAC_PARSE(ssl, input + offset, size, isRequest); break; @@ -10065,9 +10095,7 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, if (!IsAtLeastTLSv1_3(ssl->ctx->method->version)) break; - if (IsAtLeastTLSv1_3(ssl->ctx->method->version) && - msgType != client_hello && - msgType != server_hello && + if (msgType != client_hello && msgType != server_hello && msgType != hello_retry_request) { return EXT_NOT_ALLOWED; }