Ensure various record layer options can be updated

We would like the capability for the options/mode/read_ahead settings to be updateable after the record layer object has been instantiated. Reviewed-by: Hugo Landau <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from openssl#18132)
t-j-h · Aug 18, 2022 · 4566dae · 4566dae
1 parent 19d0044
commit 4566dae
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 42 deletions.
diff --git a/ssl/record/methods/dtls_meth.c b/ssl/record/methods/dtls_meth.c
@@ -729,5 +729,6 @@ const OSSL_RECORD_METHOD ossl_dtls_record_method = {
     tls_set_first_handshake,
     tls_set_max_pipelines,
     dtls_set_in_init,
-    tls_get_state
+    tls_get_state,
+    tls_set_options
 };
diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c
@@ -543,5 +543,6 @@ const OSSL_RECORD_METHOD ossl_ktls_record_method = {
     tls_set_first_handshake,
     tls_set_max_pipelines,
     NULL,
-    tls_get_state
+    tls_get_state,
+    tls_set_options
 };
diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h
@@ -286,4 +286,5 @@ void tls_set_first_handshake(OSSL_RECORD_LAYER *rl, int first);
 void tls_set_max_pipelines(OSSL_RECORD_LAYER *rl, size_t max_pipelines);
 void tls_get_state(OSSL_RECORD_LAYER *rl, const char **shortstr,
                    const char **longstr);
+int tls_set_options(OSSL_RECORD_LAYER *rl, const OSSL_PARAM *options);
 int rlayer_setup_read_buffer(OSSL_RECORD_LAYER *rl);
diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
@@ -998,6 +998,47 @@ int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle)
     return OSSL_RECORD_RETURN_SUCCESS;
 }
 
+int tls_set_options(OSSL_RECORD_LAYER *rl, const OSSL_PARAM *options)
+{
+    const OSSL_PARAM *p;
+
+    p = OSSL_PARAM_locate_const(options, OSSL_LIBSSL_RECORD_LAYER_PARAM_OPTIONS);
+    if (p != NULL && !OSSL_PARAM_get_uint64(p, &rl->options)) {
+        ERR_raise(ERR_LIB_SSL, SSL_R_FAILED_TO_GET_PARAMETER);
+        return 0;
+    }
+
+    p = OSSL_PARAM_locate_const(options, OSSL_LIBSSL_RECORD_LAYER_PARAM_MODE);
+    if (p != NULL && !OSSL_PARAM_get_uint32(p, &rl->mode)) {
+        ERR_raise(ERR_LIB_SSL, SSL_R_FAILED_TO_GET_PARAMETER);
+        return 0;
+    }
+
+    p = OSSL_PARAM_locate_const(options,
+                                OSSL_LIBSSL_RECORD_LAYER_READ_BUFFER_LEN);
+    if (p != NULL && !OSSL_PARAM_get_size_t(p, &rl->rbuf.default_len)) {
+        ERR_raise(ERR_LIB_SSL, SSL_R_FAILED_TO_GET_PARAMETER);
+        return 0;
+    }
+
+    if (rl->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.
+         */
+        p = OSSL_PARAM_locate_const(options,
+                                    OSSL_LIBSSL_RECORD_LAYER_PARAM_READ_AHEAD);
+        if (p != NULL && !OSSL_PARAM_get_int(p, &rl->read_ahead)) {
+            ERR_raise(ERR_LIB_SSL, SSL_R_FAILED_TO_GET_PARAMETER);
+            return 0;
+        }
+    }
+
+    return 1;
+}
+
 int
 tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
                          int role, int direction, int level, unsigned char *key,
@@ -1022,28 +1063,6 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
         return OSSL_RECORD_RETURN_FATAL;
     }
 
-    /*
-     * 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);
-        goto err;
-    }
-
-    p = OSSL_PARAM_locate_const(options, OSSL_LIBSSL_RECORD_LAYER_PARAM_MODE);
-    if (p != NULL && !OSSL_PARAM_get_uint32(p, &rl->mode)) {
-        RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_FAILED_TO_GET_PARAMETER);
-        goto err;
-    }
-
-    p = OSSL_PARAM_locate_const(options, OSSL_LIBSSL_RECORD_LAYER_READ_BUFFER_LEN);
-    if (p != NULL && !OSSL_PARAM_get_size_t(p, &rl->rbuf.default_len)) {
-        RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_FAILED_TO_GET_PARAMETER);
-        goto err;
-    }
-
     /* Loop through all the settings since they must all be understood */
     if (settings != NULL) {
         for (p = settings; p->key != NULL; p++) {
@@ -1088,21 +1107,6 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
         }
     }
 
