From 18b3a80a5fd368f59c322fbb16b7dc92c1f11efa Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 8 Feb 2017 09:33:44 +0000 Subject: Fix crash in tls13_enc If s->s3->tmp.new_cipher is NULL then a crash can occur. This can happen if an alert gets sent after version negotiation (i.e. we have selected TLSv1.3 and ended up in tls13_enc), but before a ciphersuite has been selected. Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/2575) --- ssl/record/ssl3_record_tls13.c | 13 ++++++++++++- test/tls13encryptiontest.c | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/ssl/record/ssl3_record_tls13.c b/ssl/record/ssl3_record_tls13.c index 9dc7075cc2..d96a042ff9 100644 --- a/ssl/record/ssl3_record_tls13.c +++ b/ssl/record/ssl3_record_tls13.c @@ -7,6 +7,7 @@ * https://www.openssl.org/source/license.html */ +#include #include "../ssl_locl.h" #include "record_locl.h" @@ -29,7 +30,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int send) unsigned char *seq; int lenu, lenf; SSL3_RECORD *rec = &recs[0]; - uint32_t alg_enc = s->s3->tmp.new_cipher->algorithm_enc; + uint32_t alg_enc; if (n_recs != 1) { /* Should not happen */ @@ -52,8 +53,18 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int send) rec->input = rec->data; return 1; } + ivlen = EVP_CIPHER_CTX_iv_length(ctx); + /* + * To get here we must have selected a ciphersuite - otherwise ctx would + * be NULL + */ + assert(s->s3->tmp.new_cipher != NULL); + if (s->s3->tmp.new_cipher == NULL) + return -1; + alg_enc = s->s3->tmp.new_cipher->algorithm_enc; + if (alg_enc & SSL_AESCCM) { if (alg_enc & (SSL_AES128CCM8 | SSL_AES256CCM8)) taglen = EVP_CCM8_TLS_TAG_LEN; diff --git a/test/tls13encryptiontest.c b/test/tls13encryptiontest.c index 06bbbb3fb3..8229293615 100644 --- a/test/tls13encryptiontest.c +++ b/test/tls13encryptiontest.c @@ -34,6 +34,10 @@ typedef struct { static RECORD_DATA refdata[] = { { + /* + * Server: EncryptedExtensions, Certificate, CertificateVerify and + * Finished + */ { "0800001e001c000a00140012001d001700180019010001010102010301040000" "00000b0001b9000001b50001b0308201ac30820115a003020102020102300d06" @@ -85,6 +89,7 @@ static RECORD_DATA refdata[] = { "0000000000000000" }, { + /* Client: Finished */ { "1400002078367856d3c8cc4e0a95eb98906ca7a48bd3cc7029f48bd4ae0dc91a" "b903ca8916","","" @@ -98,6 +103,7 @@ static RECORD_DATA refdata[] = { "0000000000000000" }, { + /* Server: NewSessionTicket */ { "040000a60002a3004abe594b00924e535321cadc96238da09caf9b02fecafdd6" "5e3e418f03e43772cf512ed8066100503b1c08abbbf298a9d138ce821dd12fe1" @@ -119,6 +125,7 @@ static RECORD_DATA refdata[] = { "0000000000000000" }, { + /* Client: Application Data */ { "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f" "202122232425262728292a2b2c2d2e2f303117","","" @@ -133,6 +140,7 @@ static RECORD_DATA refdata[] = { "0000000000000000" }, { + /* Server: Application Data */ { "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f" "202122232425262728292a2b2c2d2e2f303117","","" @@ -147,6 +155,7 @@ static RECORD_DATA refdata[] = { "0000000000000001" }, { + /* Client: CloseNotify */ { "010015","","" }, @@ -158,6 +167,7 @@ static RECORD_DATA refdata[] = { "0000000000000001" }, { + /* Server: CloseNotify */ { "010015","","" }, @@ -281,6 +291,8 @@ static int test_record(SSL3_RECORD *rec, RECORD_DATA *recd, int enc) return ret; } +#define TLS13_AES_128_GCM_SHA256_BYTES ((const unsigned char *)"\x13\x01") + static int test_tls13_encryption(void) { SSL_CTX *ctx = NULL; @@ -312,6 +324,12 @@ static int test_tls13_encryption(void) goto err; } + s->s3->tmp.new_cipher = SSL_CIPHER_find(s, TLS13_AES_128_GCM_SHA256_BYTES); + if (s->s3->tmp.new_cipher == NULL) { + fprintf(stderr, "Failed to find cipher\n"); + goto err; + } + for (ctr = 0; ctr < OSSL_NELEM(refdata); ctr++) { /* Load the record */ ivlen = EVP_CIPHER_iv_length(ciph); -- cgit v1.2.3