]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Clear away some unused fields and cruft in the record layer
authorMatt Caswell <matt@openssl.org>
Thu, 21 Jul 2022 16:01:54 +0000 (17:01 +0100)
committerMatt Caswell <matt@openssl.org>
Thu, 18 Aug 2022 15:38:13 +0000 (16:38 +0100)
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 https://github.com/openssl/openssl/pull/18132)

ssl/record/methods/recmethod_local.h
ssl/record/methods/tls_common.c
ssl/record/rec_layer_d1.c
ssl/record/rec_layer_s3.c
ssl/record/record.h
ssl/record/record_local.h

index 7e119e617418a626526bf37307c5ea7d2c06323a..5f3bc6bf6a846ccebe9b27c360d4e86b2b6ed3c9 100644 (file)
@@ -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 */
@@ -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;
 
index d414c016f1af407abed8ba9a3651df86a47947b0..5d618ca00893ca5c591f4d722d9744e4c5b0b55f 100644 (file)
@@ -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;
index a621d089840af1248f0441ac91555c7f1f6884f9..f032e3ea49c385870862cfd94e019eb3175ece0b 100644 (file)
@@ -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
index 2407efa0e0acbbbfddb185906cb6afd24aa613a9..3a1c6ef7a2002a04a133510c94c0cce52c5da117 100644 (file)
 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;
@@ -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);
@@ -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 */
@@ -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";
@@ -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";
@@ -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);
@@ -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,
index 2848df3283965c9db0bf1496d9329f5fcf4a3655..c441f7baec131b1807d078d4a2edf54fa4302d65 100644 (file)
@@ -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
@@ -158,25 +155,14 @@ 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;
@@ -184,16 +170,13 @@ typedef struct record_layer_st {
     /* 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 */
@@ -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);
index a2e0a41875aede90ec89051f235b458bad5c4133..527bc4f64e7ff862fc4789bf123cbb815884fbb9 100644 (file)
 
 /* 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);
@@ -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);