Remove some remaining SSL object references from DTLS record layer
authorMatt Caswell <matt@openssl.org>
Fri, 24 Jun 2022 15:45:14 +0000 (16:45 +0100)
committerMatt Caswell <matt@openssl.org>
Thu, 18 Aug 2022 15:38:13 +0000 (16:38 +0100)
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18132)

ssl/quic/quic_impl.c
ssl/record/methods/dtls_meth.c
ssl/record/methods/ktls_meth.c
ssl/record/methods/recmethod_local.h
ssl/record/methods/tls1_meth.c
ssl/record/methods/tls_common.c
ssl/record/recordmethod.h
ssl/statem/statem.c

index 31dc0b33691921a75e2128b1da5699c32653beaf..6ccc24bb22164fe79cb6c08b3e7dc7e7023ef106 100644 (file)
@@ -91,7 +91,7 @@ int ossl_quic_accept(SSL *s)
     if (sc == NULL)
         return 0;
 
-    sc->statem.in_init = 0;
+    ossl_statem_set_in_init(sc, 0);
     return 1;
 }
 
@@ -102,7 +102,7 @@ int ossl_quic_connect(SSL *s)
     if (sc == NULL)
         return 0;
 
-    sc->statem.in_init = 0;
+    ossl_statem_set_in_init(sc, 0);
     return 1;
 }
 
