QUIC SSL: Prohibit readahead-related functions

Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #20061)
openssl · Jul 4, 2023 · d0638fd · d0638fd
1 parent 9280d26
commit d0638fd
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 11 deletions.
diff --git a/doc/man3/SSL_CTX_set_read_ahead.pod b/doc/man3/SSL_CTX_set_read_ahead.pod
@@ -34,6 +34,8 @@ SSL_CTX_get_read_ahead() and SSL_get_read_ahead() indicate whether reading
 ahead has been set or not.
 SSL_CTX_get_default_read_ahead() is identical to SSL_CTX_get_read_ahead().
 
+These functions cannot be used with QUIC SSL objects.
+
 =head1 NOTES
 
 These functions have no impact when used with DTLS. The return values for

diff --git a/doc/man3/SSL_CTX_set_record_padding_callback.pod b/doc/man3/SSL_CTX_set_record_padding_callback.pod
@@ -53,6 +53,8 @@ The B<len> parameter is the current plaintext length of the record before encryp
 The B<arg> parameter is the value set via SSL_CTX_set_record_padding_callback_arg()
 or SSL_set_record_padding_callback_arg().
 
+These functions cannot be used with QUIC SSL objects.
+
 =head1 RETURN VALUES
 
 The SSL_CTX_get_record_padding_callback_arg() and SSL_get_record_padding_callback_arg()

diff --git a/doc/man3/SSL_CTX_set_split_send_fragment.pod b/doc/man3/SSL_CTX_set_split_send_fragment.pod
@@ -123,6 +123,8 @@ It affects the client-side as only its side may initiate this extension use.
 SSL_SESSION_get_max_fragment_length() gets the maximum fragment length
 negotiated in B<session>.
 
+These functions cannot be used with QUIC SSL objects.
+
 =head1 RETURN VALUES
 
 All non-void functions return 1 on success and 0 on failure.

diff --git a/ssl/quic/quic_local.h b/ssl/quic/quic_local.h
@@ -217,7 +217,15 @@ int ossl_quic_trace(int write_p, int version, int content_type,
                     const void *buf, size_t msglen, SSL *ssl, void *arg);
 
 #  define OSSL_QUIC_ANY_VERSION     0x5155
-#  define IS_QUIC_METHOD(m)         ((m)->version == OSSL_QUIC_ANY_VERSION)
+#  ifndef OPENSSL_NO_QUIC
+#   define IS_QUIC_METHOD(m)        ((m)->version == OSSL_QUIC_ANY_VERSION)
+#   define IS_QUIC_SSL(s)           ((s) != NULL && \
+                                     ((s)->type == SSL_TYPE_QUIC_CONNECTION || \
+                                      (s)->type == SSL_TYPE_QUIC_STREAM))
+#  else
+#   define IS_QUIC_METHOD(m)        0
+#   define IS_QUIC_SSL(s)           0
+#  endif
 #  define IS_QUIC_CTX(ctx)          IS_QUIC_METHOD((ctx)->method)
 
 #  define QUIC_CONNECTION_FROM_SSL_int(ssl, c)   \

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
@@ -162,7 +162,7 @@ void SSL_set_default_read_buffer_len(SSL *s, size_t len)
 {
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);
 
-    if (sc == NULL)
+    if (sc == NULL || IS_QUIC_SSL(s))
         return;
     sc->rlayer.default_read_buf_len = len;
 }

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
@@ -787,9 +787,11 @@ SSL *ossl_ssl_connection_new_int(SSL_CTX *ctx, const SSL_METHOD *method)
     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->rlayer.record_padding_cb = ctx->record_padding_cb;
