Extend TLSv1.3 record layer padding API calls

Added SSL_set_block_padding_ex() and SSL_CTX_set_block_padding_ex()
to allow separate padding block size values for handshake messages
and application data messages.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24796)

14 files changed, 177 insertions, 40 deletions

diff --git a/doc/man3/SSL_CONF_cmd.pod b/doc/man3/SSL_CONF_cmd.pod
index d9596b8231..d2fba6a876 100644
--- a/doc/man3/SSL_CONF_cmd.pod
+++ b/doc/man3/SSL_CONF_cmd.pod
@@ -227,9 +227,15 @@ deprecated alternative commands below.
 
 =item B<-record_padding> I<padding>
 
-Attempts to pad TLSv1.3 records so that they are a multiple of B<padding>
-in length on send. A B<padding> of 0 or 1 turns off padding. Otherwise,
-the B<padding> must be >1 or <=16384.
+Controls use of TLSv1.3 record layer padding.  B<padding> is a string of the
+form "number[,number]" where the (required) first number is the padding block
+size (in octets) for application data, and the optional second number is the
+padding block size for handshake and alert messages.  If the optional second
+number is omitted, the same padding will be applied to all messages.
+
+Padding attempts to pad TLSv1.3 records so that they are a multiple of the set
+length on send. A value of 0 or 1 turns off padding as relevant. Otherwise, the
+values must be >1 or <=16384.
 
 =item B<-debug_broken_protocol>
 
@@ -359,9 +365,15 @@ operations are permitted.
 
 =item B<RecordPadding>
 
-Attempts to pad TLSv1.3 records so that they are a multiple of B<value> in
-length on send. A B<value> of 0 or 1 turns off padding. Otherwise, the
-B<value> must be >1 or <=16384.
+Controls use of TLSv1.3 record layer padding.  B<value> is a string of the form
+"number[,number]" where the (required) first number is the padding block size
+(in octets) for application data, and the optional second number is the padding
+block size for handshake and alert messages.  If the optional second number is
+omitted, the same padding will be applied to all messages.
+
+Padding attempts to pad TLSv1.3 records so that they are a multiple of the set
+length on send. A value of 0 or 1 turns off padding as relevant. Otherwise, the
+values must be >1 or <=16384.
 
 =item B<SignatureAlgorithms>
 
diff --git a/doc/man3/SSL_CTX_set_record_padding_callback.pod b/doc/man3/SSL_CTX_set_record_padding_callback.pod
index e91f903b01..6d70ebd900 100644
--- a/doc/man3/SSL_CTX_set_record_padding_callback.pod
+++ b/doc/man3/SSL_CTX_set_record_padding_callback.pod
@@ -9,7 +9,9 @@ SSL_set_record_padding_callback_arg,
 SSL_CTX_get_record_padding_callback_arg,
 SSL_get_record_padding_callback_arg,
 SSL_CTX_set_block_padding,
-SSL_set_block_padding - install callback to specify TLS 1.3 record padding
+SSL_CTX_set_block_padding_ex,
+SSL_set_block_padding,
+SSL_set_block_padding_ex - install callback to specify TLS 1.3 record padding
 
 =head1 SYNOPSIS
 
@@ -26,6 +28,8 @@ SSL_set_block_padding - install callback to specify TLS 1.3 record padding
 
  int SSL_CTX_set_block_padding(SSL_CTX *ctx, size_t block_size);
  int SSL_set_block_padding(SSL *ssl, size_t block_size);
+ int SSL_CTX_set_block_padding_ex(SSL_CTX *ctx, size_t app_block_size, size_t hs_block_size);
+ int SSL_set_block_padding_ex(SSL *ssl, size_t app_block_size, size_t hs_block_size);
 
 =head1 DESCRIPTION
 
@@ -46,6 +50,10 @@ SSL_CTX_set_block_padding() and SSL_set_block_padding() pads the record to a mul
 of the B<block_size>. A B<block_size> of 0 or 1 disables block padding. The limit of
 B<block_size> is SSL3_RT_MAX_PLAIN_LENGTH.
 