-
-    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.
-         */
-        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;
     rl->propq = propq;
 
@@ -1145,6 +1149,11 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
         }
     }
 
+    if (!tls_set_options(rl, options)) {
+        RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_FAILED_TO_GET_PARAMETER);
+        goto err;
+    }
+
     *retrl = rl;
     return OSSL_RECORD_RETURN_SUCCESS;
  err:
@@ -1398,5 +1407,6 @@ const OSSL_RECORD_METHOD ossl_tls_record_method = {
     tls_set_first_handshake,
     tls_set_max_pipelines,
     NULL,
-    tls_get_state
+    tls_get_state,
+    tls_set_options
 };
diff --git a/ssl/record/recordmethod.h b/ssl/record/recordmethod.h
@@ -320,6 +320,12 @@ struct ossl_record_method_st {
      */
     void (*get_state)(OSSL_RECORD_LAYER *rl, const char **shortstr,
                       const char **longstr);
+
+    /*
+     * Set new options or modify ones that were originaly specified in the
+     * new_record_layer call.
+     */
+    int (*set_options)(OSSL_RECORD_LAYER *rl, const OSSL_PARAM *options);
 };
 
 

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
@@ -21,6 +21,7 @@
 #include <openssl/async.h>
 #include <openssl/ct.h>
 #include <openssl/trace.h>
+#include <openssl/core_names.h>
 #include "internal/cryptlib.h"
 #include "internal/refcount.h"
 #include "internal/ktls.h"
@@ -1733,11 +1734,19 @@ void SSL_set_verify_depth(SSL *s, int depth)
 void SSL_set_read_ahead(SSL *s, int yes)
 {
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);
+    OSSL_PARAM options[2], *opts = options;
 
     if (sc == NULL)
         return;
 
     RECORD_LAYER_set_read_ahead(&sc->rlayer, yes);
+
+    *opts++ = OSSL_PARAM_construct_int(OSSL_LIBSSL_RECORD_LAYER_PARAM_READ_AHEAD,
+                                       &sc->rlayer.read_ahead);
+    *opts = OSSL_PARAM_construct_end();
+
+    /* Ignore return value */
+    sc->rlayer.rrlmethod->set_options(sc->rlayer.rrl, options);
 }
 
 int SSL_get_read_ahead(const SSL *s)
@@ -2736,7 +2745,20 @@ long SSL_ctrl(SSL *s, int cmd, long larg, void *parg)
         return 1;
 
     case SSL_CTRL_MODE:
-        return (sc->mode |= larg);
+    {
+        OSSL_PARAM options[2], *opts = options;
+
+        sc->mode |= larg;
+
+        *opts++ = OSSL_PARAM_construct_uint32(OSSL_LIBSSL_RECORD_LAYER_PARAM_MODE,
+                                              &sc->mode);
+        *opts = OSSL_PARAM_construct_end();
+
+        /* Ignore return value */
+        sc->rlayer.rrlmethod->set_options(sc->rlayer.rrl, options);
+
+        return sc->mode;
+    }
     case SSL_CTRL_CLEAR_MODE:
         return (sc->mode &= ~larg);
     case SSL_CTRL_GET_MAX_CERT_LIST:
@@ -5652,11 +5674,21 @@ uint64_t SSL_CTX_set_options(SSL_CTX *ctx, uint64_t op)
 uint64_t SSL_set_options(SSL *s, uint64_t op)
 {
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);
+    OSSL_PARAM options[2], *opts = options;
 
     if (sc == NULL)
         return 0;
 
-    return sc->options |= op;
+    sc->options |= op;
+
+    *opts++ = OSSL_PARAM_construct_uint64(OSSL_LIBSSL_RECORD_LAYER_PARAM_OPTIONS,
+                                          &sc->options);
+    *opts = OSSL_PARAM_construct_end();
+
+    /* Ignore return value */
+    sc->rlayer.rrlmethod->set_options(sc->rlayer.rrl, options);
+
+    return sc->options;
 }
 
 uint64_t SSL_CTX_clear_options(SSL_CTX *ctx, uint64_t op)