Rationalise RECORD_LAYER_clear() and clear_record_layer()

We had two functions which were very similarly named, that did almost the
same thing, but not quite. We bring the two together. Doing this also fixes
a possible bug where some data may not be correctly freed when the
RECORD_LAYER_clear() version was used.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23256)

(cherry picked from commit 4a0e4849af1588dfe9d7e01738acc96799b83447)

3 files changed, 51 insertions, 53 deletions

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 63a77fceaa..2fa841e8a9 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -25,8 +25,17 @@ void RECORD_LAYER_init(RECORD_LAYER *rl, SSL_CONNECTION *s)
     rl->s = s;
 }
 
-void RECORD_LAYER_clear(RECORD_LAYER *rl)
+int RECORD_LAYER_clear(RECORD_LAYER *rl)
 {
+    int ret = 1;
+
+    /* Clear any buffered records we no longer need */
+    while (rl->curr_rec < rl->num_recs)
+        ret &= ssl_release_record(rl->s,
+                                  &(rl->tlsrecs[rl->curr_rec++]),
+                                  0);
+
+
     rl->wnum = 0;
     memset(rl->handshake_fragment, 0, sizeof(rl->handshake_fragment));
     rl->handshake_fragment_len = 0;
@@ -34,6 +43,12 @@ void RECORD_LAYER_clear(RECORD_LAYER *rl)
     rl->wpend_type = 0;
     rl->wpend_ret = 0;
     rl->wpend_buf = NULL;
+    rl->alert_count = 0;
+    rl->num_recs = 0;
+    rl->curr_rec = 0;
+
+    BIO_free(rl->rrlnext);
+    rl->rrlnext = NULL;
 
     if (rl->rrlmethod != NULL)
         rl->rrlmethod->free(rl->rrl); /* Ignore return value */
@@ -48,6 +63,35 @@ void RECORD_LAYER_clear(RECORD_LAYER *rl)
 
     if (rl->d)
         DTLS_RECORD_LAYER_clear(rl);
+
+    return ret;
+}
+
+int RECORD_LAYER_reset(RECORD_LAYER *rl)
+{
+    int ret;
+
+    ret = RECORD_LAYER_clear(rl);
+
+    /* We try and reset both record layers even if one fails */
+    ret &= ssl_set_new_record_layer(rl->s,
+                                    SSL_CONNECTION_IS_DTLS(rl->s)
+                                        ? DTLS_ANY_VERSION : TLS_ANY_VERSION,
+                                    OSSL_RECORD_DIRECTION_READ,
+                                    OSSL_RECORD_PROTECTION_LEVEL_NONE, NULL, 0,
+                                    NULL, 0, NULL, 0, NULL,  0, NULL, 0,
+                                    NID_undef, NULL, NULL, NULL);
+
+    ret &= ssl_set_new_record_layer(rl->s,
+                                    SSL_CONNECTION_IS_DTLS(rl->s)
+                                        ? DTLS_ANY_VERSION : TLS_ANY_VERSION,
+                                    OSSL_RECORD_DIRECTION_WRITE,
+                                    OSSL_RECORD_PROTECTION_LEVEL_NONE, NULL, 0,
+                                    NULL, 0, NULL, 0, NULL,  0, NULL, 0,
+                                    NID_undef, NULL, NULL, NULL);
+
+    /* SSLfatal already called in the event of failure */
+    return ret;
 }
 
 /* Checks if we have unprocessed read ahead data pending */
diff --git a/ssl/record/record.h b/ssl/record/record.h
index 6fb579fe19..e91cf9042e 100644
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -142,7 +142,8 @@ typedef struct record_layer_st {
 #define DTLS_RECORD_LAYER_get_w_epoch(rl)       ((rl)->d->w_epoch)
 
 void RECORD_LAYER_init(RECORD_LAYER *rl, SSL_CONNECTION *s);
-void RECORD_LAYER_clear(RECORD_LAYER *rl);
+int RECORD_LAYER_clear(RECORD_LAYER *rl);
+int RECORD_LAYER_reset(RECORD_LAYER *rl);
 int RECORD_LAYER_read_pending(const RECORD_LAYER *rl);
 int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl);
 int RECORD_LAYER_write_pending(const RECORD_LAYER *rl);
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index df113846d9..00e7ddbf27 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -558,50 +558,6 @@ static int ssl_check_allowed_versions(int min_version, int max_version)
 void OPENSSL_VPROC_FUNC(void) {}
 #endif
 
