Add support for moving data from one epoch to the next

Sometimes data read by a record layer in one epoch is actually intended for
the next epoch. For example in a TLS with read_ahead, the read_ahead data
could contain a KeyUpdate message followed by application data encrypted
with new keys. Therefore we implement a mechanism for passing this data
across the epochs.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18132)

diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c
index 3a9c836077c40bc90dfce43fa45b378fa2e86347..59f52e4488ce23a0a3771f8437f2551cb3d1588f 100644 (file)
--- a/ssl/record/methods/ktls_meth.c
+++ b/ssl/record/methods/ktls_meth.c
@@ -481,8 +481,8 @@ ktls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
                       const EVP_CIPHER *ciph, size_t taglen,
                       /* TODO(RECLAYER): This probably should not be an int */
                       int mactype,
-                      const EVP_MD *md, const SSL_COMP *comp, BIO *transport,
-                      BIO_ADDR *local, BIO_ADDR *peer,
+                      const EVP_MD *md, const SSL_COMP *comp, BIO *prev,
+                      BIO *transport, BIO *next, BIO_ADDR *local, BIO_ADDR *peer,
                       const OSSL_PARAM *settings, const OSSL_PARAM *options,
                       OSSL_RECORD_LAYER **retrl,
                       /* TODO(RECLAYER): Remove me */
@@ -492,8 +492,9 @@ ktls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
 
     ret = tls_int_new_record_layer(libctx, propq, vers, role, direction, level,
                                    key, keylen, iv, ivlen, mackey, mackeylen,
-                                   ciph, taglen, mactype, md, comp, transport,
-                                   local, peer, settings, options, retrl, s);
+                                   ciph, taglen, mactype, md, comp, prev,
+                                   transport, next, local, peer, settings,
+                                   options, retrl, s);
 
     if (ret != OSSL_RECORD_RETURN_SUCCESS)
         return ret;

diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h
index ef124c8237c366c4f36b7d8e559d66330263c209..8cad4b39794f4b345fa0a8ab98c14aa474ac54dc 100644 (file)
--- a/ssl/record/methods/recmethod_local.h
+++ b/ssl/record/methods/recmethod_local.h
@@ -68,7 +68,22 @@ struct ossl_record_layer_st
     int version;
     int role;
     int direction;
+
+    /*
+     * A BIO containing any data read in the previous epoch that was destined
+     * for this epoch
+     */
+    BIO *prev;
+
+    /* The transport BIO */
     BIO *bio;
+
+    /*
+     * A BIO where we will send any data read by us that is destined for the
+     * next epoch.
+     */
+    BIO *next;
+
     /* Types match the equivalent structures in the SSL object */
     uint64_t options;
     /*
@@ -190,13 +205,14 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
                          const EVP_CIPHER *ciph, size_t taglen,
                          /* TODO(RECLAYER): This probably should not be an int */
                          int mactype,
-                         const EVP_MD *md, const SSL_COMP *comp, BIO *transport,
+                         const EVP_MD *md, const SSL_COMP *comp, BIO *prev,
+                         BIO *transport, BIO *next,
                          BIO_ADDR *local, BIO_ADDR *peer,
                          const OSSL_PARAM *settings, const OSSL_PARAM *options,
                          OSSL_RECORD_LAYER **retrl,
                          /* TODO(RECLAYER): Remove me */
                          SSL_CONNECTION *s);
-void tls_free(OSSL_RECORD_LAYER *rl);
+int tls_free(OSSL_RECORD_LAYER *rl);
 int tls_reset(OSSL_RECORD_LAYER *rl);
 int tls_unprocessed_read_pending(OSSL_RECORD_LAYER *rl);
 int tls_processed_read_pending(OSSL_RECORD_LAYER *rl);

diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
index fa6551dffb5385c579cef58b159bdf5e484a78d0..2b26829e4bca96b1f6d7bb33da0ecd70b357d995 100644 (file)
--- a/ssl/record/methods/tls_common.c
+++ b/ssl/record/methods/tls_common.c
@@ -19,6 +19,8 @@
 
 # define SSL_AD_NO_ALERT    -1
 
