Ensure various SSL options are passed down to the record layer
authorMatt Caswell <matt@openssl.org>
Fri, 6 May 2022 14:10:00 +0000 (15:10 +0100)
committerMatt Caswell <matt@openssl.org>
Thu, 18 Aug 2022 15:38:12 +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/record/methods/tlsrecord.c
ssl/record/rec_layer_s3.c
ssl/record/record.h
ssl/record/recordmethod.h
ssl/s3_enc.c
ssl/ssl_lib.c
ssl/t1_enc.c
ssl/tls13_enc.c
test/tls13secretstest.c

index 5a50ef25f0ee0a66c01f6387221e0029339a5094..d12b38ce3947a16a672fd45a265238c167b24501 100644 (file)
@@ -1486,7 +1486,7 @@ static int tls_read_n(OSSL_RECORD_LAYER *rl, size_t n, size_t max, int extend,
      * Ktls always reads full records.
      * Also, we always act like read_ahead is set for DTLS.
      */
-    if (!BIO_get_ktls_recv(s->rbio) && !rl->read_ahead
+    if (!BIO_get_ktls_recv(rl->bio) && !rl->read_ahead
             && !rl->isdtls) {
         /* ignore max parameter */
         max = n;
@@ -2341,6 +2341,10 @@ static OSSL_RECORD_LAYER *tls_new_record_layer(OSSL_LIB_CTX *libctx,
         goto err;
     }
 
+    /*
+     * TODO(RECLAYER): Need to handle the case where the params are updated
+     * after the record layer has been created.
+     */
     p = OSSL_PARAM_locate_const(options, OSSL_LIBSSL_RECORD_LAYER_PARAM_OPTIONS);
     if (p != NULL && !OSSL_PARAM_get_uint64(p, &rl->options)) {
         RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_FAILED_TO_GET_PARAMETER);
@@ -2353,11 +2357,22 @@ static OSSL_RECORD_LAYER *tls_new_record_layer(OSSL_LIB_CTX *libctx,
         goto err;
     }
 
-
-    p = OSSL_PARAM_locate_const(options, OSSL_LIBSSL_RECORD_LAYER_PARAM_READ_AHEAD);
-    if (p != NULL && !OSSL_PARAM_get_int(p, &rl->read_ahead)) {
-        RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_FAILED_TO_GET_PARAMETER);
-        goto err;
+    if (level == OSSL_RECORD_PROTECTION_LEVEL_APPLICATION) {
+        /*
+         * We ignore any read_ahead setting prior to the application protection
+         * level. Otherwise we may read ahead data in a lower protection level
+         * that is destined for a higher protection level. To simplify the logic
+         * we don't support that at this stage.
+         */
+        /*
+         * TODO(RECLAYER): Handle the case of read_ahead at the application
+         * level and a key update/reneg occurs.
+         */
+        p = OSSL_PARAM_locate_const(options, OSSL_LIBSSL_RECORD_LAYER_PARAM_READ_AHEAD);
+        if (p != NULL && !OSSL_PARAM_get_int(p, &rl->read_ahead)) {
+            RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_FAILED_TO_GET_PARAMETER);
+            goto err;
+        }
     }
 
     rl->libctx = libctx;
index d4a92e540e8268f574584c1d3e1ca34b4a9ba994..61a4316704243553e33b242e778e832da5c0cdb1 100644 (file)
@@ -14,6 +14,8 @@
 #include <openssl/evp.h>
 #include <openssl/buffer.h>
 #include <openssl/rand.h>
+#include <openssl/core_names.h>
+#include <openssl/param_build.h>
 #include "record_local.h"
 #include "internal/packet.h"
 
@@ -1747,3 +1749,62 @@ size_t RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl)
 {
     return SSL3_RECORD_get_length(&rl->rrec[0]);
 }
+
+int ssl_set_new_record_layer(SSL_CONNECTION *s, const OSSL_RECORD_METHOD *meth,
+                             int version, int direction, int level,
+                             unsigned char *key, size_t keylen,
+                             unsigned char *iv,  size_t ivlen,
+                             unsigned char *mackey, size_t mackeylen,
+                             const EVP_CIPHER *ciph, size_t taglen,
+                             int mactype, const EVP_MD *md,
+                             const SSL_COMP *comp)
+{
+    OSSL_PARAM_BLD *tmpl = NULL;
+    OSSL_PARAM *options = NULL;
+    int ret = 0;
+    SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s);
+
+    if (s->rrlmethod != NULL)
+        s->rrlmethod->free(s->rrl);
+
+    if (meth != NULL)
+        s->rrlmethod = meth;
+
+    if (!ossl_assert(s->rrlmethod != NULL)) {
+        ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    if ((tmpl = OSSL_PARAM_BLD_new()) == NULL
+            || !OSSL_PARAM_BLD_push_uint64(tmpl,
+                                           OSSL_LIBSSL_RECORD_LAYER_PARAM_OPTIONS,
+                                           s->options)
+            || !OSSL_PARAM_BLD_push_uint32(tmpl,
+                                           OSSL_LIBSSL_RECORD_LAYER_PARAM_MODE,
+                                           s->mode)
+            || !OSSL_PARAM_BLD_push_int(tmpl,
+                                        OSSL_LIBSSL_RECORD_LAYER_PARAM_READ_AHEAD,
+                                        s->rlayer.read_ahead)
+            || (options = OSSL_PARAM_BLD_to_param(tmpl)) == NULL) {
+        ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    s->rrl = 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);
+    if (s->rrl == NULL) {
+        ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    ret = 1;
+ err:
+    OSSL_PARAM_free(options);
+    OSSL_PARAM_BLD_free(tmpl);
+
+    return ret;
+}
index 390ea92b3b2a648372d479263330b6b563180f89..fb836716fa0af5ee5a370f1182f2117f3a38cd9c 100644 (file)
@@ -8,6 +8,9 @@
  */
 
 typedef struct ssl_connection_st SSL_CONNECTION;
+typedef struct ssl3_buffer_st SSL3_BUFFER;
+
+#include "recordmethod.h"
 
 /*****************************************************************************
  *                                                                           *
@@ -16,7 +19,7 @@ typedef struct ssl_connection_st SSL_CONNECTION;
  *                                                                           *
  *****************************************************************************/
 
-typedef struct ssl3_buffer_st {
+struct ssl3_buffer_st {
     /* at least SSL3_RT_MAX_PACKET_SIZE bytes, see ssl3_setup_buffers() */
     unsigned char *buf;
     /* default buffer size (or 0 if no default set) */
@@ -29,7 +32,7 @@ typedef struct ssl3_buffer_st {
     size_t left;
     /* 'buf' is from application for KTLS */
     int app_buffer;
-} SSL3_BUFFER;
+};
 
 #define SEQ_NUM_SIZE                            8
 
@@ -279,3 +282,12 @@ int dtls_buffer_listen_record(SSL_CONNECTION *s, size_t len, unsigned char *seq,
 
 int ossl_tls_handle_rlayer_return(SSL_CONNECTION *s, int ret, char *file,
                                   int line);
+
+int ssl_set_new_record_layer(SSL_CONNECTION *s, const OSSL_RECORD_METHOD *meth,
+                             int version, int direction, int level,
+                             unsigned char *key, size_t keylen,
+                             unsigned char *iv,  size_t ivlen,
+                             unsigned char *mackey, size_t mackeylen,
+                             const EVP_CIPHER *ciph, size_t taglen,
+                             int mactype, const EVP_MD *md,
+                             const SSL_COMP *comp);
index d2eecb7dd7d988abb9b3dcc62365501025417192..a14737836dbe143540bf572e55911be886d59b0a 100644 (file)
@@ -7,7 +7,11 @@
  * https://www.openssl.org/source/license.html
  */
 
-#include <openssl/ssl.h>
+#ifndef OSSL_INTERNAL_RECORDMETHOD_H
+# define OSSL_INTERNAL_RECORDMETHOD_H
+# pragma once
+
+# include <openssl/ssl.h>
 
 /*
  * We use the term "record" here to refer to a packet of data. Records are
@@ -54,26 +58,26 @@ typedef struct ossl_record_method_st OSSL_RECORD_METHOD;
 typedef struct ossl_record_layer_st OSSL_RECORD_LAYER;
 
 
-#define OSSL_RECORD_ROLE_CLIENT 0
-#define OSSL_RECORD_ROLE_SERVER 1
+# define OSSL_RECORD_ROLE_CLIENT 0
+# define OSSL_RECORD_ROLE_SERVER 1
 
-#define OSSL_RECORD_DIRECTION_READ  0
-#define OSSL_RECORD_DIRECTION_WRITE 1
+# define OSSL_RECORD_DIRECTION_READ  0
+# define OSSL_RECORD_DIRECTION_WRITE 1
 
 /*
  * Protection level. For <= TLSv1.2 only "NONE" and "APPLICATION" are used.
  */
-#define OSSL_RECORD_PROTECTION_LEVEL_NONE        0
-#define OSSL_RECORD_PROTECTION_LEVEL_EARLY       1
-#define OSSL_RECORD_PROTECTION_LEVEL_HANDSHAKE   2
-#define OSSL_RECORD_PROTECTION_LEVEL_APPLICATION 3
+# define OSSL_RECORD_PROTECTION_LEVEL_NONE        0
+# define OSSL_RECORD_PROTECTION_LEVEL_EARLY       1
+# define OSSL_RECORD_PROTECTION_LEVEL_HANDSHAKE   2
+# define OSSL_RECORD_PROTECTION_LEVEL_APPLICATION 3
 
 
-#define OSSL_RECORD_RETURN_SUCCESS           1
-#define OSSL_RECORD_RETURN_RETRY             0
-#define OSSL_RECORD_RETURN_NON_FATAL_ERR    -1
-#define OSSL_RECORD_RETURN_FATAL            -2
-#define OSSL_RECORD_RETURN_EOF              -3
+# define OSSL_RECORD_RETURN_SUCCESS           1
+# define OSSL_RECORD_RETURN_RETRY             0
+# define OSSL_RECORD_RETURN_NON_FATAL_ERR    -1
+# define OSSL_RECORD_RETURN_FATAL            -2
+# define OSSL_RECORD_RETURN_EOF              -3
 
 /*
  * Template for creating a record. A record consists of the |type| of data it
@@ -290,4 +294,6 @@ struct ossl_record_method_st {
 
 /* Standard built-in record methods */
 extern const OSSL_RECORD_METHOD ossl_tls_record_method;
-extern const OSSL_RECORD_METHOD ossl_dtls_record_method;
\ No newline at end of file
+extern const OSSL_RECORD_METHOD ossl_dtls_record_method;
+
+#endif /* !defined(OSSL_INTERNAL_RECORDMETHOD_H) */
index 8e6db3f0c16c0a1b2ac276d8ce83b0a8fdd895b5..bd66f300ef07535832c26e2421b44a0bc4f55d56 100644 (file)
@@ -101,7 +101,6 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which)
     int mdi;
     size_t n, iv_len, key_len;
     int reuse_dd = 0;
-    SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s);
 
     ciph = s->s3.tmp.new_sym_enc;
     md = s->s3.tmp.new_hash;
@@ -147,18 +146,12 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which)
     }
 
     if (which & SSL3_CC_READ) {
-        s->rrlmethod->free(s->rrl);
-        s->rrl = s->rrlmethod->new_record_layer(sctx->libctx,
-                                                sctx->propq,
-                                                SSL3_VERSION, s->server,
-                                                OSSL_RECORD_DIRECTION_READ,
-                                                OSSL_RECORD_PROTECTION_LEVEL_APPLICATION,
-                                                key, key_len, iv, iv_len,
-                                                mac_secret, md_len, ciph, 0,
-                                                NID_undef, md, comp, s->rbio,
-                                                NULL, NULL, NULL, NULL, s);
-        if (s->rrl == NULL) {
-            SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+        if (!ssl_set_new_record_layer(s, NULL, SSL3_VERSION,
+                                      OSSL_RECORD_DIRECTION_READ,
+                                      OSSL_RECORD_PROTECTION_LEVEL_APPLICATION,
+                                      key, key_len, iv, iv_len, mac_secret,
+                                      md_len, ciph, 0, NID_undef, md, comp)) {
+            /* SSLfatal already called */
             goto err;
         }
 
index f1c2db02e22b89b06dc7a3be54735774ce58d8f7..e076a27560f113561326f527e795b667e80a5df0 100644 (file)
@@ -657,28 +657,20 @@ int ossl_ssl_connection_reset(SSL *s)
 
     RECORD_LAYER_clear(&sc->rlayer);
 
-    if (sc->rrlmethod != NULL)
-        sc->rrlmethod->free(sc->rrl);
-
     /*
-     * TODO(RECLAYER): This assignment should probably initialy come from the
+     * TODO(RECLAYER): The record method should probably initialy come from the
      * SSL_METHOD, and potentially be updated later. For now though we just
      * assign it.
      */
-    if (SSL_CONNECTION_IS_DTLS(sc))
-        sc->rrlmethod = &ossl_dtls_record_method;
-    else
-        sc->rrlmethod = &ossl_tls_record_method;
-
-    sc->rrl = sc->rrlmethod->new_record_layer(s->ctx->libctx, s->ctx->propq,
-                                            TLS_ANY_VERSION, sc->server,
-                                            OSSL_RECORD_DIRECTION_READ,
-                                            OSSL_RECORD_PROTECTION_LEVEL_NONE,
-                                            NULL, 0, NULL, 0, NULL, 0, NULL, 0,
-                                            NID_undef, NULL, NULL,  sc->rbio,
-                                            NULL, NULL, NULL, NULL, sc);
-    if (sc->rrl == NULL) {
-        ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
+    if (!ssl_set_new_record_layer(sc,
+                                  SSL_CONNECTION_IS_DTLS(sc) ? &ossl_dtls_record_method
+                                                             : &ossl_tls_record_method,
+                                  TLS_ANY_VERSION,
+                                  OSSL_RECORD_DIRECTION_READ,
+                                  OSSL_RECORD_PROTECTION_LEVEL_NONE,
+                                  NULL, 0, NULL, 0, NULL,  0, NULL, 0,
+                                  NID_undef, NULL, NULL)) {
+        /* SSLfatal already called */
         return 0;
     }
 
index 2539b42209b12196ada1a1f00cdf4a56f61c3d6b..6fee020f32f433d561e033c68f435ae6da5a3ecf 100644 (file)
@@ -275,21 +275,16 @@ int tls1_change_cipher_state(SSL_CONNECTION *s, int which)
                     taglen = EVP_CCM_TLS_TAG_LEN;
             }
 
-            s->rrlmethod->free(s->rrl);
-            s->rrl = s->rrlmethod->new_record_layer(sctx->libctx,
-                                                    sctx->propq,
-                                                    s->version, s->server,
-                                                    OSSL_RECORD_DIRECTION_READ,
-                                                    OSSL_RECORD_PROTECTION_LEVEL_APPLICATION,
-                                                    key, cl, iv, (size_t)k,
-                                                    mac_secret,
-                                                    mac_secret_size, c, taglen,
-                                                    mac_type, m, comp, s->rbio,
-                                                    NULL, NULL, NULL, NULL, s);
-            if (s->rrl == NULL) {
-                SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+            if (!ssl_set_new_record_layer(s, NULL, s->version,
+                                        OSSL_RECORD_DIRECTION_READ,
+                                        OSSL_RECORD_PROTECTION_LEVEL_APPLICATION,
+                                        key, cl, iv, (size_t)k, mac_secret,
+                                        mac_secret_size, c, taglen, mac_type, m,
+                                        comp)) {
+                /* SSLfatal already called */
                 goto err;
             }
+
             /* TODO(RECLAYER): Temporary - remove me */
             goto check_ktls;
         }
index 9001a4455a30696ffc2a56a259786d34732421cb..8ef0ca69818f058b6a0a4dd2b4bf7f4a28129023 100644 (file)
@@ -709,22 +709,16 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which)
                     : ((which &SSL3_CC_HANDSHAKE) != 0
                        ? OSSL_RECORD_PROTECTION_LEVEL_HANDSHAKE
                        : OSSL_RECORD_PROTECTION_LEVEL_APPLICATION);
-        s->rrlmethod->free(s->rrl);
-        s->rrl = s->rrlmethod->new_record_layer(sctx->libctx,
-                                                sctx->propq,
-                                                s->version, s->server,
-                                                OSSL_RECORD_DIRECTION_READ,
-                                                level, key, keylen, iv, ivlen,
-                                                NULL, 0, cipher, taglen,
-                                                NID_undef, NULL, NULL, s->rbio,
-                                                NULL, NULL, NULL, NULL, s);
-        if (s->rrl == NULL) {
-            SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+
+        if (!ssl_set_new_record_layer(s, NULL, s->version,
+                                    OSSL_RECORD_DIRECTION_READ,
+                                    level, key, keylen, iv, ivlen, NULL, 0,
+                                    cipher, taglen, NID_undef, NULL, NULL)) {
+            /* SSLfatal already called */
             goto err;
         }
     }
 
-
 #ifndef OPENSSL_NO_KTLS
 # if defined(OPENSSL_KTLS_TLS13)
     if (!(which & SSL3_CC_APPLICATION)
@@ -803,7 +797,6 @@ int tls13_update_key(SSL_CONNECTION *s, int sending)
     EVP_CIPHER_CTX *ciph_ctx;
     size_t keylen, ivlen, taglen;
     int ret = 0;
-    SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s);
 
     if (s->server == sending)
         insecret = s->server_app_traffic_secret;
@@ -833,24 +826,17 @@ int tls13_update_key(SSL_CONNECTION *s, int sending)
     memcpy(insecret, secret, hashlen);
 
     if (!sending) {
-        s->rrlmethod->free(s->rrl);
-        s->rrl = s->rrlmethod->new_record_layer(sctx->libctx,
-                                                sctx->propq,
-                                                s->version, s->server,
-                                                OSSL_RECORD_DIRECTION_READ,
-                                                OSSL_RECORD_PROTECTION_LEVEL_APPLICATION,
-                                                key, keylen, iv, ivlen,
-                                                NULL, 0, s->s3.tmp.new_sym_enc,
-                                                taglen, NID_undef, NULL, NULL,
-                                                s->rbio, NULL, NULL, NULL, NULL,
-                                                s);
-        if (s->rrl == NULL) {
-            SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+        if (!ssl_set_new_record_layer(s, NULL, s->version,
+                                OSSL_RECORD_DIRECTION_READ,
+                                OSSL_RECORD_PROTECTION_LEVEL_APPLICATION,
+                                key, keylen, iv, ivlen, NULL, 0,
+                                s->s3.tmp.new_sym_enc, taglen, NID_undef, NULL,
+                                NULL)) {
+            /* SSLfatal already called */
             goto err;
         }
     }
 
-
     s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
     ret = 1;
  err:
index 6a2479210adb3a827b72933013705fcec1da0a72..f8d14777b36170abd51ba3b57d91f891403fcfcb 100644 (file)
@@ -225,6 +225,18 @@ void ssl_evp_md_free(const EVP_MD *md)
 {
 }
 
+int ssl_set_new_record_layer(SSL_CONNECTION *s, const OSSL_RECORD_METHOD *meth,
+                             int version, int direction, int level,
+                             unsigned char *key, size_t keylen,
+                             unsigned char *iv,  size_t ivlen,
+                             unsigned char *mackey, size_t mackeylen,
+                             const EVP_CIPHER *ciph, size_t taglen,
+                             int mactype, const EVP_MD *md,
+                             const SSL_COMP *comp)
+{
+    return 0;
+}
+
 /* End of mocked out code */
 
 static int test_secret(SSL_CONNECTION *s, unsigned char *prk,