From 0755722c28309a52f29573221e411a2b37175e37 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 24 May 2022 16:00:50 +0100 Subject: [PATCH] Move the sequence number into the OSSL_RECORD_LAYER object This removes some references to the SSL object from the record layer. Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/18132) --- ssl/record/methods/ktls_meth.c | 8 +------- ssl/record/methods/recmethod_local.h | 3 +++ ssl/record/methods/ssl3_meth.c | 4 +--- ssl/record/methods/tls13_meth.c | 5 +---- ssl/record/methods/tls1_meth.c | 20 +++++--------------- ssl/record/methods/tls_common.c | 5 +++-- test/sslapitest.c | 9 +++++---- 7 files changed, 19 insertions(+), 35 deletions(-) diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c index e608b530ff471..a396c111e59fa 100644 --- a/ssl/record/methods/ktls_meth.c +++ b/ssl/record/methods/ktls_meth.c @@ -449,7 +449,6 @@ static int ktls_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s) { - void *rl_sequence; ktls_crypto_info_t crypto_info; /* @@ -488,12 +487,7 @@ static int ktls_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, return OSSL_RECORD_RETURN_NON_FATAL_ERR; } - if (rl->direction == OSSL_RECORD_DIRECTION_WRITE) - rl_sequence = RECORD_LAYER_get_write_sequence(&s->rlayer); - else - rl_sequence = RECORD_LAYER_get_read_sequence(&s->rlayer); - - if (!ktls_configure_crypto(s, ciph, rl_sequence, &crypto_info, + if (!ktls_configure_crypto(s, ciph, rl->sequence, &crypto_info, rl->direction == OSSL_RECORD_DIRECTION_WRITE, iv, ivlen, key, keylen, mackey, mackeylen)) return OSSL_RECORD_RETURN_NON_FATAL_ERR; diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h index 6a7d63fd0f97a..74d8f0132865e 100644 --- a/ssl/record/methods/recmethod_local.h +++ b/ssl/record/methods/recmethod_local.h @@ -113,6 +113,9 @@ struct ossl_record_layer_st unsigned char *packet; size_t packet_length; + /* Sequence number for the next record */ + unsigned char sequence[SEQ_NUM_SIZE]; + int alert; /* diff --git a/ssl/record/methods/ssl3_meth.c b/ssl/record/methods/ssl3_meth.c index a2761de96ab30..7e8219a094a6e 100644 --- a/ssl/record/methods/ssl3_meth.c +++ b/ssl/record/methods/ssl3_meth.c @@ -215,7 +215,7 @@ static const unsigned char ssl3_pad_2[48] = { static int ssl3_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md, int sending, SSL_CONNECTION *ssl) { - unsigned char *mac_sec, *seq; + unsigned char *mac_sec, *seq = rl->sequence; const EVP_MD_CTX *hash; unsigned char *p, rec_char; size_t md_size; @@ -224,11 +224,9 @@ static int ssl3_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md, if (sending) { mac_sec = &(ssl->s3.write_mac_secret[0]); - seq = RECORD_LAYER_get_write_sequence(&ssl->rlayer); hash = ssl->write_hash; } else { mac_sec = &(rl->mac_secret[0]); - seq = RECORD_LAYER_get_read_sequence(&ssl->rlayer); hash = rl->read_hash; } diff --git a/ssl/record/methods/tls13_meth.c b/ssl/record/methods/tls13_meth.c index 3ea0f5fec34ce..d5a8af4f8d42c 100644 --- a/ssl/record/methods/tls13_meth.c +++ b/ssl/record/methods/tls13_meth.c @@ -41,7 +41,6 @@ static int tls13_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, return OSSL_RECORD_RETURN_FATAL; } - RECORD_LAYER_reset_read_sequence(&s->rlayer); rl->taglen = taglen; mode = EVP_CIPHER_get_mode(ciph); @@ -66,7 +65,7 @@ static int tls13_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs, unsigned char iv[EVP_MAX_IV_LENGTH], recheader[SSL3_RT_HEADER_LENGTH]; size_t ivlen, offset, loop, hdrlen; unsigned char *staticiv; - unsigned char *seq; + unsigned char *seq = rl->sequence; int lenu, lenf; SSL3_RECORD *rec = &recs[0]; WPACKET wpkt; @@ -82,11 +81,9 @@ static int tls13_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs, if (sending) { ctx = s->enc_write_ctx; staticiv = s->write_iv; - seq = RECORD_LAYER_get_write_sequence(&s->rlayer); } else { ctx = rl->enc_read_ctx; staticiv = rl->iv; - seq = RECORD_LAYER_get_read_sequence(&s->rlayer); } cipher = EVP_CIPHER_CTX_get0_cipher(ctx); diff --git a/ssl/record/methods/tls1_meth.c b/ssl/record/methods/tls1_meth.c index d744c6f260fa2..8d189bcb89d3b 100644 --- a/ssl/record/methods/tls1_meth.c +++ b/ssl/record/methods/tls1_meth.c @@ -55,11 +55,6 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, } } #endif - /* - * this is done by dtls1_reset_seq_numbers for DTLS - */ - if (!rl->isdtls) - RECORD_LAYER_reset_read_sequence(&s->rlayer); /* * If we have an AEAD Cipher, then there is no separate MAC, so we can skip @@ -248,8 +243,7 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs, & EVP_CIPH_FLAG_AEAD_CIPHER) != 0) { unsigned char *seq; - seq = sending ? RECORD_LAYER_get_write_sequence(&s->rlayer) - : RECORD_LAYER_get_read_sequence(&s->rlayer); + seq = rl->sequence; if (SSL_CONNECTION_IS_DTLS(s)) { /* DTLS does not support pipelining */ @@ -350,8 +344,7 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs, if (sending && !rl->use_etm) decrement_seq = 1; - seq = sending ? RECORD_LAYER_get_write_sequence(&s->rlayer) - : RECORD_LAYER_get_read_sequence(&s->rlayer); + seq = rl->sequence; if (EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_TLSTREE, decrement_seq, seq) <= 0) { RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; @@ -468,7 +461,7 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs, static int tls1_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md, int sending, SSL_CONNECTION *ssl) { - unsigned char *seq; + unsigned char *seq = rl->sequence; EVP_MD_CTX *hash; size_t md_size; int i; @@ -481,13 +474,10 @@ static int tls1_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md, int t; int ret = 0; - if (sending) { - seq = RECORD_LAYER_get_write_sequence(&ssl->rlayer); + if (sending) hash = ssl->write_hash; - } else { - seq = RECORD_LAYER_get_read_sequence(&ssl->rlayer); + else hash = rl->read_hash; - } t = EVP_MD_CTX_get_size(hash); if (!ossl_assert(t >= 0)) diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index 14f40dbb9c13a..27b1089092943 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -724,7 +724,8 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl, rl->num_recs = 0; rl->curr_rec = 0; rl->num_released = 0; - RECORD_LAYER_reset_read_sequence(&s->rlayer); + /* Reset the read sequence */ + memset(rl->sequence, 0, sizeof(rl->sequence)); ret = 1; goto end; } @@ -1260,7 +1261,7 @@ int tls_reset(OSSL_RECORD_LAYER *rl) int tls_unprocessed_read_pending(OSSL_RECORD_LAYER *rl) { - return SSL3_BUFFER_get_left(&rl->rbuf) != 0;; + return SSL3_BUFFER_get_left(&rl->rbuf) != 0; } int tls_processed_read_pending(OSSL_RECORD_LAYER *rl) diff --git a/test/sslapitest.c b/test/sslapitest.c index a12bb3e6707da..f470d91a7be8e 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -40,6 +40,7 @@ #include "internal/nelem.h" #include "internal/ktls.h" #include "../ssl/ssl_local.h" +#include "../ssl/record/methods/recmethod_local.h" #include "filterprov.h" #undef OSSL_NO_USABLE_TLS1_3 @@ -1087,9 +1088,9 @@ static int ping_pong_query(SSL *clientssl, SSL *serverssl) cbuf[0] = count++; memcpy(crec_wseq_before, &clientsc->rlayer.write_sequence, SEQ_NUM_SIZE); - memcpy(crec_rseq_before, &clientsc->rlayer.read_sequence, SEQ_NUM_SIZE); + memcpy(crec_rseq_before, &clientsc->rrl->sequence, SEQ_NUM_SIZE); memcpy(srec_wseq_before, &serversc->rlayer.write_sequence, SEQ_NUM_SIZE); - memcpy(srec_rseq_before, &serversc->rlayer.read_sequence, SEQ_NUM_SIZE); + memcpy(srec_rseq_before, &serversc->rrl->sequence, SEQ_NUM_SIZE); if (!TEST_true(SSL_write(clientssl, cbuf, sizeof(cbuf)) == sizeof(cbuf))) goto end; @@ -1110,9 +1111,9 @@ static int ping_pong_query(SSL *clientssl, SSL *serverssl) } memcpy(crec_wseq_after, &clientsc->rlayer.write_sequence, SEQ_NUM_SIZE); - memcpy(crec_rseq_after, &clientsc->rlayer.read_sequence, SEQ_NUM_SIZE); + memcpy(crec_rseq_after, &clientsc->rrl->sequence, SEQ_NUM_SIZE); memcpy(srec_wseq_after, &serversc->rlayer.write_sequence, SEQ_NUM_SIZE); - memcpy(srec_rseq_after, &serversc->rlayer.read_sequence, SEQ_NUM_SIZE); + memcpy(srec_rseq_after, &serversc->rrl->sequence, SEQ_NUM_SIZE); /* verify the payload */ if (!TEST_mem_eq(cbuf, sizeof(cbuf), sbuf, sizeof(sbuf)))