+static void tls_int_free(OSSL_RECORD_LAYER *rl);
+
 void ossl_rlayer_fatal(OSSL_RECORD_LAYER *rl, int al, int reason,
                        const char *fmt, ...)
 {
@@ -284,6 +286,7 @@ int tls_default_read_n(OSSL_RECORD_LAYER *rl, size_t n, size_t max, int extend,
     while (left < n) {
         size_t bioread = 0;
         int ret;
+        BIO *bio = rl->prev != NULL ? rl->prev : rl->bio;
 
         /*
          * Now we have len+left bytes at the front of s->s3.rbuf.buf and
@@ -292,14 +295,23 @@ int tls_default_read_n(OSSL_RECORD_LAYER *rl, size_t n, size_t max, int extend,
          */
 
         clear_sys_error();
-        if (rl->bio != NULL) {
-            ret = BIO_read(rl->bio, pkt + len + left, max - left);
+        if (bio != NULL) {
+            ret = BIO_read(bio, pkt + len + left, max - left);
             if (ret > 0) {
                 bioread = ret;
                 ret = OSSL_RECORD_RETURN_SUCCESS;
-            } else if (BIO_should_retry(rl->bio)) {
+            } else if (BIO_should_retry(bio)) {
+                if (rl->prev != NULL) {
+                    /*
+                     * We were reading from the previous epoch. Now there is no
+                     * more data, so swap to the actual transport BIO
+                     */
+                    BIO_free(rl->prev);
+                    rl->prev = NULL;
+                    continue;
+                }
                 ret = OSSL_RECORD_RETURN_RETRY;
-            } else if (BIO_eof(rl->bio)) {
+            } else if (BIO_eof(bio)) {
                 ret = OSSL_RECORD_RETURN_EOF;
             } else {
                 ret = OSSL_RECORD_RETURN_FATAL;
@@ -989,10 +1001,10 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
                          const EVP_CIPHER *ciph, size_t taglen,
                          /* TODO(RECLAYER): This probably should not be an int */
                          int mactype,
-                         const EVP_MD *md, const SSL_COMP *comp, BIO *transport,
-                         BIO_ADDR *local, BIO_ADDR *peer,
-                         const OSSL_PARAM *settings, const OSSL_PARAM *options,
-                         OSSL_RECORD_LAYER **retrl,
+                         const EVP_MD *md, const SSL_COMP *comp, BIO *prev,
+                         BIO *transport, BIO *next, BIO_ADDR *local,
+                         BIO_ADDR *peer, const OSSL_PARAM *settings,
+                         const OSSL_PARAM *options, OSSL_RECORD_LAYER **retrl,
                          /* TODO(RECLAYER): Remove me */
                          SSL_CONNECTION *s)
 {
@@ -1006,11 +1018,6 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
         return OSSL_RECORD_RETURN_FATAL;
     }
 
-    if (transport != NULL && !BIO_up_ref(transport)) {
-        RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE);
-        goto err;
-    }
-
     /*
      * TODO(RECLAYER): Need to handle the case where the params are updated
      * after the record layer has been created.
@@ -1058,10 +1065,18 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
     if (!tls_set1_bio(rl, transport))
         goto err;
 
+    if (prev != NULL && !BIO_up_ref(prev))
+        goto err;
+    rl->prev = prev;
+
+    if (next != NULL && !BIO_up_ref(next))
+        goto err;
+    rl->next = next;
+
     *retrl = rl;
     return OSSL_RECORD_RETURN_SUCCESS;
  err:
-    OPENSSL_free(rl);
+    tls_int_free(rl);
     return OSSL_RECORD_RETURN_FATAL;
 }
 
@@ -1073,8 +1088,8 @@ tls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
                      const EVP_CIPHER *ciph, size_t taglen,
                      /* TODO(RECLAYER): This probably should not be an int */
                      int mactype,
-                     const EVP_MD *md, const SSL_COMP *comp, BIO *transport,
-                     BIO_ADDR *local, BIO_ADDR *peer,
+                     const EVP_MD *md, const SSL_COMP *comp, BIO *prev, 
+                     BIO *transport, BIO *next, BIO_ADDR *local, BIO_ADDR *peer,
                      const OSSL_PARAM *settings, const OSSL_PARAM *options,
                      OSSL_RECORD_LAYER **retrl,
                      /* TODO(RECLAYER): Remove me */
@@ -1084,8 +1099,9 @@ tls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
     
     ret = tls_int_new_record_layer(libctx, propq, vers, role, direction, level,
                                    key, keylen, iv, ivlen, mackey, mackeylen,
-                                   ciph, taglen, mactype, md, comp, transport,
-                                   local, peer, settings, options, retrl, s);
+                                   ciph, taglen, mactype, md, comp, prev,
+                                   transport, next, local, peer, settings,
+                                   options, retrl, s);
 
     if (ret != OSSL_RECORD_RETURN_SUCCESS)
         return ret;
@@ -1147,8 +1163,8 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
                       const EVP_CIPHER *ciph, size_t taglen,
                       /* TODO(RECLAYER): This probably should not be an int */
                       int mactype,
-                      const EVP_MD *md, const SSL_COMP *comp, BIO *transport,
-                      BIO_ADDR *local, BIO_ADDR *peer,
+                      const EVP_MD *md, const SSL_COMP *comp, BIO *prev,
+                      BIO *transport, BIO *next, BIO_ADDR *local, BIO_ADDR *peer,
                       const OSSL_PARAM *settings, const OSSL_PARAM *options,
                       OSSL_RECORD_LAYER **retrl,
                       /* TODO(RECLAYER): Remove me */
@@ -1159,8 +1175,9 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
 
     ret = tls_int_new_record_layer(libctx, propq, vers, role, direction, level,
                                    key, keylen, iv, ivlen, mackey, mackeylen,
-                                   ciph, taglen, mactype, md, comp, transport,
-                                   local, peer, settings, options, retrl, s);
+                                   ciph, taglen, mactype, md, comp, prev,
+                                   transport, next, local, peer, settings,
+                                   options, retrl, s);
 
     if (ret != OSSL_RECORD_RETURN_SUCCESS)
         return ret;
@@ -1171,13 +1188,43 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
     return OSSL_RECORD_RETURN_SUCCESS;
 }
 
-void tls_free(OSSL_RECORD_LAYER *rl)
+static void tls_int_free(OSSL_RECORD_LAYER *rl)
 {
     /* TODO(RECLAYER): Cleanse sensitive fields */
+    BIO_free(rl->prev);
     BIO_free(rl->bio);
+    BIO_free(rl->next);
+    SSL3_BUFFER_release(&rl->rbuf);
+
+    EVP_CIPHER_CTX_free(rl->enc_read_ctx);
+    EVP_MD_CTX_free(rl->read_hash);
+    COMP_CTX_free(rl->expand);
+
     OPENSSL_free(rl);
 }
 
+int tls_free(OSSL_RECORD_LAYER *rl)
+{
+    SSL3_BUFFER *rbuf;
+    size_t left, written;
+    int ret = 1;
+
+    rbuf = &rl->rbuf;
+
+    left = SSL3_BUFFER_get_left(rbuf);
+    if (left > 0) {
+        /*
+         * This record layer is closing but we still have data left in our
+         * buffer. It must be destined for the next epoch - so push it there.
+         */
+        ret = BIO_write_ex(rl->next, rbuf->buf + rbuf->offset, left, &written);
+    }
+    tls_int_free(rl);
+
+    return ret;
+}
+
+
 int tls_reset(OSSL_RECORD_LAYER *rl)
 {
     memset(rl, 0, sizeof(*rl));

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index c5f4591391005144116f501a2d0f0eb739e432d5..8ea42d19b911b6da86d8931d0617d208d9121981 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1806,8 +1806,10 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
 
     meth = ssl_select_next_record_layer(s, level);
 
-    if (s->rrlmethod != NULL)
-        s->rrlmethod->free(s->rrl);
+    if (s->rrlmethod != NULL && !s->rrlmethod->free(s->rrl)) {
+        ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
     if (meth != NULL)
         s->rrlmethod = meth;
@@ -1834,14 +1836,24 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
 
     for (;;) {
         int rlret;
+        BIO *prev = s->rrlnext;
+
+        s->rrlnext = BIO_new(BIO_s_mem());
+
+        if (s->rrlnext == NULL) {
+            BIO_free(prev);
+            SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+            goto err;
+        }
 
         rlret = s->rrlmethod->new_record_layer(sctx->libctx, sctx->propq,
                                                version, s->server, direction,
                                                level, key, keylen, iv, ivlen,
                                                mackey, mackeylen, ciph, taglen,
-                                               mactype, md, comp,  s->rbio,
-                                               NULL, NULL, NULL, options,
-                                               &s->rrl, s);
+                                               mactype, md, comp, prev, s->rbio,
+                                               s->rrlnext, NULL, NULL, NULL,
+                                               options, &s->rrl, s);
+        BIO_free(prev);
         switch (rlret) {
         case OSSL_RECORD_RETURN_FATAL:
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_RECORD_LAYER_FAILURE);

diff --git a/ssl/record/recordmethod.h b/ssl/record/recordmethod.h
index 8f025cc7859d3a48ddfc4f6f79300eafc0b8da3d..b86de6518afa4e4a2170b3c53a63b7f4798c0289 100644 (file)
--- a/ssl/record/recordmethod.h
+++ b/ssl/record/recordmethod.h
@@ -158,14 +158,17 @@ struct ossl_record_method_st {
                             int mactype,
                             const EVP_MD *md,
                             const SSL_COMP *comp,
-                            BIO *transport, BIO_ADDR *local,
+                            BIO *prev,
+                            BIO *transport,
+                            BIO *next,
+                            BIO_ADDR *local,
                             BIO_ADDR *peer,
                             const OSSL_PARAM *settings,
                             const OSSL_PARAM *options,
                             OSSL_RECORD_LAYER **ret,
                             /* TODO(RECLAYER): Remove me */
                             SSL_CONNECTION *s);
-    void (*free)(OSSL_RECORD_LAYER *rl);
+    int (*free)(OSSL_RECORD_LAYER *rl);
 
     int (*reset)(OSSL_RECORD_LAYER *rl); /* Is this needed? */
 

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 9905a188b9996eca61674625f8ae829db1604017..402ae157b5e14dda511fa459bf0912a54e0f2f4c 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -656,6 +656,8 @@ int ossl_ssl_connection_reset(SSL *s)
     }
 
     RECORD_LAYER_clear(&sc->rlayer);
+    BIO_free(sc->rrlnext);
+    sc->rrlnext = NULL;
 
     /*
      * TODO(RECLAYER): The record method should probably initialy come from the
@@ -1351,6 +1353,10 @@ void ossl_ssl_connection_free(SSL *ssl)
     if (s == NULL)
         return;
 
+    if (s->rrlmethod != NULL)
+        s->rrlmethod->free(s->rrl); /* Ignore return value */
+    BIO_free(s->rrlnext);
+
     X509_VERIFY_PARAM_free(s->param);
     dane_final(&s->dane);
 

diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 90fb9516abac6219a573775b0b6e49ef83c176b0..b4920a1c12e3450f78cf1ba0a7aaa7ce21135867 100644 (file)
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -1771,7 +1771,8 @@ struct ssl_connection_st {
     const OSSL_RECORD_METHOD *rrlmethod;
     /* The read direction record layer */
     OSSL_RECORD_LAYER *rrl;
-
+    /* BIO to store data destined for the next record layer epoch */
+    BIO *rrlnext;
 
     /* Default password callback. */
     pem_password_cb *default_passwd_callback;