From e158ada6a74e5903354fdd5a6f56a32bbbba69fd Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 17 Oct 2022 15:46:02 +0100 Subject: Remove the old buffer management code We no longer use the old buffer management code now that it has all been moved to the new record layer. Reviewed-by: Richard Levitte Reviewed-by: Tomas Mraz Reviewed-by: Hugo Landau (Merged from https://github.com/openssl/openssl/pull/19424) --- ssl/d1_lib.c | 9 ++-- ssl/record/build.info | 2 +- ssl/record/methods/recmethod_local.h | 16 ++++++ ssl/record/methods/tls_common.c | 6 +++ ssl/record/rec_layer_d1.c | 13 ----- ssl/record/rec_layer_s3.c | 12 ++--- ssl/record/record.h | 12 +---- ssl/record/record_local.h | 24 --------- ssl/record/ssl3_buffer.c | 99 ------------------------------------ ssl/ssl_lib.c | 3 +- ssl/ssl_local.h | 2 - ssl/ssl_utst.c | 1 - ssl/statem/extensions.c | 6 --- ssl/statem/statem.c | 4 -- 14 files changed, 35 insertions(+), 174 deletions(-) (limited to 'ssl') diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index b82ddec517..a4ac629926 100644 --- a/ssl/d1_lib.c +++ b/ssl/d1_lib.c @@ -454,14 +454,12 @@ int DTLSv1_listen(SSL *ssl, BIO_ADDR *client) return -1; } - if (!ssl3_setup_buffers(s)) { - /* ERR_raise() already called */ - return -1; - } buf = OPENSSL_malloc(DTLS1_RT_HEADER_LENGTH + SSL3_RT_MAX_PLAIN_LENGTH); if (buf == NULL) return -1; - wbuf = RECORD_LAYER_get_wbuf(&s->rlayer)[0].buf; + wbuf = OPENSSL_malloc(DTLS1_RT_HEADER_LENGTH + SSL3_RT_MAX_PLAIN_LENGTH); + if (buf == NULL) + return -1; do { /* Get a packet */ @@ -836,6 +834,7 @@ int DTLSv1_listen(SSL *ssl, BIO_ADDR *client) end: BIO_ADDR_free(tmpclient); OPENSSL_free(buf); + OPENSSL_free(wbuf); return ret; } #endif diff --git a/ssl/record/build.info b/ssl/record/build.info index 15a4f8b386..8dfac91dae 100644 --- a/ssl/record/build.info +++ b/ssl/record/build.info @@ -11,7 +11,7 @@ IF[{- !$disabled{asm} -}] ENDIF SOURCE[../../libssl]=\ - rec_layer_s3.c rec_layer_d1.c ssl3_buffer.c ssl3_record.c \ + rec_layer_s3.c rec_layer_d1.c ssl3_record.c \ ssl3_record_tls13.c # For shared builds we need to include the sources needed in providers diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h index 2552a8c0ac..21a8297b08 100644 --- a/ssl/record/methods/recmethod_local.h +++ b/ssl/record/methods/recmethod_local.h @@ -460,3 +460,19 @@ int tls_post_encryption_processing_default(OSSL_RECORD_LAYER *rl, int tls_write_records_default(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates, size_t numtempl); + +/* Macros/functions provided by the SSL3_BUFFER component */ + +#define SSL3_BUFFER_get_buf(b) ((b)->buf) +#define SSL3_BUFFER_set_buf(b, n) ((b)->buf = (n)) +#define SSL3_BUFFER_get_len(b) ((b)->len) +#define SSL3_BUFFER_get_left(b) ((b)->left) +#define SSL3_BUFFER_set_left(b, l) ((b)->left = (l)) +#define SSL3_BUFFER_sub_left(b, l) ((b)->left -= (l)) +#define SSL3_BUFFER_get_offset(b) ((b)->offset) +#define SSL3_BUFFER_set_offset(b, o) ((b)->offset = (o)) +#define SSL3_BUFFER_add_offset(b, o) ((b)->offset += (o)) +#define SSL3_BUFFER_set_app_buffer(b, l) ((b)->app_buffer = (l)) +#define SSL3_BUFFER_is_app_buffer(b) ((b)->app_buffer) + +void SSL3_BUFFER_release(SSL3_BUFFER *b); diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index 5c27a6af15..7f1658899d 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -22,6 +22,12 @@ static void tls_int_free(OSSL_RECORD_LAYER *rl); +void SSL3_BUFFER_release(SSL3_BUFFER *b) +{ + OPENSSL_free(b->buf); + b->buf = NULL; +} + void ossl_rlayer_fatal(OSSL_RECORD_LAYER *rl, int al, int reason, const char *fmt, ...) { diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c index e8b3242e28..2d74280188 100644 --- a/ssl/record/rec_layer_d1.c +++ b/ssl/record/rec_layer_d1.c @@ -639,21 +639,8 @@ int do_dtls1_write(SSL_CONNECTION *sc, int type, const unsigned char *buf, int i; OSSL_RECORD_TEMPLATE tmpl; SSL *s = SSL_CONNECTION_GET_SSL(sc); - SSL3_BUFFER *wb; int ret; - wb = &sc->rlayer.wbuf[0]; - - /* - * DTLS writes whole datagrams, so there can't be anything left in - * the buffer. - */ - /* TODO(RECLAYER): Remove me */ - if (!ossl_assert(SSL3_BUFFER_get_left(wb) == 0)) { - SSLfatal(sc, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; - } - /* If we have an alert to send, lets send it */ if (sc->s3.alert_dispatch) { i = s->method->ssl_dispatch_alert(s); diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index bc115684b7..301742dba5 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -33,8 +33,6 @@ void RECORD_LAYER_clear(RECORD_LAYER *rl) rl->wpend_ret = 0; rl->wpend_buf = NULL; - ssl3_release_write_buffer(rl->s); - RECORD_LAYER_reset_write_sequence(rl); if (rl->rrlmethod != NULL) @@ -54,8 +52,10 @@ void RECORD_LAYER_clear(RECORD_LAYER *rl) void RECORD_LAYER_release(RECORD_LAYER *rl) { - if (rl->numwpipes > 0) - ssl3_release_write_buffer(rl->s); + /* + * TODO(RECLAYER): Need a way to release the write buffers in the record + * layer on demand + */ } /* Checks if we have unprocessed read ahead data pending */ @@ -73,10 +73,6 @@ int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl) int RECORD_LAYER_write_pending(const RECORD_LAYER *rl) { - /* TODO(RECLAYER): Remove me when DTLS is moved to the write record layer */ - if (SSL_CONNECTION_IS_DTLS(rl->s)) - return (rl->numwpipes > 0) - && SSL3_BUFFER_get_left(&rl->wbuf[rl->numwpipes - 1]) != 0; return rl->wpend_tot > 0; } diff --git a/ssl/record/record.h b/ssl/record/record.h index 501963756b..1cb24b835b 100644 --- a/ssl/record/record.h +++ b/ssl/record/record.h @@ -21,7 +21,7 @@ typedef struct ssl3_buffer_st SSL3_BUFFER; *****************************************************************************/ struct ssl3_buffer_st { - /* at least SSL3_RT_MAX_PACKET_SIZE bytes, see ssl3_setup_buffers() */ + /* at least SSL3_RT_MAX_PACKET_SIZE bytes */ unsigned char *buf; /* default buffer size (or 0 if no default set) */ size_t default_len; @@ -149,14 +149,7 @@ typedef struct record_layer_st { * non-blocking reads) */ int read_ahead; - /* - * TODO(RECLAYER): These next 2 fields can be removed when DTLS is moved to - * the new write record layer architecture. - */ - /* How many pipelines can be used to write data */ - size_t numwpipes; - /* write IO goes into here */ - SSL3_BUFFER wbuf[SSL_MAX_PIPELINES + 1]; + /* number of bytes sent so far */ size_t wnum; unsigned char handshake_fragment[4]; @@ -225,7 +218,6 @@ __owur int ssl3_write_bytes(SSL *s, int type, const void *buf, size_t len, __owur int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, size_t len, int peek, size_t *readbytes); -__owur int ssl3_setup_buffers(SSL_CONNECTION *s); __owur int tls1_enc(SSL_CONNECTION *s, SSL3_RECORD *recs, size_t n_recs, int sending, SSL_MAC_BUF *mac, size_t macsize); __owur int tls1_mac_old(SSL_CONNECTION *s, SSL3_RECORD *rec, unsigned char *md, diff --git a/ssl/record/record_local.h b/ssl/record/record_local.h index 04bf2ef6a0..b7535ef004 100644 --- a/ssl/record/record_local.h +++ b/ssl/record/record_local.h @@ -27,30 +27,6 @@ int dtls_buffer_record(SSL_CONNECTION *s, TLS_RECORD *rec); -/* Macros/functions provided by the SSL3_BUFFER component */ - -#define SSL3_BUFFER_get_buf(b) ((b)->buf) -#define SSL3_BUFFER_set_buf(b, n) ((b)->buf = (n)) -#define SSL3_BUFFER_get_len(b) ((b)->len) -#define SSL3_BUFFER_set_len(b, l) ((b)->len = (l)) -#define SSL3_BUFFER_get_left(b) ((b)->left) -#define SSL3_BUFFER_set_left(b, l) ((b)->left = (l)) -#define SSL3_BUFFER_sub_left(b, l) ((b)->left -= (l)) -#define SSL3_BUFFER_get_offset(b) ((b)->offset) -#define SSL3_BUFFER_set_offset(b, o) ((b)->offset = (o)) -#define SSL3_BUFFER_add_offset(b, o) ((b)->offset += (o)) -#define SSL3_BUFFER_is_initialised(b) ((b)->buf != NULL) -#define SSL3_BUFFER_set_default_len(b, l) ((b)->default_len = (l)) -#define SSL3_BUFFER_set_app_buffer(b, l) ((b)->app_buffer = (l)) -#define SSL3_BUFFER_is_app_buffer(b) ((b)->app_buffer) - -void SSL3_BUFFER_clear(SSL3_BUFFER *b); -void SSL3_BUFFER_set_data(SSL3_BUFFER *b, const unsigned char *d, size_t n); -void SSL3_BUFFER_release(SSL3_BUFFER *b); -__owur int ssl3_setup_write_buffer(SSL_CONNECTION *s, size_t numwpipes, - size_t len); -int ssl3_release_write_buffer(SSL_CONNECTION *s); - /* Macros/functions provided by the SSL3_RECORD component */ #define SSL3_RECORD_get_type(r) ((r)->type) diff --git a/ssl/record/ssl3_buffer.c b/ssl/record/ssl3_buffer.c index 1bb4840f88..f165e08705 100644 --- a/ssl/record/ssl3_buffer.c +++ b/ssl/record/ssl3_buffer.c @@ -33,102 +33,3 @@ void SSL3_BUFFER_release(SSL3_BUFFER *b) OPENSSL_free(b->buf); b->buf = NULL; } - -int ssl3_setup_write_buffer(SSL_CONNECTION *s, size_t numwpipes, size_t len) -{ - unsigned char *p; - size_t align = 0, headerlen; - SSL3_BUFFER *wb; - size_t currpipe; - - /* - * TODO(RECLAYER): Eventually this function can be removed once everything - * is moved to the write record layer. - */ - if (!SSL_CONNECTION_IS_DTLS(s)) - return 1; - - s->rlayer.numwpipes = numwpipes; - - if (len == 0) { - if (SSL_CONNECTION_IS_DTLS(s)) - headerlen = DTLS1_RT_HEADER_LENGTH + 1; - else - headerlen = SSL3_RT_HEADER_LENGTH; - -#if defined(SSL3_ALIGN_PAYLOAD) && SSL3_ALIGN_PAYLOAD!=0 - align = SSL3_ALIGN_PAYLOAD - 1; -#endif - - len = ssl_get_max_send_fragment(s) - + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD + headerlen + align; -#ifndef OPENSSL_NO_COMP - if (ssl_allow_compression(s)) - len += SSL3_RT_MAX_COMPRESSED_OVERHEAD; -#endif - if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS)) - len += headerlen + align + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD; - } - - wb = RECORD_LAYER_get_wbuf(&s->rlayer); - for (currpipe = 0; currpipe < numwpipes; currpipe++) { - SSL3_BUFFER *thiswb = &wb[currpipe]; - - if (thiswb->len != len) { - OPENSSL_free(thiswb->buf); - thiswb->buf = NULL; /* force reallocation */ - } - - if (thiswb->buf == NULL) { - if (s->wbio == NULL || !BIO_get_ktls_send(s->wbio)) { - p = OPENSSL_malloc(len); - if (p == NULL) { - s->rlayer.numwpipes = currpipe; - /* - * We've got a malloc failure, and we're still initialising - * buffers. We assume we're so doomed that we won't even be able - * to send an alert. - */ - SSLfatal(s, SSL_AD_NO_ALERT, ERR_R_CRYPTO_LIB); - return 0; - } - } else { - p = NULL; - } - memset(thiswb, 0, sizeof(SSL3_BUFFER)); - thiswb->buf = p; - thiswb->len = len; - } - } - - return 1; -} - -int ssl3_setup_buffers(SSL_CONNECTION *s) -{ - if (!ssl3_setup_write_buffer(s, 1, 0)) { - /* SSLfatal() already called */ - return 0; - } - return 1; -} - -int ssl3_release_write_buffer(SSL_CONNECTION *s) -{ - SSL3_BUFFER *wb; - size_t pipes; - - pipes = s->rlayer.numwpipes; - while (pipes > 0) { - wb = &RECORD_LAYER_get_wbuf(&s->rlayer)[pipes - 1]; - - if (SSL3_BUFFER_is_app_buffer(wb)) - SSL3_BUFFER_set_app_buffer(wb, 0); - else - OPENSSL_free(wb->buf); - wb->buf = NULL; - pipes--; - } - s->rlayer.numwpipes = 0; - return 1; -} diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 77b0fcefc3..d47526b6cf 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -6388,7 +6388,8 @@ int SSL_alloc_buffers(SSL *ssl) if (sc == NULL) return 0; - return ssl3_setup_buffers(sc); + /* TODO(RECLAYER): Need a way to make this happen in the record layer */ + return 1; } void SSL_CTX_set_keylog_callback(SSL_CTX *ctx, SSL_CTX_keylog_cb_func cb) diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index a1f15a712f..6a935760a0 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -2447,7 +2447,6 @@ const SSL_METHOD *func_name(void) \ struct openssl_ssl_test_functions { int (*p_ssl_init_wbio_buffer) (SSL_CONNECTION *s); - int (*p_ssl3_setup_buffers) (SSL_CONNECTION *s); }; const char *ssl_protocol_to_string(int version); @@ -2959,7 +2958,6 @@ void ssl_session_calculate_timeout(SSL_SESSION *ss); # else /* OPENSSL_UNIT_TEST */ # define ssl_init_wbio_buffer SSL_test_functions()->p_ssl_init_wbio_buffer -# define ssl3_setup_buffers SSL_test_functions()->p_ssl3_setup_buffers # endif diff --git a/ssl/ssl_utst.c b/ssl/ssl_utst.c index 690db6d497..91be7398ca 100644 --- a/ssl/ssl_utst.c +++ b/ssl/ssl_utst.c @@ -13,7 +13,6 @@ static const struct openssl_ssl_test_functions ssl_test_functions = { ssl_init_wbio_buffer, - ssl3_setup_buffers, }; const struct openssl_ssl_test_functions *SSL_test_functions(void) diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index b879050c40..601682152a 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1732,12 +1732,6 @@ static int final_maxfragmentlen(SSL_CONNECTION *s, unsigned int context, GET_MAX_FRAGMENT_LENGTH(s->session)); s->rlayer.wrlmethod->set_max_frag_len(s->rlayer.wrl, ssl_get_max_send_fragment(s)); - /* trigger a larger buffer reallocation */ - /* TODO(RECLAYER): Remove me when DTLS moved to write record layer */ - if (SSL_CONNECTION_IS_DTLS(s) && !ssl3_setup_buffers(s)) { - /* SSLfatal() already called */ - return 0; - } } return 1; diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c index 448d655a17..921d7cfb1e 100644 --- a/ssl/statem/statem.c +++ b/ssl/statem/statem.c @@ -439,10 +439,6 @@ static int state_machine(SSL_CONNECTION *s, int server) buf = NULL; } - if (!ssl3_setup_buffers(s)) { - SSLfatal(s, SSL_AD_NO_ALERT, ERR_R_INTERNAL_ERROR); - goto end; - } s->init_num = 0; /* -- cgit v1.2.3