From f63a17d66dec01c123630682e0b20450b34c086a Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 21 Nov 2017 17:18:43 +0000 Subject: Convert the state machine code to use SSLfatal() Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/4778) --- ssl/t1_enc.c | 87 ++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 55 insertions(+), 32 deletions(-) (limited to 'ssl/t1_enc.c') diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 8fe2dfd3aa..24978353ff 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -113,15 +113,18 @@ int tls1_change_cipher_state(SSL *s, int which) else s->mac_flags &= ~SSL_MAC_FLAG_READ_MAC_STREAM; - if (s->enc_read_ctx != NULL) + if (s->enc_read_ctx != NULL) { reuse_dd = 1; - else if ((s->enc_read_ctx = EVP_CIPHER_CTX_new()) == NULL) + } else if ((s->enc_read_ctx = EVP_CIPHER_CTX_new()) == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_MALLOC_FAILURE); goto err; - else + } else { /* * make sure it's initialised in case we exit later with an error */ EVP_CIPHER_CTX_reset(s->enc_read_ctx); + } dd = s->enc_read_ctx; mac_ctx = ssl_replace_hash(&s->read_hash, NULL); if (mac_ctx == NULL) @@ -132,9 +135,10 @@ int tls1_change_cipher_state(SSL *s, int which) if (comp != NULL) { s->expand = COMP_CTX_new(comp->method); if (s->expand == NULL) { - SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, - SSL_R_COMPRESSION_LIBRARY_ERROR); - goto err2; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_TLS1_CHANGE_CIPHER_STATE, + SSL_R_COMPRESSION_LIBRARY_ERROR); + goto err; } } #endif @@ -155,20 +159,31 @@ int tls1_change_cipher_state(SSL *s, int which) s->mac_flags |= SSL_MAC_FLAG_WRITE_MAC_STREAM; else s->mac_flags &= ~SSL_MAC_FLAG_WRITE_MAC_STREAM; - if (s->enc_write_ctx != NULL && !SSL_IS_DTLS(s)) + if (s->enc_write_ctx != NULL && !SSL_IS_DTLS(s)) { reuse_dd = 1; - else if ((s->enc_write_ctx = EVP_CIPHER_CTX_new()) == NULL) + } else if ((s->enc_write_ctx = EVP_CIPHER_CTX_new()) == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_MALLOC_FAILURE); goto err; + } dd = s->enc_write_ctx; if (SSL_IS_DTLS(s)) { mac_ctx = EVP_MD_CTX_new(); - if (mac_ctx == NULL) + if (mac_ctx == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_MALLOC_FAILURE); goto err; + } s->write_hash = mac_ctx; } else { mac_ctx = ssl_replace_hash(&s->write_hash, NULL); - if (mac_ctx == NULL) + if (mac_ctx == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_MALLOC_FAILURE); goto err; + } } #ifndef OPENSSL_NO_COMP COMP_CTX_free(s->compress); @@ -176,9 +191,10 @@ int tls1_change_cipher_state(SSL *s, int which) if (comp != NULL) { s->compress = COMP_CTX_new(comp->method); if (s->compress == NULL) { - SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, - SSL_R_COMPRESSION_LIBRARY_ERROR); - goto err2; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_TLS1_CHANGE_CIPHER_STATE, + SSL_R_COMPRESSION_LIBRARY_ERROR); + goto err; } } #endif @@ -227,8 +243,9 @@ int tls1_change_cipher_state(SSL *s, int which) } if (n > s->s3->tmp.key_block_length) { - SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); - goto err2; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_INTERNAL_ERROR); + goto err; } memcpy(mac_secret, ms, i); @@ -240,8 +257,9 @@ int tls1_change_cipher_state(SSL *s, int which) if (mac_key == NULL || EVP_DigestSignInit(mac_ctx, NULL, m, NULL, mac_key) <= 0) { EVP_PKEY_free(mac_key); - SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); - goto err2; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_INTERNAL_ERROR); + goto err; } EVP_PKEY_free(mac_key); } @@ -258,8 +276,9 @@ int tls1_change_cipher_state(SSL *s, int which) if (!EVP_CipherInit_ex(dd, c, NULL, key, NULL, (which & SSL3_CC_WRITE)) || !EVP_CIPHER_CTX_ctrl(dd, EVP_CTRL_GCM_SET_IV_FIXED, (int)k, iv)) { - SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); - goto err2; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_INTERNAL_ERROR); + goto err; } } else if (EVP_CIPHER_mode(c) == EVP_CIPH_CCM_MODE) { int taglen; @@ -273,21 +292,24 @@ int tls1_change_cipher_state(SSL *s, int which) || !EVP_CIPHER_CTX_ctrl(dd, EVP_CTRL_AEAD_SET_TAG, taglen, NULL) || !EVP_CIPHER_CTX_ctrl(dd, EVP_CTRL_CCM_SET_IV_FIXED, (int)k, iv) || !EVP_CipherInit_ex(dd, NULL, NULL, key, NULL, -1)) { - SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); - goto err2; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_INTERNAL_ERROR); + goto err; } } else { if (!EVP_CipherInit_ex(dd, c, NULL, key, iv, (which & SSL3_CC_WRITE))) { - SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); - goto err2; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_INTERNAL_ERROR); + goto err; } } /* Needed for "composite" AEADs, such as RC4-HMAC-MD5 */ if ((EVP_CIPHER_flags(c) & EVP_CIPH_FLAG_AEAD_CIPHER) && *mac_secret_size && !EVP_CIPHER_CTX_ctrl(dd, EVP_CTRL_AEAD_SET_MAC_KEY, (int)*mac_secret_size, mac_secret)) { - SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); - goto err2; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_CHANGE_CIPHER_STATE, + ERR_R_INTERNAL_ERROR); + goto err; } #ifdef SSL_DEBUG @@ -312,8 +334,6 @@ int tls1_change_cipher_state(SSL *s, int which) OPENSSL_cleanse(iv2, sizeof(iv2)); return 1; err: - SSLerr(SSL_F_TLS1_CHANGE_CIPHER_STATE, ERR_R_MALLOC_FAILURE); - err2: OPENSSL_cleanse(tmp1, sizeof(tmp1)); OPENSSL_cleanse(tmp2, sizeof(tmp1)); OPENSSL_cleanse(iv1, sizeof(iv1)); @@ -336,7 +356,8 @@ int tls1_setup_key_block(SSL *s) if (!ssl_cipher_get_evp(s->session, &c, &hash, &mac_type, &mac_secret_size, &comp, s->ext.use_etm)) { - SSLerr(SSL_F_TLS1_SETUP_KEY_BLOCK, SSL_R_CIPHER_OR_HASH_UNAVAILABLE); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_SETUP_KEY_BLOCK, + SSL_R_CIPHER_OR_HASH_UNAVAILABLE); return 0; } @@ -350,7 +371,8 @@ int tls1_setup_key_block(SSL *s) ssl3_cleanup_key_block(s); if ((p = OPENSSL_malloc(num)) == NULL) { - SSLerr(SSL_F_TLS1_SETUP_KEY_BLOCK, ERR_R_MALLOC_FAILURE); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_SETUP_KEY_BLOCK, + ERR_R_MALLOC_FAILURE); goto err; } @@ -446,10 +468,11 @@ int tls1_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p, * affect client auth because we're freezing the buffer at the same * point (after client key exchange and before certificate verify) */ - if (!ssl3_digest_cached_records(s, 1)) - return 0; - if (!ssl_handshake_hash(s, hash, sizeof(hash), &hashlen)) + if (!ssl3_digest_cached_records(s, 1) + || !ssl_handshake_hash(s, hash, sizeof(hash), &hashlen)) { + /* SSLfatal() already called */ return 0; + } #ifdef SSL_DEBUG fprintf(stderr, "Handshake hashes:\n"); BIO_dump_fp(stderr, (char *)hash, hashlen); -- cgit v1.2.3