-static int clear_record_layer(SSL_CONNECTION *s)
-{
-    int ret = 1;
-
-    /* Clear any buffered records we no longer need */
-    while (s->rlayer.curr_rec < s->rlayer.num_recs)
-        ret &= ssl_release_record(s,
-                                  &(s->rlayer.tlsrecs[s->rlayer.curr_rec ++]),
-                                  0);
-
-    BIO_free(s->rlayer.rrlnext);
-    s->rlayer.rrlnext = NULL;
-
-    /* Reset various fields */
-    s->rlayer.wnum = 0;
-    s->rlayer.handshake_fragment_len = 0;
-    s->rlayer.wpend_tot = 0;
-    s->rlayer.wpend_type = 0;
-    s->rlayer.wpend_buf = NULL;
-    s->rlayer.alert_count = 0;
-    s->rlayer.num_recs = 0;
-    s->rlayer.curr_rec = 0;
-
-    /* We try and reset both record layers even if one fails */
-    ret &= ssl_set_new_record_layer(s,
-                                    SSL_CONNECTION_IS_DTLS(s) ? DTLS_ANY_VERSION
-                                                              : TLS_ANY_VERSION,
-                                    OSSL_RECORD_DIRECTION_READ,
-                                    OSSL_RECORD_PROTECTION_LEVEL_NONE, NULL, 0,
-                                    NULL, 0, NULL, 0, NULL,  0, NULL, 0,
-                                    NID_undef, NULL, NULL, NULL);
-
-    ret &= ssl_set_new_record_layer(s,
-                                    SSL_CONNECTION_IS_DTLS(s) ? DTLS_ANY_VERSION
-                                                              : TLS_ANY_VERSION,
-                                    OSSL_RECORD_DIRECTION_WRITE,
-                                    OSSL_RECORD_PROTECTION_LEVEL_NONE, NULL, 0,
-                                    NULL, 0, NULL, 0, NULL,  0, NULL, 0,
-                                    NID_undef, NULL, NULL, NULL);
-
-    /* SSLfatal already called in the event of failure */
-    return ret;
-}
-
 int SSL_clear(SSL *s)
 {
     if (s->method == NULL) {
@@ -687,11 +643,7 @@ int ossl_ssl_connection_reset(SSL *s)
             return 0;
     }
 
-    RECORD_LAYER_clear(&sc->rlayer);
-    BIO_free(sc->rlayer.rrlnext);
-    sc->rlayer.rrlnext = NULL;
-
-    if (!clear_record_layer(sc))
+    if (!RECORD_LAYER_reset(&sc->rlayer))
         return 0;
 
     return 1;
@@ -1455,6 +1407,7 @@ void ossl_ssl_connection_free(SSL *ssl)
     /* Ignore return value */
     ssl_free_wbio_buffer(s);
 
+    /* Ignore return value */
     RECORD_LAYER_clear(&s->rlayer);
 
     BUF_MEM_free(s->init_buf);
@@ -4783,7 +4736,7 @@ void SSL_set_accept_state(SSL *s)
     ossl_statem_clear(sc);
     sc->handshake_func = s->method->ssl_accept;
     /* Ignore return value. Its a void public API function */
-    clear_record_layer(sc);
+    RECORD_LAYER_reset(&sc->rlayer);
 }
 
 void SSL_set_connect_state(SSL *s)
@@ -4802,7 +4755,7 @@ void SSL_set_connect_state(SSL *s)
     ossl_statem_clear(sc);
     sc->handshake_func = s->method->ssl_connect;
     /* Ignore return value. Its a void public API function */
-    clear_record_layer(sc);
+    RECORD_LAYER_reset(&sc->rlayer);
 }
 
 int ssl_undefined_function(SSL *s)