Replace instances of OPENSSL_assert() with soft asserts in libssl

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3496)

ssl/record/rec_layer_d1.c

@@ -734,7 +734,8 @@ int dtls1_write_bytes(SSL *s, int type, const void *buf, size_t len,
 {
     int i;
 
-    OPENSSL_assert(len <= SSL3_RT_MAX_PLAIN_LENGTH);
+    if (!ossl_assert(len <= SSL3_RT_MAX_PLAIN_LENGTH))
+        return -1;
     s->rwstate = SSL_NOTHING;
     i = do_dtls1_write(s, type, buf, len, 0, written);
     return i;
@@ -757,9 +758,9 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf,
      * first check if there is a SSL3_BUFFER still being written out.  This
      * will happen with non blocking IO
      */
-    if (SSL3_BUFFER_get_left(wb) != 0) {
+    if (!ossl_assert(SSL3_BUFFER_get_left(wb) == 0)) {
-        OPENSSL_assert(0);      /* XDTLS: want to see if we ever get here */
+        SSLerr(SSL_F_DO_DTLS1_WRITE, ERR_R_INTERNAL_ERROR);
-        return ssl3_write_pending(s, type, buf, len, written);
+        return 0;
     }
 
     /* If we have an alert to send, lets send it */

ssl/record/record_locl.h

@@ -106,7 +106,7 @@ void SSL3_RECORD_set_seq_num(SSL3_RECORD *r, const unsigned char *seq_num);
 int ssl3_get_record(SSL *s);
 __owur int ssl3_do_compress(SSL *ssl, SSL3_RECORD *wr);
 __owur int ssl3_do_uncompress(SSL *ssl, SSL3_RECORD *rr);
-void ssl3_cbc_copy_mac(unsigned char *out,
+int ssl3_cbc_copy_mac(unsigned char *out,
                        const SSL3_RECORD *rec, size_t md_size);
 __owur int ssl3_cbc_remove_padding(SSL3_RECORD *rec,
                                    size_t block_size, size_t mac_size);

ssl/record/ssl3_record.c

@@ -515,7 +515,11 @@ int ssl3_get_record(SSL *s)
         unsigned char mac_tmp[EVP_MAX_MD_SIZE];
 
         mac_size = EVP_MD_CTX_size(s->read_hash);
-        OPENSSL_assert(mac_size <= EVP_MAX_MD_SIZE);
+        if (!ossl_assert(mac_size <= EVP_MAX_MD_SIZE)) {
+            al = SSL_AD_INTERNAL_ERROR;
+            SSLerr(SSL_F_SSL3_GET_RECORD, ERR_R_INTERNAL_ERROR);
+            goto f_err;
+        }
 
         for (j = 0; j < num_recs; j++) {
             thisrr = &rr[j];
@@ -542,7 +546,11 @@ int ssl3_get_record(SSL *s)
                  * contents of the padding bytes.
                  */
                 mac = mac_tmp;
-                ssl3_cbc_copy_mac(mac_tmp, thisrr, mac_size);
+                if (!ssl3_cbc_copy_mac(mac_tmp, thisrr, mac_size)) {
+                    al = SSL_AD_INTERNAL_ERROR;
+                    SSLerr(SSL_F_SSL3_GET_RECORD, ERR_R_INTERNAL_ERROR);
+                    goto f_err;
+                }
                 thisrr->length -= mac_size;
             } else {
                 /*
@@ -859,7 +867,10 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
     if (sending) {
         if (EVP_MD_CTX_md(s->write_hash)) {
             int n = EVP_MD_CTX_size(s->write_hash);
-            OPENSSL_assert(n >= 0);
+            if (!ossl_assert(n >= 0)) {
+                SSLerr(SSL_F_TLS1_ENC, ERR_R_INTERNAL_ERROR);
+                return -1;
+            }
         }
         ds = s->enc_write_ctx;
         if (s->enc_write_ctx == NULL)
@@ -892,7 +903,10 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
     } else {
         if (EVP_MD_CTX_md(s->read_hash)) {
             int n = EVP_MD_CTX_size(s->read_hash);
-            OPENSSL_assert(n >= 0);
+            if (!ossl_assert(n >= 0)) {
+                SSLerr(SSL_F_TLS1_ENC, ERR_R_INTERNAL_ERROR);
+                return -1;
+            }
         }
         ds = s->enc_read_ctx;
         if (s->enc_read_ctx == NULL)
@@ -1179,7 +1193,8 @@ int tls1_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int sending)
     }
 
     t = EVP_MD_CTX_size(hash);
-    OPENSSL_assert(t >= 0);
+    if (!ossl_assert(t >= 0))
+        return 0;
     md_size = t;
 
     /* I should fix this up TLS TLS TLS TLS TLS XXXXXXXX */
@@ -1404,7 +1419,7 @@ int tls1_cbc_remove_padding(const SSL *s,
  */
 #define CBC_MAC_ROTATE_IN_PLACE
 
-void ssl3_cbc_copy_mac(unsigned char *out,
+int ssl3_cbc_copy_mac(unsigned char *out,
                        const SSL3_RECORD *rec, size_t md_size)
 {
 #if defined(CBC_MAC_ROTATE_IN_PLACE)
@@ -1428,8 +1443,9 @@ void ssl3_cbc_copy_mac(unsigned char *out,
     size_t i, j;
     size_t rotate_offset;
 
-    OPENSSL_assert(rec->orig_len >= md_size);
+    if (!ossl_assert(rec->orig_len >= md_size
-    OPENSSL_assert(md_size <= EVP_MAX_MD_SIZE);
+                     && md_size <= EVP_MAX_MD_SIZE))
+        return 0;
 
 #if defined(CBC_MAC_ROTATE_IN_PLACE)
     rotated_mac = rotated_mac_buf + ((0 - (size_t)rotated_mac_buf) & 63);
@@ -1474,6 +1490,8 @@ void ssl3_cbc_copy_mac(unsigned char *out,
         rotate_offset &= constant_time_lt_s(rotate_offset, md_size);
     }
 #endif
+
+    return 1;
 }
 
 int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
@@ -1521,7 +1539,11 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
     if (SSL_READ_ETM(s) && s->read_hash) {
         unsigned char *mac;
         mac_size = EVP_MD_CTX_size(s->read_hash);
-        OPENSSL_assert(mac_size <= EVP_MAX_MD_SIZE);
+        if (!ossl_assert(mac_size <= EVP_MAX_MD_SIZE)) {
+            al = SSL_AD_INTERNAL_ERROR;
+            SSLerr(SSL_F_DTLS1_PROCESS_RECORD, ERR_R_INTERNAL_ERROR);
+            goto f_err;
+        }
         if (rr->orig_len < mac_size) {
             al = SSL_AD_DECODE_ERROR;
             SSLerr(SSL_F_DTLS1_PROCESS_RECORD, SSL_R_LENGTH_TOO_SHORT);
@@ -1576,7 +1598,11 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
             goto f_err;
         }
         mac_size = (size_t)imac_size;
-        OPENSSL_assert(mac_size <= EVP_MAX_MD_SIZE);
+        if (!ossl_assert(mac_size <= EVP_MAX_MD_SIZE)) {
+            al = SSL_AD_INTERNAL_ERROR;
+            SSLerr(SSL_F_DTLS1_PROCESS_RECORD, ERR_R_INTERNAL_ERROR);
+            goto f_err;
+        }
 
         /*
          * orig_len is the length of the record before any padding was
@@ -1601,7 +1627,11 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
              * contents of the padding bytes.
              */
             mac = mac_tmp;
-            ssl3_cbc_copy_mac(mac_tmp, rr, mac_size);
+            if (!ssl3_cbc_copy_mac(mac_tmp, rr, mac_size)) {
+                al = SSL_AD_INTERNAL_ERROR;
+                SSLerr(SSL_F_DTLS1_PROCESS_RECORD, ERR_R_INTERNAL_ERROR);
+                goto f_err;
+            }
             rr->length -= mac_size;
         } else {
             /*

ssl/s3_cbc.c

@@ -7,6 +7,7 @@
  * https://www.openssl.org/source/license.html
  */
 
+#include <assert.h>
 #include "internal/constant_time_locl.h"
 #include "ssl_locl.h"
 
@@ -165,7 +166,8 @@ int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
      * This is a, hopefully redundant, check that allows us to forget about
      * many possible overflows later in this function.
      */
-    OPENSSL_assert(data_plus_mac_plus_padding_size < 1024 * 1024);
+    if (!ossl_assert(data_plus_mac_plus_padding_size < 1024 * 1024))
+        return 0;
 
     switch (EVP_MD_CTX_type(ctx)) {
     case NID_md5:
@@ -227,15 +229,16 @@ int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
          * ssl3_cbc_record_digest_supported should have been called first to
          * check that the hash function is supported.
          */
-        OPENSSL_assert(0);
+        assert(0);
         if (md_out_size)
             *md_out_size = 0;
         return 0;
     }
 
-    OPENSSL_assert(md_length_size <= MAX_HASH_BIT_COUNT_BYTES);
+    if (!ossl_assert(md_length_size <= MAX_HASH_BIT_COUNT_BYTES
-    OPENSSL_assert(md_block_size <= MAX_HASH_BLOCK_SIZE);
+                    && md_block_size <= MAX_HASH_BLOCK_SIZE
-    OPENSSL_assert(md_size <= EVP_MAX_MD_SIZE);
+                    && md_size <= EVP_MAX_MD_SIZE))
+        return 0;
 
     header_length = 13;
     if (is_sslv3) {
@@ -331,7 +334,8 @@ int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
          */
         bits += 8 * md_block_size;
         memset(hmac_pad, 0, md_block_size);
-        OPENSSL_assert(mac_secret_length <= sizeof(hmac_pad));
+        if (!ossl_assert(mac_secret_length <= sizeof(hmac_pad)))
+            return 0;
         memcpy(hmac_pad, mac_secret, mac_secret_length);
         for (i = 0; i < md_block_size; i++)
             hmac_pad[i] ^= 0x36;

ssl/s3_enc.c

@@ -120,7 +120,10 @@ int ssl3_change_cipher_state(SSL *s, int which)
     c = s->s3->tmp.new_sym_enc;
     m = s->s3->tmp.new_hash;
     /* m == NULL will lead to a crash later */
-    OPENSSL_assert(m);
+    if (!ossl_assert(m != NULL)) {
+        SSLerr(SSL_F_SSL3_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR);
+        goto err2;
+    }
 #ifndef OPENSSL_NO_COMP
     if (s->s3->tmp.new_compression == NULL)
         comp = NULL;

ssl/ssl_ciph.c

@@ -371,7 +371,7 @@ static uint32_t disabled_mac_mask;
 static uint32_t disabled_mkey_mask;
 static uint32_t disabled_auth_mask;
 
-void ssl_load_ciphers(void)
+int ssl_load_ciphers(void)
 {
     size_t i;
     const ssl_cipher_table *t;
@@ -396,13 +396,16 @@ void ssl_load_ciphers(void)
             disabled_mac_mask |= t->mask;
         } else {
             int tmpsize = EVP_MD_size(md);
-            OPENSSL_assert(tmpsize >= 0);
+            if (!ossl_assert(tmpsize >= 0))
+                return 0;
             ssl_mac_secret_size[i] = tmpsize;
         }
     }
     /* Make sure we can access MD5 and SHA1 */
-    OPENSSL_assert(ssl_digest_methods[SSL_MD_MD5_IDX] != NULL);
+    if (!ossl_assert(ssl_digest_methods[SSL_MD_MD5_IDX] != NULL))
-    OPENSSL_assert(ssl_digest_methods[SSL_MD_SHA1_IDX] != NULL);
+        return 0;
+    if (!ossl_assert(ssl_digest_methods[SSL_MD_SHA1_IDX] != NULL))
+        return 0;
 
     disabled_mkey_mask = 0;
     disabled_auth_mask = 0;
@@ -460,6 +463,8 @@ void ssl_load_ciphers(void)
     if ((disabled_auth_mask & (SSL_aGOST01 | SSL_aGOST12)) ==
         (SSL_aGOST01 | SSL_aGOST12))
         disabled_mkey_mask |= SSL_kGOST;
+
+    return 1;
 }
 
 #ifndef OPENSSL_NO_COMP

ssl/ssl_init.c

@@ -96,7 +96,8 @@ DEFINE_RUN_ONCE_STATIC(ossl_init_ssl_base)
     SSL_COMP_get_compression_methods();
 #endif
     /* initialize cipher/digest methods table */
-    ssl_load_ciphers();
+    if (!ssl_load_ciphers())
+        return 0;
 
 #ifdef OPENSSL_INIT_DEBUG
     fprintf(stderr, "OPENSSL_INIT: ossl_init_ssl_base: "

ssl/ssl_lib.c

@@ -575,7 +575,8 @@ SSL *SSL_new(SSL_CTX *ctx)
     s->record_padding_arg = ctx->record_padding_arg;
     s->block_padding = ctx->block_padding;
     s->sid_ctx_length = ctx->sid_ctx_length;
-    OPENSSL_assert(s->sid_ctx_length <= sizeof s->sid_ctx);
+    if (!ossl_assert(s->sid_ctx_length <= sizeof s->sid_ctx))
+        goto err;
     memcpy(&s->sid_ctx, &ctx->sid_ctx, sizeof(s->sid_ctx));
     s->verify_callback = ctx->default_verify_callback;
     s->generate_session_id = ctx->generate_session_id;
@@ -3609,7 +3610,8 @@ SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx)
      * Program invariant: |sid_ctx| has fixed size (SSL_MAX_SID_CTX_LENGTH),
      * so setter APIs must prevent invalid lengths from entering the system.
      */
-    OPENSSL_assert(ssl->sid_ctx_length <= sizeof(ssl->sid_ctx));
+    if (!ossl_assert(ssl->sid_ctx_length <= sizeof(ssl->sid_ctx)))
+        return NULL;
 
     /*
      * If the session ID context matches that of the parent SSL_CTX,

ssl/ssl_locl.h

@@ -2136,7 +2136,7 @@ void ssl_set_masks(SSL *s);
 __owur STACK_OF(SSL_CIPHER) *ssl_get_ciphers_by_id(SSL *s);
 __owur int ssl_verify_alarm_type(long type);
 void ssl_sort_cipher_list(void);
-void ssl_load_ciphers(void);
+int ssl_load_ciphers(void);
 __owur int ssl_fill_hello_random(SSL *s, int server, unsigned char *field,
                                  size_t len, DOWNGRADE dgrd);
 __owur int ssl_generate_master_secret(SSL *s, unsigned char *pms, size_t pmslen,

ssl/statem/statem_dtls.c

@@ -32,7 +32,6 @@
 
 #define RSMBLY_BITMASK_IS_COMPLETE(bitmask, msg_len, is_complete) { \
                         long ii; \
-                        OPENSSL_assert((msg_len) > 0); \
                         is_complete = 1; \
                         if (bitmask[(((msg_len) - 1) >> 3)] != bitmask_end_values[((msg_len) & 7)]) is_complete = 0; \
                         if (is_complete) for (ii = (((msg_len) - 1) >> 3) - 1; ii >= 0 ; ii--) \
@@ -122,9 +121,11 @@ int dtls1_do_write(SSL *s, int type)
         /* should have something reasonable now */
         return -1;
 
-    if (s->init_off == 0 && type == SSL3_RT_HANDSHAKE)
+    if (s->init_off == 0 && type == SSL3_RT_HANDSHAKE) {
-        OPENSSL_assert(s->init_num ==
+        if (!ossl_assert(s->init_num ==
-                       s->d1->w_msg_hdr.msg_len + DTLS1_HM_HEADER_LENGTH);
+                         s->d1->w_msg_hdr.msg_len + DTLS1_HM_HEADER_LENGTH))
+            return -1;
+    }
 
     if (s->write_hash) {
         if (s->enc_write_ctx
@@ -254,7 +255,7 @@ int dtls1_do_write(SSL *s, int type)
                 } else
                     return -1;
             } else {
-                return (-1);
+                return -1;
             }
         } else {
 
@@ -262,7 +263,8 @@ int dtls1_do_write(SSL *s, int type)
              * bad if this assert fails, only part of the handshake message
              * got sent.  but why would this happen?
              */
-            OPENSSL_assert(len == written);
+            if (!ossl_assert(len == written))
+                return -1;
 
             if (type == SSL3_RT_HANDSHAKE && !s->d1->retransmitting) {
                 /*
@@ -578,6 +580,8 @@ dtls1_reassemble_fragment(SSL *s, const struct hm_header_st *msg_hdr)
     RSMBLY_BITMASK_MARK(frag->reassembly, (long)msg_hdr->frag_off,
                         (long)(msg_hdr->frag_off + frag_len));
 
+    if (!ossl_assert(msg_hdr->msg_len > 0))
+        goto err;
     RSMBLY_BITMASK_IS_COMPLETE(frag->reassembly, (long)msg_hdr->msg_len,
                                is_complete);
 
@@ -600,7 +604,8 @@ dtls1_reassemble_fragment(SSL *s, const struct hm_header_st *msg_hdr)
          * would have returned it and control would never have reached this
          * branch.
          */
-        OPENSSL_assert(item != NULL);
+        if (!ossl_assert(item != NULL))
+            goto err;
     }
 
     return DTLS1_HM_FRAGMENT_RETRY;
@@ -697,7 +702,8 @@ dtls1_process_out_of_seq_message(SSL *s, const struct hm_header_st *msg_hdr)
          * have been processed with |dtls1_reassemble_fragment|, above, or
          * the record will have been discarded.
          */
-        OPENSSL_assert(item != NULL);
+        if (!ossl_assert(item != NULL))
+            goto err;
     }
 
     return DTLS1_HM_FRAGMENT_RETRY;
@@ -981,7 +987,8 @@ int dtls1_buffer_message(SSL *s, int is_ccs)
      * this function is called immediately after a message has been
      * serialized
      */
-    OPENSSL_assert(s->init_off == 0);
+    if (!ossl_assert(s->init_off == 0))
+        return 0;
 
     frag = dtls1_hm_fragment_new(s->init_num, 0);
     if (frag == NULL)
@@ -991,13 +998,15 @@ int dtls1_buffer_message(SSL *s, int is_ccs)
 
     if (is_ccs) {
         /* For DTLS1_BAD_VER the header length is non-standard */
-        OPENSSL_assert(s->d1->w_msg_hdr.msg_len +
+        if (!ossl_assert(s->d1->w_msg_hdr.msg_len +
-                       ((s->version ==
+                         ((s->version ==
-                         DTLS1_BAD_VER) ? 3 : DTLS1_CCS_HEADER_LENGTH)
+                           DTLS1_BAD_VER) ? 3 : DTLS1_CCS_HEADER_LENGTH)
-                       == (unsigned int)s->init_num);
+                         == (unsigned int)s->init_num))
+            return 0;
     } else {
-        OPENSSL_assert(s->d1->w_msg_hdr.msg_len +
+        if (!ossl_assert(s->d1->w_msg_hdr.msg_len +
-                       DTLS1_HM_HEADER_LENGTH == (unsigned int)s->init_num);
+                         DTLS1_HM_HEADER_LENGTH == (unsigned int)s->init_num))
+            return 0;
     }
 
     frag->msg_header.msg_len = s->d1->w_msg_hdr.msg_len;
@@ -1045,11 +1054,6 @@ int dtls1_retransmit_message(SSL *s, unsigned short seq, int *found)
     unsigned char seq64be[8];
     struct dtls1_retransmit_state saved_state;
 
-    /*-
-      OPENSSL_assert(s->init_num == 0);
-      OPENSSL_assert(s->init_off == 0);
-     */
-
     /* XDTLS:  the requested message ought to be found, otherwise error */
     memset(seq64be, 0, sizeof(seq64be));
     seq64be[6] = (unsigned char)(seq >> 8);

ssl/statem/statem_lib.c

@@ -522,19 +522,23 @@ int tls_construct_finished(SSL *s, WPACKET *pkt)
      */
     if (!SSL_IS_TLS13(s) && !ssl_log_secret(s, MASTER_SECRET_LABEL,
                                             s->session->master_key,
-                                            s->session->master_key_length))
+                                            s->session->master_key_length)) {
-        return 0;
+        SSLerr(SSL_F_TLS_CONSTRUCT_FINISHED, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
     /*
      * Copy the finished so we can use it for renegotiation checks
      */
+    if (!ossl_assert(finish_md_len <= EVP_MAX_MD_SIZE)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_FINISHED, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
     if (!s->server) {
-        OPENSSL_assert(finish_md_len <= EVP_MAX_MD_SIZE);
         memcpy(s->s3->previous_client_finished, s->s3->tmp.finish_md,
                finish_md_len);
         s->s3->previous_client_finished_len = finish_md_len;
     } else {
-        OPENSSL_assert(finish_md_len <= EVP_MAX_MD_SIZE);
         memcpy(s->s3->previous_server_finished, s->s3->tmp.finish_md,
                finish_md_len);
         s->s3->previous_server_finished_len = finish_md_len;
@@ -765,13 +769,16 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt)
     /*
      * Copy the finished so we can use it for renegotiation checks
      */
+    if (!ossl_assert(md_len <= EVP_MAX_MD_SIZE)) {
+        al = SSL_AD_INTERNAL_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_FINISHED, ERR_R_INTERNAL_ERROR);
+        goto f_err;
+    }
     if (s->server) {
-        OPENSSL_assert(md_len <= EVP_MAX_MD_SIZE);
         memcpy(s->s3->previous_client_finished, s->s3->tmp.peer_finish_md,
                md_len);
         s->s3->previous_client_finished_len = md_len;
     } else {
-        OPENSSL_assert(md_len <= EVP_MAX_MD_SIZE);
         memcpy(s->s3->previous_server_finished, s->s3->tmp.peer_finish_md,
                md_len);
         s->s3->previous_server_finished_len = md_len;