Move the record block_padding capability fully into the record layer

Previously we were referencing the block_padding value through the
SSL_CONNECTION. Now it is held within OSSL_RECORD_LAYER.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19198)

diff --git a/include/openssl/core_names.h b/include/openssl/core_names.h
index 487b928e44e05c0a11428138e5e559e37403f3e0..d0b9c37bc5a6c558745f2a5cbcbd3e3e67a7c247 100644 (file)
--- a/include/openssl/core_names.h
+++ b/include/openssl/core_names.h
@@ -572,6 +572,7 @@ extern "C" {
 #define OSSL_LIBSSL_RECORD_LAYER_PARAM_TLSTREE        "tlstree"
 #define OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_FRAG_LEN   "max_frag_len"
 #define OSSL_LIBSSL_RECORD_LAYER_PARAM_MAX_EARLY_DATA "max_early_data"
+#define OSSL_LIBSSL_RECORD_LAYER_PARAM_BLOCK_PADDING  "block_padding"
 
 # ifdef __cplusplus
 }

diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c
index 663dbd1d37be23f415c643295166bc7e0d359c5c..7f6c9602f0c28ff5982a25be057165fc32425ca2 100644 (file)
--- a/ssl/record/methods/ktls_meth.c
+++ b/ssl/record/methods/ktls_meth.c
@@ -404,15 +404,14 @@ static int ktls_set_crypto_state(OSSL_RECORD_LAYER *rl, int level,
     if (!ktls_int_check_supported_cipher(rl, ciph, md, taglen))
         return OSSL_RECORD_RETURN_NON_FATAL_ERR;
 
-    /*
-     * TODO(RECLAYER): For the write side we need to add a check for
-     * use of s->record_padding_cb
-     */
-
     /* All future data will get encrypted by ktls. Flush the BIO or skip ktls */
     if (rl->direction == OSSL_RECORD_DIRECTION_WRITE) {
-       if (BIO_flush(rl->bio) <= 0)
-           return OSSL_RECORD_RETURN_NON_FATAL_ERR;
+        if (BIO_flush(rl->bio) <= 0)
+            return OSSL_RECORD_RETURN_NON_FATAL_ERR;
+
+        /* KTLS does not support record padding */
+        if (rl->padding != NULL || rl->block_padding > 0)
+            return OSSL_RECORD_RETURN_NON_FATAL_ERR;
     }
 
     if (!ktls_configure_crypto(rl->libctx, rl->version, ciph, md, rl->sequence,

diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h
index 5fa451d916fd18f19562e3abd39c3cf83c12d3d8..850c7ae8fa66c59b2cbf4e873e5d0efd3e82d5ad 100644 (file)
--- a/ssl/record/methods/recmethod_local.h
+++ b/ssl/record/methods/recmethod_local.h
@@ -167,6 +167,9 @@ struct ossl_record_layer_st
     /* The amount of early data that we have sent/received */
     size_t early_data_count;
 
+    /* TLSv1.3 record padding */
+    size_t block_padding;
+
     /* Only used by SSLv3 */
     unsigned char mac_secret[EVP_MAX_MD_SIZE];
 

diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
index e3cc6c14dc96302980200a29ce90df738321a3c8..85b5bcd483b35dec82cd7cb1a214522ce51d60e0 100644 (file)
--- a/ssl/record/methods/tls_common.c
+++ b/ssl/record/methods/tls_common.c
@@ -1101,11 +1101,20 @@ int tls_set_options(OSSL_RECORD_LAYER *rl, const OSSL_PARAM *options)
         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->direction == OSSL_RECORD_DIRECTION_READ) {
+        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;
+        }
+    } else {
+        p = OSSL_PARAM_locate_const(options,
+                                    OSSL_LIBSSL_RECORD_LAYER_PARAM_BLOCK_PADDING);
+        if (p != NULL && !OSSL_PARAM_get_size_t(p, &rl->block_padding)) {
+            ERR_raise(ERR_LIB_SSL, SSL_R_FAILED_TO_GET_PARAMETER);
+            return 0;
+        }
     }
 
     if (rl->level == OSSL_RECORD_PROTECTION_LEVEL_APPLICATION) {
@@ -1666,20 +1675,20 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
 
                 if (rl->padding != NULL) {
                     padding = rl->padding(rl->cbarg, thistempl->type, rlen);
-                } else if (s->block_padding > 0) {
-                    size_t mask = s->block_padding - 1;
+                } else if (rl->block_padding > 0) {
+                    size_t mask = rl->block_padding - 1;
                     size_t remainder;
 
                     /* optimize for power of 2 */
-                    if ((s->block_padding & mask) == 0)
+                    if ((rl->block_padding & mask) == 0)
                         remainder = rlen & mask;
                     else
-                        remainder = rlen % s->block_padding;
+                        remainder = rlen % rl->block_padding;
                     /* don't want to add a block of padding if we don't have to */
                     if (remainder == 0)
                         padding = 0;
                     else
-                        padding = s->block_padding - remainder;
+                        padding = rl->block_padding - remainder;
                 }
                 if (padding > 0) {
                     /* do not allow the record to exceed max plaintext length */

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 6d0251407ff683b06407fcef9049ab2b8f904a99..9152ea86aacf66369bfae2f6d54d0b1cb061983d 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1198,7 +1198,8 @@ static size_t rlayer_padding_wrapper(void *cbarg, int type, size_t len)
     SSL_CONNECTION *s = cbarg;
     SSL *ssl = SSL_CONNECTION_GET_SSL(s);
 
-    return s->record_padding_cb(ssl, type, len, s->record_padding_arg);
+    return s->rlayer.record_padding_cb(ssl, type, len,
+                                       s->rlayer.record_padding_arg);
 }
 
 static const OSSL_DISPATCH rlayer_dispatch[] = {
@@ -1315,6 +1316,9 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
                                               &s->rlayer.default_read_buf_len);
         *opts++ = OSSL_PARAM_construct_int(OSSL_LIBSSL_RECORD_LAYER_PARAM_READ_AHEAD,
                                            &s->rlayer.read_ahead);
+    } else {
+        *opts++ = OSSL_PARAM_construct_size_t(OSSL_LIBSSL_RECORD_LAYER_PARAM_BLOCK_PADDING,
+                                              &s->rlayer.block_padding);
     }
     *opts = OSSL_PARAM_construct_end();
 
@@ -1414,7 +1418,7 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
                     continue;
                 break;
             case OSSL_FUNC_RLAYER_PADDING:
-                if (s->record_padding_cb == NULL)
+                if (s->rlayer.record_padding_cb == NULL)
                     continue;
                 break;
             default:

diff --git a/ssl/record/record.h b/ssl/record/record.h
index 19021ca0135e3a89903735c49144549794178566..03151e58d60bff565773a565b8281659913dc73e 100644 (file)
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -174,6 +174,11 @@ typedef struct record_layer_st {
     unsigned int alert_count;
     DTLS_RECORD_LAYER *d;
 
+    /* TLS1.3 padding callback */
+    size_t (*record_padding_cb)(SSL *s, int type, size_t len, void *arg);
+    void *record_padding_arg;
+    size_t block_padding;
+
     /* 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 */

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index f77bd7b2799b3e2910eb3bd4ee83bf376b71c4d6..35758df33bfc7b2e2d90fb76b0c9a8c5cdb0f0d0 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -797,9 +797,9 @@ SSL *ossl_ssl_connection_new(SSL_CTX *ctx)
     s->msg_callback_arg = ctx->msg_callback_arg;
     s->verify_mode = ctx->verify_mode;
     s->not_resumable_session_cb = ctx->not_resumable_session_cb;
-    s->record_padding_cb = ctx->record_padding_cb;
-    s->record_padding_arg = ctx->record_padding_arg;
-    s->block_padding = ctx->block_padding;
+    s->rlayer.record_padding_cb = ctx->record_padding_cb;
+    s->rlayer.record_padding_arg = ctx->record_padding_arg;
+    s->rlayer.block_padding = ctx->block_padding;
     s->sid_ctx_length = ctx->sid_ctx_length;
     if (!ossl_assert(s->sid_ctx_length <= sizeof(s->sid_ctx)))
         goto err;
@@ -5397,7 +5397,7 @@ int SSL_set_record_padding_callback(SSL *ssl,
 
     b = SSL_get_wbio(ssl);
     if (b == NULL || !BIO_get_ktls_send(b)) {
-        sc->record_padding_cb = cb;
+        sc->rlayer.record_padding_cb = cb;
         return 1;
     }
     return 0;
@@ -5410,7 +5410,7 @@ void SSL_set_record_padding_callback_arg(SSL *ssl, void *arg)
     if (sc == NULL)
         return;
 
-    sc->record_padding_arg = arg;
+    sc->rlayer.record_padding_arg = arg;
 }
 
 void *SSL_get_record_padding_callback_arg(const SSL *ssl)
@@ -5420,7 +5420,7 @@ void *SSL_get_record_padding_callback_arg(const SSL *ssl)
     if (sc == NULL)
         return NULL;
 
-    return sc->record_padding_arg;
+    return sc->rlayer.record_padding_arg;
 }
 
 int SSL_set_block_padding(SSL *ssl, size_t block_size)
@@ -5432,9 +5432,9 @@ int SSL_set_block_padding(SSL *ssl, size_t block_size)
 
     /* block size of 0 or 1 is basically no padding */
     if (block_size == 1)
-        sc->block_padding = 0;
+        sc->rlayer.block_padding = 0;
     else if (block_size <= SSL3_RT_MAX_PLAIN_LENGTH)
-        sc->block_padding = block_size;
+        sc->rlayer.block_padding = block_size;
     else
         return 0;
     return 1;

diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 61b77602bac53d7f6e56ea29f4acf320f6b3c471..90e6209f77741da861f7198efe4636baae50b57b 100644 (file)
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -1793,11 +1793,6 @@ struct ssl_connection_st {
      */
     uint32_t early_data_count;
 
-    /* TLS1.3 padding callback */
-    size_t (*record_padding_cb)(SSL *s, int type, size_t len, void *arg);
-    void *record_padding_arg;
-    size_t block_padding;
-
     /* The number of TLS1.3 tickets to automatically send */
     size_t num_tickets;
     /* The number of TLS1.3 tickets actually sent so far */

diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c
index 1e8e189b7c91199d96ad38fa154d61d90108af60..431a7fff7083096fc0d1df786a751893facce3cb 100644 (file)
--- a/ssl/tls13_enc.c
+++ b/ssl/tls13_enc.c
@@ -735,7 +735,7 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which)
         goto skip_ktls;
 
     /* ktls does not support record padding */
-    if (s->record_padding_cb != NULL)
+    if (s->rlayer.record_padding_cb != NULL || s->rlayer.block_padding > 0)
         goto skip_ktls;
 
     /* check that cipher is supported */