+SSL_CTX_set_block_padding_ex() and SSL_set_block_padding_ex() do similarly but
+allow the caller to separately specify the padding block size to be applied to
+handshake and application data messages.
+
 The callback is invoked for every record before encryption.
 The B<type> parameter is the TLS record type that is being processed; may be
 one of SSL3_RT_APPLICATION_DATA, SSL3_RT_HANDSHAKE, or SSL3_RT_ALERT.
diff --git a/include/openssl/ssl.h.in b/include/openssl/ssl.h.in
index c4a40c95c3..4bab2ac767 100644
--- a/include/openssl/ssl.h.in
+++ b/include/openssl/ssl.h.in
@@ -2263,6 +2263,8 @@ void SSL_CTX_set_record_padding_callback(SSL_CTX *ctx,
 void SSL_CTX_set_record_padding_callback_arg(SSL_CTX *ctx, void *arg);
 void *SSL_CTX_get_record_padding_callback_arg(const SSL_CTX *ctx);
 int SSL_CTX_set_block_padding(SSL_CTX *ctx, size_t block_size);
+int SSL_CTX_set_block_padding_ex(SSL_CTX *ctx, size_t app_block_size,
+                                 size_t hs_block_size);
 
 int SSL_set_record_padding_callback(SSL *ssl,
                                     size_t (*cb) (SSL *ssl, int type,
@@ -2270,7 +2272,8 @@ int SSL_set_record_padding_callback(SSL *ssl,
 void SSL_set_record_padding_callback_arg(SSL *ssl, void *arg);
 void *SSL_get_record_padding_callback_arg(const SSL *ssl);
 int SSL_set_block_padding(SSL *ssl, size_t block_size);
-
+int SSL_set_block_padding_ex(SSL *ssl, size_t app_block_size,
+                             size_t hs_block_size);
 int SSL_set_num_tickets(SSL *s, size_t num_tickets);
 size_t SSL_get_num_tickets(const SSL *s);
 int SSL_CTX_set_num_tickets(SSL_CTX *ctx, size_t num_tickets);
diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h
index 5a3d010503..e08737eb76 100644
--- a/ssl/record/methods/recmethod_local.h
+++ b/ssl/record/methods/recmethod_local.h
@@ -324,6 +324,7 @@ struct ossl_record_layer_st
 
     /* TLSv1.3 record padding */
     size_t block_padding;
+    size_t hs_padding;
 
     /* Only used by SSLv3 */
     unsigned char mac_secret[EVP_MAX_MD_SIZE];
diff --git a/ssl/record/methods/tls13_meth.c b/ssl/record/methods/tls13_meth.c
index afae14ad22..706a0b8623 100644
--- a/ssl/record/methods/tls13_meth.c
+++ b/ssl/record/methods/tls13_meth.c
@@ -335,22 +335,51 @@ static int tls13_add_record_padding(OSSL_RECORD_LAYER *rl,
         size_t padding = 0;
         size_t max_padding = rl->max_frag_len - rlen;
 
+        /*
+         * We might want to change the "else if" below so that
+         * library-added padding can still happen even if there
+         * is an application-layer callback. The reason being
+         * the application may not be aware that the effectivness
+         * of ECH could be damaged if the callback e.g. only
+         * padded application data. However, doing so would be
+         * a change that could break some application that has
+         * a client and server that both know what padding they
+         * like, and that dislike any other padding. That'd need
+         * one of those to have been updated though so the 
+         * probability may be low enough that we could change
+         * the "else if" below to just an "if" and pick the
+         * larger of the library and callback's idea of padding.
+         * (Still subject to max_padding though.)
+         */
         if (rl->padding != NULL) {
             padding = rl->padding(rl->cbarg, thistempl->type, rlen);
-        } else if (rl->block_padding > 0) {
-            size_t mask = rl->block_padding - 1;
-            size_t remainder;
-
-            /* optimize for power of 2 */
-            if ((rl->block_padding & mask) == 0)
-                remainder = rlen & mask;
-            else
-                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 = rl->block_padding - remainder;
+        } else if (rl->block_padding > 0 || rl->hs_padding > 0) {
+            size_t mask, bp = 0, remainder;
+
+            /*
+             * pad handshake or alert messages based on |hs_padding|
+             * but application data based on |block_padding|
+             */
+            if (thistempl->type == SSL3_RT_HANDSHAKE && rl->hs_padding > 0)
+                bp = rl->hs_padding;
+            else if (thistempl->type == SSL3_RT_ALERT && rl->hs_padding > 0)
+                bp = rl->hs_padding;
+            else if (thistempl->type == SSL3_RT_APPLICATION_DATA
+                     && rl->block_padding > 0)
+                bp = rl->block_padding;
+            if (bp > 0) {
+                mask = bp - 1;
+                /* optimize for power of 2 */
+                if ((bp & mask) == 0)
+                    remainder = rlen & mask;
+                else
+                    remainder = rlen % bp;
+                /* don't want to add a block of padding if we don't have to */
+                if (remainder == 0)
+                    padding = 0;
+                else
+                    padding = bp - remainder;
+            }
         }
         if (padding > 0) {
             /* do not allow the record to exceed max plaintext length */
diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
index b09991cafb..0d92bdce9b 100644
--- a/ssl/record/methods/tls_common.c
+++ b/ssl/record/methods/tls_common.c
@@ -1218,6 +1218,12 @@ int tls_set_options(OSSL_RECORD_LAYER *rl, const OSSL_PARAM *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_HS_PADDING);
+        if (p != NULL && !OSSL_PARAM_get_size_t(p, &rl->hs_padding)) {
+            ERR_raise(ERR_LIB_SSL, SSL_R_FAILED_TO_GET_PARAMETER);
+            return 0;
+        }
     }
 
     if (rl->level == OSSL_RECORD_PROTECTION_LEVEL_APPLICATION) {
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index e61861d9fd..14db7dab2c 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1288,6 +1288,8 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
     } else {
         *opts++ = OSSL_PARAM_construct_size_t(OSSL_LIBSSL_RECORD_LAYER_PARAM_BLOCK_PADDING,
                                               &s->rlayer.block_padding);
+        *opts++ = OSSL_PARAM_construct_size_t(OSSL_LIBSSL_RECORD_LAYER_PARAM_HS_PADDING,
+                                              &s->rlayer.hs_padding);
     }
     *opts = OSSL_PARAM_construct_end();
 
diff --git a/ssl/record/record.h b/ssl/record/record.h
index 9a076a1fb8..13f09fda8c 100644
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -113,6 +113,7 @@ typedef struct record_layer_st {
     size_t (*record_padding_cb)(SSL *s, int type, size_t len, void *arg);
     void *record_padding_arg;
     size_t block_padding;
+    size_t hs_padding;
 
     /* How many records we have read from the record layer */
     size_t num_recs;
diff --git a/ssl/ssl_conf.c b/ssl/ssl_conf.c
index 77de00542b..0deae1604f 100644
--- a/ssl/ssl_conf.c
+++ b/ssl/ssl_conf.c
@@ -649,20 +649,44 @@ static int cmd_DHParameters(SSL_CONF_CTX *cctx, const char *value)
     return rv > 0;
 }
 
+/*
+ * |value| input is "<number[,number]>"
+ * where the first number is the padding block size for
+ * application data, and the optional second is the
+ * padding block size for handshake messages
+ */
 static int cmd_RecordPadding(SSL_CONF_CTX *cctx, const char *value)
 {
     int rv = 0;
-    int block_size = atoi(value);
+    size_t block_padding = 0, hs_padding = 0;
+    char *commap = NULL, *copy = NULL;
 
+    copy = OPENSSL_strdup(value);
+    if (copy == NULL)
+        return 0;
+    commap = strstr(copy, ",");
+    if (commap != NULL) {
+        *commap = '\0';
+        if (*(commap + 1) == '\0') {
+            OPENSSL_free(copy);
+            return 0;
+        }
+        hs_padding = (size_t) atoi(commap + 1);
+    }
+    block_padding = (size_t) atoi(copy);
+    if (commap == NULL)
+        hs_padding = block_padding;
+    OPENSSL_free(copy);
     /*
-     * All we care about is a non-negative value,
+     * All we care about are non-negative values,
      * the setters check the range
      */
-    if (block_size >= 0) {
+    if (block_padding >= 0 || hs_padding >= 0) {
         if (cctx->ctx)
-            rv = SSL_CTX_set_block_padding(cctx->ctx, block_size);
+            rv = SSL_CTX_set_block_padding_ex(cctx->ctx, block_padding,
+                                              hs_padding);
         if (cctx->ssl)
-            rv = SSL_set_block_padding(cctx->ssl, block_size);
+            rv = SSL_set_block_padding_ex(cctx->ssl, block_padding, hs_padding);
     }
     return rv;
 }
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 589ed2c64d..3b82673957 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -785,6 +785,7 @@ SSL *ossl_ssl_connection_new_int(SSL_CTX *ctx, const SSL_METHOD *method)
     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->rlayer.hs_padding = ctx->hs_padding;
     s->sid_ctx_length = ctx->sid_ctx_length;
     if (!ossl_assert(s->sid_ctx_length <= sizeof(s->sid_ctx)))
         goto err;
@@ -5713,21 +5714,35 @@ void *SSL_CTX_get_record_padding_callback_arg(const SSL_CTX *ctx)
     return ctx->record_padding_arg;
 }
 
-int SSL_CTX_set_block_padding(SSL_CTX *ctx, size_t block_size)
+int SSL_CTX_set_block_padding_ex(SSL_CTX *ctx, size_t app_block_size,
+                                 size_t hs_block_size)
 {
-    if (IS_QUIC_CTX(ctx) && block_size > 1)
+    if (IS_QUIC_CTX(ctx) && (app_block_size > 1 || hs_block_size > 1))
         return 0;
 
     /* block size of 0 or 1 is basically no padding */
-    if (block_size == 1)
+    if (app_block_size == 1) {
         ctx->block_padding = 0;
-    else if (block_size <= SSL3_RT_MAX_PLAIN_LENGTH)
-        ctx->block_padding = block_size;
-    else
+    } else if (app_block_size <= SSL3_RT_MAX_PLAIN_LENGTH) {
+        ctx->block_padding = app_block_size;
+    } else {
+        return 0;
+    }
+    if (hs_block_size == 1) {
+        ctx->hs_padding = 0;
+    } else if (hs_block_size <= SSL3_RT_MAX_PLAIN_LENGTH) {
+        ctx->hs_padding = hs_block_size;
+    } else {
         return 0;
+    }
     return 1;
 }
 
+int SSL_CTX_set_block_padding(SSL_CTX *ctx, size_t block_size)
+{
+    return SSL_CTX_set_block_padding_ex(ctx, block_size, block_size);
+}
+
 int SSL_set_record_padding_callback(SSL *ssl,
                                      size_t (*cb) (SSL *ssl, int type,
                                                    size_t len, void *arg))
@@ -5766,23 +5781,39 @@ void *SSL_get_record_padding_callback_arg(const SSL *ssl)
     return sc->rlayer.record_padding_arg;
 }
 
-int SSL_set_block_padding(SSL *ssl, size_t block_size)
+int SSL_set_block_padding_ex(SSL *ssl, size_t app_block_size,
+                             size_t hs_block_size)
 {
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(ssl);
 
-    if (sc == NULL || (IS_QUIC(ssl) && block_size > 1))
+    if (sc == NULL
+        || (IS_QUIC(ssl)
+            && (app_block_size > 1 || hs_block_size > 1)))
         return 0;
 
     /* block size of 0 or 1 is basically no padding */
-    if (block_size == 1)
+    if (app_block_size == 1) {
         sc->rlayer.block_padding = 0;
-    else if (block_size <= SSL3_RT_MAX_PLAIN_LENGTH)
-        sc->rlayer.block_padding = block_size;
-    else
+    } else if (app_block_size <= SSL3_RT_MAX_PLAIN_LENGTH) {
+        sc->rlayer.block_padding = app_block_size;
+    } else {
+        return 0;
+    }
+    if (hs_block_size == 1) {
+        sc->rlayer.hs_padding = 0;
+    } else if (hs_block_size <= SSL3_RT_MAX_PLAIN_LENGTH) {
+        sc->rlayer.hs_padding = hs_block_size;
+    } else {
         return 0;
+    }
     return 1;
 }
 
+int SSL_set_block_padding(SSL *ssl, size_t block_size)
+{
+    return SSL_set_block_padding_ex(ssl, block_size, block_size);
+}
+
 int SSL_set_num_tickets(SSL *s, size_t num_tickets)
 {
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);
diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index daab329133..d76a014cab 100644
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -1126,6 +1126,7 @@ struct ssl_ctx_st {
     size_t (*record_padding_cb)(SSL *s, int type, size_t len, void *arg);
     void *record_padding_arg;
     size_t block_padding;
+    size_t hs_padding;
 
     /* Session ticket appdata */
     SSL_CTX_generate_session_ticket_fn generate_ticket_cb;
diff --git a/test/sslapitest.c b/test/sslapitest.c
index 7ad12f2971..4cd469174d 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -11104,6 +11104,8 @@ static size_t record_pad_cb(SSL *s, int type, size_t len, void *arg)
  * Test 1: Record padding callback on the SSL
  * Test 2: Record block padding on the SSL_CTX
  * Test 3: Record block padding on the SSL
+ * Test 4: Extended record block padding on the SSL_CTX
+ * Test 5: Extended record block padding on the SSL
  */
 static int test_tls13_record_padding(int idx)
 {
@@ -11133,6 +11135,10 @@ static int test_tls13_record_padding(int idx)
             goto end;
         if (!TEST_true(SSL_CTX_set_block_padding(cctx, 512)))
             goto end;
+    } else if (idx == 4) {
+        /* pad only handshake/alert messages */
+        if (!TEST_true(SSL_CTX_set_block_padding_ex(cctx, 0, 512)))
+            goto end;
     }
 
     if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
@@ -11151,6 +11157,16 @@ static int test_tls13_record_padding(int idx)
             goto end;
         if (!TEST_true(SSL_set_block_padding(clientssl, 512)))
             goto end;
+    } else if (idx == 5) {
+        /* Exceeding the max plain length should fail */
+        if (!TEST_false(SSL_set_block_padding_ex(clientssl, 0,
+                                                 SSL3_RT_MAX_PLAIN_LENGTH + 1)))
+            goto end;
+        /* pad server and client handshake only */
+        if (!TEST_true(SSL_set_block_padding_ex(clientssl, 0, 512)))
+            goto end;
+        if (!TEST_true(SSL_set_block_padding_ex(serverssl, 0, 512)))
+            goto end;
     }
 
     if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE)))
@@ -12637,7 +12653,7 @@ int setup_tests(void)
     ADD_TEST(test_load_dhfile);
 #ifndef OSSL_NO_USABLE_TLS1_3
     ADD_TEST(test_read_ahead_key_change);
-    ADD_ALL_TESTS(test_tls13_record_padding, 4);
+    ADD_ALL_TESTS(test_tls13_record_padding, 6);
 #endif
 #if !defined(OPENSSL_NO_TLS1_2) && !defined(OSSL_NO_USABLE_TLS1_3)
     ADD_ALL_TESTS(test_serverinfo_custom, 4);
diff --git a/util/libssl.num b/util/libssl.num
index e232d3efd4..cd2c7f06a1 100644
--- a/util/libssl.num
+++ b/util/libssl.num
@@ -584,3 +584,5 @@ SSL_poll                                584	3_3_0	EXIST::FUNCTION:
 SSL_SESSION_get_time_ex                 585	3_3_0	EXIST::FUNCTION:
 SSL_SESSION_set_time_ex                 586	3_3_0	EXIST::FUNCTION:
 SSL_CTX_flush_sessions_ex               587	3_4_0	EXIST::FUNCTION:
+SSL_CTX_set_block_padding_ex            ?	3_4_0	EXIST::FUNCTION:
+SSL_set_block_padding_ex                ?	3_4_0	EXIST::FUNCTION:
diff --git a/util/perl/OpenSSL/paramnames.pm b/util/perl/OpenSSL/paramnames.pm
index 204a8bb213..6e5c027a22 100644
--- a/util/perl/OpenSSL/paramnames.pm
+++ b/util/perl/OpenSSL/paramnames.pm
@@ -506,6 +506,7 @@ my %params = (
     'LIBSSL_RECORD_LAYER_PARAM_MAX_FRAG_LEN' =>   "max_frag_len",
     'LIBSSL_RECORD_LAYER_PARAM_MAX_EARLY_DATA' => "max_early_data",
     'LIBSSL_RECORD_LAYER_PARAM_BLOCK_PADDING' =>  "block_padding",
+    'LIBSSL_RECORD_LAYER_PARAM_HS_PADDING' =>     "hs_padding",
 );
 
 # Generate string based macros for public consumption