-    s->rlayer.record_padding_arg = ctx->record_padding_arg;
-    s->rlayer.block_padding = ctx->block_padding;
+    if (!IS_QUIC_CTX(ctx)) {
+        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;
@@ -803,7 +805,9 @@ SSL *ossl_ssl_connection_new_int(SSL_CTX *ctx, const SSL_METHOD *method)
     X509_VERIFY_PARAM_inherit(s->param, ctx->param);
     s->quiet_shutdown = ctx->quiet_shutdown;
 
-    s->ext.max_fragment_len_mode = ctx->ext.max_fragment_len_mode;
+    if (!IS_QUIC_SSL(ssl))
+        s->ext.max_fragment_len_mode = ctx->ext.max_fragment_len_mode;
+
     s->max_send_fragment = ctx->max_send_fragment;
     s->split_send_fragment = ctx->split_send_fragment;
     s->max_pipelines = ctx->max_pipelines;
@@ -1830,7 +1834,7 @@ 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)
+    if (sc == NULL || IS_QUIC_SSL(s))
         return;
 
     RECORD_LAYER_set_read_ahead(&sc->rlayer, yes);
@@ -1847,7 +1851,7 @@ int SSL_get_read_ahead(const SSL *s)
 {
     const SSL_CONNECTION *sc = SSL_CONNECTION_FROM_CONST_SSL(s);
 
-    if (sc == NULL)
+    if (sc == NULL || IS_QUIC_SSL(s))
         return 0;
 
     return RECORD_LAYER_get_read_ahead(&sc->rlayer);
@@ -2884,8 +2888,12 @@ long SSL_ctrl(SSL *s, int cmd, long larg, void *parg)
 
     switch (cmd) {
     case SSL_CTRL_GET_READ_AHEAD:
+        if (IS_QUIC_SSL(s))
+            return 0;
         return RECORD_LAYER_get_read_ahead(&sc->rlayer);
     case SSL_CTRL_SET_READ_AHEAD:
+        if (IS_QUIC_SSL(s))
+            return 0;
         l = RECORD_LAYER_get_read_ahead(&sc->rlayer);
         RECORD_LAYER_set_read_ahead(&sc->rlayer, larg);
         return l;
@@ -5641,7 +5649,7 @@ int SSL_set_record_padding_callback(SSL *ssl,
     BIO *b;
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(ssl);
 
-    if (sc == NULL)
+    if (sc == NULL || IS_QUIC_SSL(ssl))
         return 0;
 
     b = SSL_get_wbio(ssl);
@@ -5676,7 +5684,7 @@ int SSL_set_block_padding(SSL *ssl, size_t block_size)
 {
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(ssl);
 
-    if (sc == NULL)
+    if (sc == NULL || (IS_QUIC_SSL(ssl) && block_size > 1))
         return 0;
 
     /* block size of 0 or 1 is basically no padding */

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
@@ -3865,7 +3865,8 @@ int SSL_set_tlsext_max_fragment_length(SSL *ssl, uint8_t mode)
 {
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(ssl);
 
-    if (sc == NULL)
+    if (sc == NULL
+        || (IS_QUIC_SSL(ssl) && mode != TLSEXT_max_fragment_length_DISABLED))
         return 0;
 
     if (mode != TLSEXT_max_fragment_length_DISABLED

diff --git a/test/quicapitest.c b/test/quicapitest.c
@@ -451,6 +451,22 @@ static int test_quic_forbidden_options(void)
     if (!TEST_uint64_t_eq(SSL_get_options(ssl), 0))
         goto err;
 
+    /* Readahead */
+    SSL_set_read_ahead(ssl, 1);
+    if (!TEST_false(SSL_get_read_ahead(ssl)))
+        goto err;
+
+    /* Block padding */
+    if (!TEST_true(SSL_set_block_padding(ssl, 0))
+        || !TEST_true(SSL_set_block_padding(ssl, 1))
+        || !TEST_false(SSL_set_block_padding(ssl, 2)))
+        goto err;
+
+    /* Max fragment length */
+    if (!TEST_true(SSL_set_tlsext_max_fragment_length(ssl, TLSEXT_max_fragment_length_DISABLED))
+        || !TEST_false(SSL_set_tlsext_max_fragment_length(ssl, TLSEXT_max_fragment_length_512)))
+        goto err;
+
     testresult = 1;
 err:
     SSL_free(ssl);