Skip to content

Commit

Permalink
Clear away some unused fields and cruft in the record layer
Browse files Browse the repository at this point in the history
Now that the read record layer has moved to the new architecture we can
clear some of the old stuff away.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #18132)
  • Loading branch information
mattcaswell committed Aug 18, 2022
1 parent 8bbf7ef commit b0a9042
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 73 deletions.
10 changes: 2 additions & 8 deletions ssl/record/methods/recmethod_local.h
Expand Up @@ -85,12 +85,8 @@ struct ossl_record_layer_st
*/
BIO *next;

/* Types match the equivalent structures in the SSL object */
/* Types match the equivalent fields in the SSL object */
uint64_t options;
/*
* TODO(RECLAYER): Should we take the opportunity to make this uint64_t
* even though upper layer continue to use uint32_t?
*/
uint32_t mode;

/* read IO goes into here */
Expand Down Expand Up @@ -120,9 +116,7 @@ struct ossl_record_layer_st
int alert;

/*
* Read as many input bytes as possible (for
* non-blocking reads)
* TODO(RECLAYER): Why isn't this just an option?
* Read as many input bytes as possible (for non-blocking reads)
*/
int read_ahead;

Expand Down
10 changes: 5 additions & 5 deletions ssl/record/methods/tls_common.c
Expand Up @@ -158,11 +158,11 @@ int tls_default_read_n(OSSL_RECORD_LAYER *rl, size_t n, size_t max, int extend,
/*
* If extend == 0, obtain new n-byte packet; if extend == 1, increase
* packet by another n bytes. The packet will be in the sub-array of
* s->rlayer.rbuf.buf specified by s->rlayer.packet and
* s->rlayer.packet_length. (If s->rlayer.read_ahead is set, 'max' bytes may
* be stored in rbuf [plus s->rlayer.packet_length bytes if extend == 1].)
* if clearold == 1, move the packet to the start of the buffer; if
* clearold == 0 then leave any old packets where they were
* rl->rbuf.buf specified by rl->packet and rl->packet_length. (If
* rl->read_ahead is set, 'max' bytes may be stored in rbuf [plus
* rl->packet_length bytes if extend == 1].) if clearold == 1, move the
* packet to the start of the buffer; if clearold == 0 then leave any old
* packets where they were
*/
size_t len, left, align = 0;
unsigned char *pkt;
Expand Down
3 changes: 0 additions & 3 deletions ssl/record/rec_layer_d1.c
Expand Up @@ -858,9 +858,6 @@ void dtls1_reset_seq_numbers(SSL_CONNECTION *s, int rw)
if (rw & SSL3_CC_READ) {
seq = s->rlayer.read_sequence;
s->rlayer.d->r_epoch++;
memcpy(&s->rlayer.d->bitmap, &s->rlayer.d->next_bitmap,
sizeof(s->rlayer.d->bitmap));
memset(&s->rlayer.d->next_bitmap, 0, sizeof(s->rlayer.d->next_bitmap));

/*
* We must not use any buffered messages received from the previous
Expand Down
26 changes: 3 additions & 23 deletions ssl/record/rec_layer_s3.c
Expand Up @@ -30,22 +30,12 @@
void RECORD_LAYER_init(RECORD_LAYER *rl, SSL_CONNECTION *s)
{
rl->s = s;
RECORD_LAYER_set_first_record(&s->rlayer);
SSL3_RECORD_clear(rl->rrec, SSL_MAX_PIPELINES);
}

void RECORD_LAYER_clear(RECORD_LAYER *rl)
{
rl->rstate = SSL_ST_READ_HEADER;

/*
* Do I need to clear read_ahead? As far as I can tell read_ahead did not
* previously get reset by SSL_clear...so I'll keep it that way..but is
* that right?
*/

rl->packet = NULL;
rl->packet_length = 0;
rl->wnum = 0;
memset(rl->handshake_fragment, 0, sizeof(rl->handshake_fragment));
rl->handshake_fragment_len = 0;
Expand All @@ -54,10 +44,7 @@ void RECORD_LAYER_clear(RECORD_LAYER *rl)
rl->wpend_ret = 0;
rl->wpend_buf = NULL;

SSL3_BUFFER_clear(&rl->rbuf);
ssl3_release_write_buffer(rl->s);
rl->numrpipes = 0;
SSL3_RECORD_clear(rl->rrec, SSL_MAX_PIPELINES);

RECORD_LAYER_reset_read_sequence(rl);
RECORD_LAYER_reset_write_sequence(rl);
Expand All @@ -70,7 +57,6 @@ void RECORD_LAYER_release(RECORD_LAYER *rl)
{
if (rl->numwpipes > 0)
ssl3_release_write_buffer(rl->s);
SSL3_RECORD_release(rl->rrec, SSL_MAX_PIPELINES);
}

/* Checks if we have unprocessed read ahead data pending */
Expand Down Expand Up @@ -153,6 +139,7 @@ const char *SSL_rstate_string_long(const SSL *s)
if (sc == NULL)
return NULL;

/* TODO(RECLAYER): Fix me */
switch (sc->rlayer.rstate) {
case SSL_ST_READ_HEADER:
return "read header";
Expand All @@ -172,6 +159,7 @@ const char *SSL_rstate_string(const SSL *s)
if (sc == NULL)
return NULL;

/* TODO(RECLAYER): Fix me */
switch (sc->rlayer.rstate) {
case SSL_ST_READ_HEADER:
return "RH";
Expand Down Expand Up @@ -1263,7 +1251,7 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
rr = &s->rlayer.tlsrecs[s->rlayer.curr_rec];

if (s->rlayer.handshake_fragment_len > 0
&& SSL3_RECORD_get_type(rr) != SSL3_RT_HANDSHAKE
&& rr->type != SSL3_RT_HANDSHAKE
&& SSL_CONNECTION_IS_TLS13(s)) {
SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE,
SSL_R_MIXED_HANDSHAKE_AND_NON_HANDSHAKE_DATA);
Expand Down Expand Up @@ -1697,14 +1685,6 @@ int RECORD_LAYER_is_sslv2_record(RECORD_LAYER *rl)
return rl->tlsrecs[0].version == SSL2_VERSION;
}

/*
* Returns the length in bytes of the current rrec
*/
size_t RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl)
{
return SSL3_RECORD_get_length(&rl->rrec[0]);
}

static OSSL_FUNC_rlayer_msg_callback_fn rlayer_msg_callback_wrapper;
static void rlayer_msg_callback_wrapper(int write_p, int version,
int content_type, const void *buf,
Expand Down
22 changes: 2 additions & 20 deletions ssl/record/record.h
Expand Up @@ -125,10 +125,7 @@ typedef struct dtls_record_layer_st {
*/
unsigned short r_epoch;
unsigned short w_epoch;
/* records being received in the current epoch */
DTLS1_BITMAP bitmap;
/* renegotiation starts a new set of sequence numbers */
DTLS1_BITMAP next_bitmap;

/*
* Buffered application records. Only for records between CCS and
* Finished to prevent either protocol violation or unnecessary message
Expand Down Expand Up @@ -158,42 +155,28 @@ typedef struct record_layer_st {
int read_ahead;
/* where we are when reading */
int rstate;
/* How many pipelines can be used to read data */
size_t numrpipes;
/* How many pipelines can be used to write data */
size_t numwpipes;
/* read IO goes into here */
SSL3_BUFFER rbuf;
/* write IO goes into here */
SSL3_BUFFER wbuf[SSL_MAX_PIPELINES];
/* each decoded record goes in here */
SSL3_RECORD rrec[SSL_MAX_PIPELINES];
/* used internally to point at a raw packet */
unsigned char *packet;
size_t packet_length;
/* number of bytes sent so far */
size_t wnum;
unsigned char handshake_fragment[4];
size_t handshake_fragment_len;
/* The number of consecutive empty records we have received */
size_t empty_record_count;
/* partial write - check the numbers match */
/* number bytes written */
size_t wpend_tot;
int wpend_type;
/* number of bytes submitted */
size_t wpend_ret;
const unsigned char *wpend_buf;
/* TODO(RECLAYER): Why do we need this */
unsigned char read_sequence[SEQ_NUM_SIZE];
unsigned char write_sequence[SEQ_NUM_SIZE];
/* Set to true if this is the first record in a connection */
unsigned int is_first_record;
/* Count of the number of consecutive warning alerts received */
unsigned int alert_count;
DTLS_RECORD_LAYER *d;

/* TODO(RECLAYER): Tidy me up. New fields for record management */

/* How many records we have read from the record layer */
size_t num_recs;
/* The next record from the record layer that we need to process */
Expand Down Expand Up @@ -235,7 +218,6 @@ int RECORD_LAYER_write_pending(const RECORD_LAYER *rl);
void RECORD_LAYER_reset_read_sequence(RECORD_LAYER *rl);
void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl);
int RECORD_LAYER_is_sslv2_record(RECORD_LAYER *rl);
size_t RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl);
__owur size_t ssl3_pending(const SSL *s);
__owur int ssl3_write_bytes(SSL *s, int type, const void *buf, size_t len,
size_t *written);
Expand Down
14 changes: 0 additions & 14 deletions ssl/record/record_local.h
Expand Up @@ -18,22 +18,12 @@

/* Functions/macros provided by the RECORD_LAYER component */

#define RECORD_LAYER_get_rrec(rl) ((rl)->rrec)
#define RECORD_LAYER_set_packet(rl, p) ((rl)->packet = (p))
#define RECORD_LAYER_reset_packet_length(rl) ((rl)->packet_length = 0)
#define RECORD_LAYER_get_rstate(rl) ((rl)->rstate)
#define RECORD_LAYER_set_rstate(rl, st) ((rl)->rstate = (st))
#define RECORD_LAYER_get_read_sequence(rl) ((rl)->read_sequence)
#define RECORD_LAYER_get_write_sequence(rl) ((rl)->write_sequence)
#define RECORD_LAYER_get_numrpipes(rl) ((rl)->numrpipes)
#define RECORD_LAYER_set_numrpipes(rl, n) ((rl)->numrpipes = (n))
#define RECORD_LAYER_inc_empty_record_count(rl) ((rl)->empty_record_count++)
#define RECORD_LAYER_reset_empty_record_count(rl) \
((rl)->empty_record_count = 0)
#define RECORD_LAYER_get_empty_record_count(rl) ((rl)->empty_record_count)
#define RECORD_LAYER_is_first_record(rl) ((rl)->is_first_record)
#define RECORD_LAYER_set_first_record(rl) ((rl)->is_first_record = 1)
#define RECORD_LAYER_clear_first_record(rl) ((rl)->is_first_record = 0)
#define DTLS_RECORD_LAYER_get_r_epoch(rl) ((rl)->d->r_epoch)

int dtls_buffer_record(SSL_CONNECTION *s, TLS_RECORD *rec);
Expand Down Expand Up @@ -83,10 +73,6 @@ int ssl3_release_write_buffer(SSL_CONNECTION *s);
#define SSL3_RECORD_set_off(r, o) ((r)->off = (o))
#define SSL3_RECORD_add_off(r, o) ((r)->off += (o))
#define SSL3_RECORD_get_epoch(r) ((r)->epoch)
#define SSL3_RECORD_is_sslv2_record(r) \
((r)->rec_version == SSL2_VERSION)
#define SSL3_RECORD_is_read(r) ((r)->read)
#define SSL3_RECORD_set_read(r) ((r)->read = 1)

void SSL3_RECORD_clear(SSL3_RECORD *r, size_t);
void SSL3_RECORD_release(SSL3_RECORD *r, size_t num_recs);
Expand Down

0 comments on commit b0a9042

Please sign in to comment.