index e623c99d75e6fee49b0a59d011a51b9138561dcd..ca72c821f8dfeafbd9251a6bc79b1e39d2ca0162 100644 (file)
@@ -82,13 +82,11 @@ static void dtls1_record_bitmap_update(OSSL_RECORD_LAYER *rl,
 static DTLS1_BITMAP *dtls1_get_bitmap(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rr,
                                       unsigned int *is_next_epoch)
 {
-    SSL_CONNECTION *s = (SSL_CONNECTION *)rl->cbarg;
-
     *is_next_epoch = 0;
 
     /* In current epoch, accept HM, CCS, DATA, & ALERT */
     if (rr->epoch == rl->epoch)
-        return &s->rlayer.d->bitmap;
+        return &rl->bitmap;
 
     /*
      * Only HM and ALERT messages can be from the next epoch and only if we
@@ -99,12 +97,17 @@ static DTLS1_BITMAP *dtls1_get_bitmap(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rr,
              rl->unprocessed_rcds.epoch != rl->epoch &&
              (rr->type == SSL3_RT_HANDSHAKE || rr->type == SSL3_RT_ALERT)) {
         *is_next_epoch = 1;
-        return &s->rlayer.d->next_bitmap;
+        return &rl->next_bitmap;
     }
 
     return NULL;
 }
 
+static void dtls_set_in_init(OSSL_RECORD_LAYER *rl, int in_init)
+{
+    rl->in_init = in_init;
+}
+
 static int dtls1_process_record(OSSL_RECORD_LAYER *rl, DTLS1_BITMAP *bitmap)
 {
     int i;
@@ -329,7 +332,7 @@ static int dtls_rlayer_buffer_record(OSSL_RECORD_LAYER *rl, record_pqueue *queue
     return 1;
 }
 
-/* copy buffered record into SSL structure */
+/* copy buffered record into OSSL_RECORD_LAYER structure */
 static int dtls_copy_rlayer_record(OSSL_RECORD_LAYER *rl, pitem *item)
 {
     DTLS_RLAYER_RECORD_DATA *rdata;
@@ -414,9 +417,6 @@ int dtls_get_more_records(OSSL_RECORD_LAYER *rl)
     unsigned short version;
     DTLS1_BITMAP *bitmap;
     unsigned int is_next_epoch;
-    /* TODO(RECLAYER): Remove me */
-    SSL_CONNECTION *s = (SSL_CONNECTION *)rl->cbarg;
-    SSL *ssl = SSL_CONNECTION_GET_SSL(s);
 
     rl->num_recs = 0;
     rl->curr_rec = 0;
@@ -583,7 +583,7 @@ int dtls_get_more_records(OSSL_RECORD_LAYER *rl)
      * processed at this time.
      */
     if (is_next_epoch) {
-        if ((SSL_in_init(ssl) || ossl_statem_get_in_handshake(s))) {
+        if (rl->in_init) {
             if (dtls_rlayer_buffer_record(rl,
                     &(rl->unprocessed_rcds),
                     rr->seq_num) < 0) {
@@ -700,6 +700,7 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
 
     (*retrl)->isdtls = 1;
     (*retrl)->epoch = epoch;
+    (*retrl)->in_init = 1;
 
     switch (vers) {
     case DTLS_ANY_VERSION:
@@ -729,8 +730,6 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
     return ret;
 }
 
-
-
 const OSSL_RECORD_METHOD ossl_dtls_record_method = {
     dtls_new_record_layer,
     dtls_free,
@@ -751,6 +750,7 @@ const OSSL_RECORD_METHOD ossl_dtls_record_method = {
     NULL,
     tls_set_first_handshake,
     tls_set_max_pipelines,
+    dtls_set_in_init,
 
     /*
      * TODO(RECLAYER): Remove these. These function pointers are temporary hacks
index 3a99e2d9eb24198b632f49df679b3715cba90340..f5911cc7d54aaea89a8bfc2bb10461f351028f1b 100644 (file)
@@ -545,6 +545,7 @@ const OSSL_RECORD_METHOD ossl_ktls_record_method = {
     tls_set_plain_alerts,
     tls_set_first_handshake,
     tls_set_max_pipelines,
+    NULL,
 
     /*
      * TODO(RECLAYER): Remove these. These function pointers are temporary hacks
index b203c44cf633e1729a49b1b32c4c1dd892ea3898..821737161f3b5fdeb36d38e43221488105057c4e 100644 (file)
@@ -174,10 +174,20 @@ struct ossl_record_layer_st
 
     size_t taglen;
 
-    /* DTLS eceived handshake records (processed and unprocessed) */
+    /* DTLS received handshake records (processed and unprocessed) */
     record_pqueue unprocessed_rcds;
     record_pqueue processed_rcds;
 
+    /* records being received in the current epoch */
+    DTLS1_BITMAP bitmap;
+    /* renegotiation starts a new set of sequence numbers */
+    DTLS1_BITMAP next_bitmap;
+
+    /*
+     * Whether we are currently in a hanshake or not. Only maintained for DTLS
+     */
+    int in_init;
+
     /* Callbacks */
     void *cbarg;
     OSSL_FUNC_rlayer_skip_early_data_fn *skip_early_data;
@@ -288,4 +298,4 @@ void tls_set0_packet(OSSL_RECORD_LAYER *rl, unsigned char *packet,
                      size_t packetlen);
 size_t tls_get_packet_length(OSSL_RECORD_LAYER *rl);
 void tls_reset_packet_length(OSSL_RECORD_LAYER *rl);
-int rlayer_setup_read_buffer(OSSL_RECORD_LAYER *rl);
\ No newline at end of file
+int rlayer_setup_read_buffer(OSSL_RECORD_LAYER *rl);
index 5379e64abaf4c89106516be8f5377fccf12b83b5..af2a18858c8977e57b30a5e6ea73d3fc4eb14fee 100644 (file)
@@ -148,8 +148,6 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
     size_t bs, ctr, padnum, loop;
     unsigned char padval;
     const EVP_CIPHER *enc;
-    /* TODO(RECLAYER): FIXME */
-    SSL_CONNECTION *s = (SSL_CONNECTION *)rl->cbarg;
 
     if (n_recs == 0) {
         RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
@@ -232,8 +230,7 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
                 /* DTLS does not support pipelining */
                 unsigned char dtlsseq[8], *p = dtlsseq;
 
-                s2n(sending ? DTLS_RECORD_LAYER_get_w_epoch(&s->rlayer) :
-                    DTLS_RECORD_LAYER_get_r_epoch(&s->rlayer), p);
+                s2n(rl->epoch, p);
                 memcpy(p, &seq[2], 6);
                 memcpy(buf[ctr], dtlsseq, 8);
             } else {
@@ -452,8 +449,6 @@ static int tls1_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md,
     unsigned char header[13];
     int t;
     int ret = 0;
-    /* TODO(RECLAYER): FIXME */
-    SSL_CONNECTION *ssl = (SSL_CONNECTION *)rl->cbarg;
 
     hash = rl->md_ctx;
 
@@ -482,8 +477,7 @@ static int tls1_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md,
         /* TODO(RECLAYER): FIX ME */
         unsigned char dtlsseq[8], *p = dtlsseq;
 
-        s2n(sending ? DTLS_RECORD_LAYER_get_w_epoch(&ssl->rlayer) :
-            DTLS_RECORD_LAYER_get_r_epoch(&ssl->rlayer), p);
+        s2n(rl->epoch, p);
         memcpy(p, &seq[2], 6);
 
         memcpy(header, dtlsseq, 8);
index 72be209f7f896a3ee8fd5a6cfad15f228ed7af5c..95b78da705730bfcebab01797e43320db0752559 100644 (file)
@@ -1389,6 +1389,7 @@ const OSSL_RECORD_METHOD ossl_tls_record_method = {
     tls_set_plain_alerts,
     tls_set_first_handshake,
     tls_set_max_pipelines,
+    NULL,
 
     /*
      * TODO(RECLAYER): Remove these. These function pointers are temporary hacks
index b92c72ac4a0b67052127487fab0f7ce883b9f7c2..4b255dbc7c6bc96cef08fc52d893490985daa55e 100644 (file)
@@ -299,7 +299,7 @@ struct ossl_record_method_st {
     void (*set_plain_alerts)(OSSL_RECORD_LAYER *rl, int allow);
 
     /*
-     * Called immediately after creation of the recory layer if we are in a
+     * Called immediately after creation of the record layer if we are in a
      * first handshake. Also called at the end of the first handshake
      */
     void (*set_first_handshake)(OSSL_RECORD_LAYER *rl, int first);
@@ -310,6 +310,12 @@ struct ossl_record_method_st {
      */
     void (*set_max_pipelines)(OSSL_RECORD_LAYER *rl, size_t max_pipelines);
 
+    /*
+     * Called to tell the record layer whether we are currently "in init" or
+     * not. Default at creation of the record layer is "yes".
+     */
+    void (*set_in_init)(OSSL_RECORD_LAYER *rl, int in_init);
+
     /*
      * TODO(RECLAYER): Remove these. These function pointers are temporary hacks
      * during the record layer refactoring. They need to be removed before the
index d3ac21d357ecafafb21e851bd0a5edcd81090a62..ed27d27bcec38e48d8403c7da2ffaf4ff2a18101 100644 (file)
@@ -123,7 +123,7 @@ void ossl_statem_clear(SSL_CONNECTION *s)
 {
     s->statem.state = MSG_FLOW_UNINITED;
     s->statem.hand_state = TLS_ST_BEFORE;
-    s->statem.in_init = 1;
+    ossl_statem_set_in_init(s, 1);
     s->statem.no_cert_verify = 0;
 }
 
@@ -132,7 +132,7 @@ void ossl_statem_clear(SSL_CONNECTION *s)
  */
 void ossl_statem_set_renegotiate(SSL_CONNECTION *s)
 {
-    s->statem.in_init = 1;
+    ossl_statem_set_in_init(s, 1);
     s->statem.request_state = TLS_ST_SW_HELLO_REQ;
 }
 
@@ -141,7 +141,7 @@ void ossl_statem_send_fatal(SSL_CONNECTION *s, int al)
     /* We shouldn't call SSLfatal() twice. Once is enough */
     if (s->statem.in_init && s->statem.state == MSG_FLOW_ERROR)
       return;
-    s->statem.in_init = 1;
+    ossl_statem_set_in_init(s, 1);
     s->statem.state = MSG_FLOW_ERROR;
     if (al != SSL_AD_NO_ALERT
             && s->statem.enc_write_state != ENC_WRITE_STATE_INVALID)
@@ -196,6 +196,8 @@ int ossl_statem_in_error(const SSL_CONNECTION *s)
 void ossl_statem_set_in_init(SSL_CONNECTION *s, int init)
 {
     s->statem.in_init = init;
+    if (s->rrlmethod != NULL && s->rrlmethod->set_in_init != NULL)
+        s->rrlmethod->set_in_init(s->rrl, init);
 }
 
 int ossl_statem_get_in_handshake(SSL_CONNECTION *s)
@@ -270,7 +272,7 @@ void ossl_statem_check_finish_init(SSL_CONNECTION *s, int sending)
 void ossl_statem_set_hello_verify_done(SSL_CONNECTION *s)
 {
     s->statem.state = MSG_FLOW_UNINITED;
-    s->statem.in_init = 1;
+    ossl_statem_set_in_init(s, 1);
     /*
      * This will get reset (briefly) back to TLS_ST_BEFORE when we enter
      * state_machine() because |state| is MSG_FLOW_UNINITED, but until then any