Handle a memory allocation failure in ssl3_init_finished_mac()

The ssl3_init_finished_mac() function can fail, in which case we need to
propagate the error up through the stack.

RT#3198

Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index c6c357642ab1590a6f4cd019d0913f05211f2ea7..2779fff03a501c54d4168307c527cf4b45311993 100644 (file)
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2047,6 +2047,7 @@ void ERR_load_SSL_strings(void);
 # define SSL_F_SSL3_GENERATE_KEY_BLOCK                    238
 # define SSL_F_SSL3_GENERATE_MASTER_SECRET                388
 # define SSL_F_SSL3_GET_RECORD                            143
+# define SSL_F_SSL3_INIT_FINISHED_MAC                     339
 # define SSL_F_SSL3_OUTPUT_CERT_CHAIN                     147
 # define SSL_F_SSL3_READ_BYTES                            148
 # define SSL_F_SSL3_READ_N                                149

diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c
index cb571c1d07ff3b6686ecbf91440a413f46a07d55..f7089bd6fb858b4bba704ac31e578ec710060948 100644 (file)
--- a/ssl/s3_enc.c
+++ b/ssl/s3_enc.c
@@ -326,11 +326,18 @@ void ssl3_cleanup_key_block(SSL *s)
     s->s3->tmp.key_block_length = 0;
 }
 
-void ssl3_init_finished_mac(SSL *s)
+int ssl3_init_finished_mac(SSL *s)
 {
+    BIO *buf = BIO_new(BIO_s_mem());
+
+    if (buf == NULL) {
+        SSLerr(SSL_F_SSL3_INIT_FINISHED_MAC, ERR_R_MALLOC_FAILURE);
+        return 0;
+    }
     ssl3_free_digest_list(s);
-    s->s3->handshake_buffer = BIO_new(BIO_s_mem());
+    s->s3->handshake_buffer = buf;
     (void)BIO_set_close(s->s3->handshake_buffer, BIO_CLOSE);
+    return 1;
 }
 
 /*

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 377061b47d8fb59135f9bd7f7097b7fbee9c61ad..0c46768482fd4ed7d5ffea1d3fe63bd9b35ff6ce 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -60,6 +60,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_SSL3_GENERATE_MASTER_SECRET),
      "ssl3_generate_master_secret"},
     {ERR_FUNC(SSL_F_SSL3_GET_RECORD), "ssl3_get_record"},
+    {ERR_FUNC(SSL_F_SSL3_INIT_FINISHED_MAC), "ssl3_init_finished_mac"},
     {ERR_FUNC(SSL_F_SSL3_OUTPUT_CERT_CHAIN), "ssl3_output_cert_chain"},
     {ERR_FUNC(SSL_F_SSL3_READ_BYTES), "ssl3_read_bytes"},
     {ERR_FUNC(SSL_F_SSL3_READ_N), "ssl3_read_n"},

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 243535fe50c6eb8fb72b77bc4d9a846636b230a6..35fd3fc7ac1cec5b62df1fac97fbafd859f2ed74 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1859,7 +1859,7 @@ __owur EVP_PKEY *ssl_dh_to_pkey(DH *dh);
 
 __owur const SSL_CIPHER *ssl3_get_cipher_by_char(const unsigned char *p);
 __owur int ssl3_put_cipher_by_char(const SSL_CIPHER *c, unsigned char *p);
-void ssl3_init_finished_mac(SSL *s);
+int ssl3_init_finished_mac(SSL *s);
 __owur int ssl3_setup_key_block(SSL *s);
 __owur int ssl3_change_cipher_state(SSL *s, int which);
 void ssl3_cleanup_key_block(SSL *s);

diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c
index 0b0595d237ea81863e249356d2a23c0cd1a53a78..28483e7944ff6e7cd0bfc1a59bf732d1cfad01a3 100644 (file)
--- a/ssl/statem/statem.c
+++ b/ssl/statem/statem.c
@@ -332,8 +332,12 @@ static int state_machine(SSL *s, int server)
                 goto end;
             }
 
-        if (!server || st->state != MSG_FLOW_RENEGOTIATE)
-            ssl3_init_finished_mac(s);
+        if (!server || st->state != MSG_FLOW_RENEGOTIATE) {
+            if (!ssl3_init_finished_mac(s)) {
+                ossl_statem_set_error(s);
+                goto end;
+            }
+        }
 
         if (server) {
             if (st->state != MSG_FLOW_RENEGOTIATE) {

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index ecbc43b3d09fae4b8957ccb8a6cdd835a06cdfd8..a97f0cc6bf2c9b5a2625f741032c5a8a03a4b1f6 100644 (file)
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -391,7 +391,10 @@ WORK_STATE ossl_statem_client_pre_work(SSL *s, WORK_STATE wst)
         s->shutdown = 0;
         if (SSL_IS_DTLS(s)) {
             /* every DTLS ClientHello resets Finished MAC */
-            ssl3_init_finished_mac(s);
+            if (!ssl3_init_finished_mac(s)) {
+                ossl_statem_set_error(s);
+                return WORK_ERROR;
+            }
         }
         break;
 

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 67df5f2640d27f794beb216e731c8a3ebc33297f..71dd27f7bc4457b0dabdeeee24c792b006c2ada4 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -496,15 +496,20 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
     case TLS_ST_SW_HELLO_REQ:
         if (statem_flush(s) != 1)
             return WORK_MORE_A;
-        ssl3_init_finished_mac(s);
+        if (!ssl3_init_finished_mac(s)) {
+            ossl_statem_set_error(s);
+            return WORK_ERROR;
+        }
         break;
 
     case DTLS_ST_SW_HELLO_VERIFY_REQUEST:
         if (statem_flush(s) != 1)
             return WORK_MORE_A;
         /* HelloVerifyRequest resets Finished MAC */
-        if (s->version != DTLS1_BAD_VER)
-            ssl3_init_finished_mac(s);
+        if (s->version != DTLS1_BAD_VER && !ssl3_init_finished_mac(s)) {
+            ossl_statem_set_error(s);
+            return WORK_ERROR;
+        }
         /*
          * The next message should be another ClientHello which we need to
          * treat like it was the first packet