diff options
author | Matt Caswell <matt@openssl.org> | 2024-01-18 12:08:52 +0000 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2024-01-31 10:10:55 +0000 |
commit | 4a0e4849af1588dfe9d7e01738acc96799b83447 (patch) | |
tree | 61b2a7786c577fb5d979e3d8b892dd59305aa485 /ssl | |
parent | a86714041d8a5868c629e9027e28c6d1dacde5f9 (diff) |
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)
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/record/rec_layer_s3.c | 46 | ||||
-rw-r--r-- | ssl/record/record.h | 3 | ||||
-rw-r--r-- | ssl/ssl_lib.c | 55 |
3 files changed, 51 insertions, 53 deletions
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 4d057ec184..12a4ff8e98 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -25,14 +25,29 @@ 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; rl->wpend_tot = 0; rl->wpend_type = 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 */ @@ -47,6 +62,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 ae5afc7b4a..6c8545d706 100644 --- a/ssl/record/record.h +++ b/ssl/record/record.h @@ -139,7 +139,8 @@ typedef struct record_layer_st { #define RECORD_LAYER_get_read_ahead(rl) ((rl)->read_ahead) 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 8a834c3527..818d5d11a1 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); @@ -4795,7 +4748,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) @@ -4814,7 +4767,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) |