Add a test for a server that doesn't provide transport params

Check that we fail if the server has failed to provide transport params.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20030)

diff --git a/include/internal/quic_tserver.h b/include/internal/quic_tserver.h
index 5cf0804a2dd100654d76a9122565f8558e0143b5..9d7fab5d12621ef914f44fd6ef8fd2d0035fdb38 100644 (file)
--- a/include/internal/quic_tserver.h
+++ b/include/internal/quic_tserver.h
@@ -57,8 +57,10 @@ int ossl_quic_tserver_set_handshake_mutator(QUIC_TSERVER *srv,
 /* Advances the state machine. */
 int ossl_quic_tserver_tick(QUIC_TSERVER *srv);
 
-/* Returns 1 if we have a (non-terminated) client. */
-int ossl_quic_tserver_is_connected(QUIC_TSERVER *srv);
+/*
+ * Returns 1 if we have finished the TLS handshake
+ */
+int ossl_quic_tserver_is_handshake_complete(QUIC_TSERVER *srv);
 
 /* Returns 1 if the server is in any terminating or terminated state */
 int ossl_quic_tserver_is_term_any(QUIC_TSERVER *srv,

diff --git a/ssl/quic/quic_tls.c b/ssl/quic/quic_tls.c
index 5ce71a2e4fdad0bd359796aa10a73ec7d2f88af7..088a59f28ab4c3f6bcda2ad347a4ba95a4c6bf8c 100644 (file)
--- a/ssl/quic/quic_tls.c
+++ b/ssl/quic/quic_tls.c
@@ -694,10 +694,15 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls)
          */
         SSL_set_bio(qtls->args.s, nullbio, nullbio);
 
-        if (qtls->args.is_server)
+        if (qtls->args.is_server) {
             SSL_set_accept_state(qtls->args.s);
-        else
+            if (!SSL_set_num_tickets(qtls->args.s, 0)) {
+                qtls->inerror = 1;
+                return 0;
+            }
+        } else {
             SSL_set_connect_state(qtls->args.s);
+        }
 
         qtls->configured = 1;
     }

diff --git a/ssl/quic/quic_tserver.c b/ssl/quic/quic_tserver.c
index 2a6049b1349e58a477397cd37568d91b3250bfee..444b59b746296b4ccf3a0ca9fcfe830de87e8d71 100644 (file)
--- a/ssl/quic/quic_tserver.c
+++ b/ssl/quic/quic_tserver.c
@@ -153,11 +153,6 @@ int ossl_quic_tserver_tick(QUIC_TSERVER *srv)
     return 1;
 }
 
-int ossl_quic_tserver_is_connected(QUIC_TSERVER *srv)
-{
-    return ossl_quic_channel_is_active(srv->ch);
-}
-
 /* Returns 1 if the server is in any terminating or terminated state */
 int ossl_quic_tserver_is_term_any(QUIC_TSERVER *srv,
                                   QUIC_TERMINATE_CAUSE *cause)
@@ -172,6 +167,11 @@ int ossl_quic_tserver_is_terminated(QUIC_TSERVER *srv,
     return ossl_quic_channel_is_terminated(srv->ch, cause);
 }
 
+int ossl_quic_tserver_is_handshake_complete(QUIC_TSERVER *srv)
+{
+    return ossl_quic_channel_is_handshake_complete(srv->ch);
+}
+
 int ossl_quic_tserver_read(QUIC_TSERVER *srv,
                            unsigned char *buf,
                            size_t buf_len,

diff --git a/test/helpers/quictestlib.c b/test/helpers/quictestlib.c
index 871a0e208fd20f2864ee975b07b1045528f0dec9..b9c437ba57eb3cd47b8a92317e819789836ce114 100644 (file)
--- a/test/helpers/quictestlib.c
+++ b/test/helpers/quictestlib.c
@@ -12,6 +12,7 @@
 #include "../testutil.h"
 #include "internal/quic_wire_pkt.h"
 #include "internal/quic_record_tx.h"
+#include "internal/quic_error.h"
 #include "internal/packet.h"
 
 #define GROWTH_ALLOWANCE 1024
@@ -156,13 +157,13 @@ int qtest_create_quic_connection(QUIC_TSERVER *qtserv, SSL *clientssl)
          * the communications and don't expect network delays. This shouldn't
          * be done in a real application.
          */
-        if (!clienterr)
+        if (!clienterr && retc <= 0)
             SSL_tick(clientssl);
-        if (!servererr) {
+        if (!servererr && rets <= 0) {
             ossl_quic_tserver_tick(qtserv);
             servererr = ossl_quic_tserver_is_term_any(qtserv, NULL);
-            if (!servererr && !rets)
-                rets = ossl_quic_tserver_is_connected(qtserv);
+            if (!servererr)
+                rets = ossl_quic_tserver_is_handshake_complete(qtserv);
         }
 
         if (clienterr && servererr)
@@ -172,13 +173,32 @@ int qtest_create_quic_connection(QUIC_TSERVER *qtserv, SSL *clientssl)
             TEST_info("No progress made");
             goto err;
         }
-    } while (retc <=0 || rets <= 0);
+    } while ((retc <= 0 && !clienterr) || (rets <= 0 && !servererr));
 
-    ret = 1;
+    if (!clienterr && !servererr)
+        ret = 1;
  err:
     return ret;
 }
 
+int qtest_check_server_protocol_err(QUIC_TSERVER *qtserv)
+{
+    QUIC_TERMINATE_CAUSE cause;
+
+    ossl_quic_tserver_tick(qtserv);
+
+    /*
+     * Check that the server has received the protocol violation error
+     * connection close from the client
+     */
+    if (!TEST_true(ossl_quic_tserver_is_term_any(qtserv, &cause))
+            || !TEST_true(cause.remote)
+            || !TEST_uint64_t_eq(cause.error_code, QUIC_ERR_PROTOCOL_VIOLATION))
+        return 0;
+
+    return 1;
+}
+
 void ossl_quic_fault_free(OSSL_QUIC_FAULT *fault)
 {
     if (fault == NULL)
@@ -438,12 +458,13 @@ int ossl_quic_fault_resize_message(OSSL_QUIC_FAULT *fault, size_t newlen)
 
 int ossl_quic_fault_delete_extension(OSSL_QUIC_FAULT *fault,
                                      unsigned int exttype, unsigned char *ext,
-                                     size_t *extlen, size_t *msglen)
+                                     size_t *extlen)
 {
     PACKET pkt, sub, subext;
     unsigned int type;
     const unsigned char *start, *end;
     size_t newlen;
+    size_t msglen = fault->handbuflen;
 
     if (!PACKET_buf_init(&pkt, ext, *extlen))
         return 0;
@@ -455,7 +476,7 @@ int ossl_quic_fault_delete_extension(OSSL_QUIC_FAULT *fault,
     do {
         start = PACKET_data(&sub);
         if (!PACKET_get_net_2(&sub, &type)
-                || !PACKET_as_length_prefixed_2(&sub, &subext))
+                || !PACKET_get_length_prefixed_2(&sub, &subext))
             return 0;
     } while (type != exttype);
 
@@ -489,8 +510,10 @@ int ossl_quic_fault_delete_extension(OSSL_QUIC_FAULT *fault,
     *extlen = newlen + 2;
 
     /* We can now resize the message */
-    *msglen -= (end - start);
-    if (!ossl_quic_fault_resize_message(fault, *msglen))
+    if ((size_t)(end - start) + SSL3_HM_HEADER_LENGTH > msglen)
+        return 0; /* Should not happen */
+    msglen -= (end - start) + SSL3_HM_HEADER_LENGTH;
+    if (!ossl_quic_fault_resize_message(fault, msglen))
         return 0;
 
     return 1;

diff --git a/test/helpers/quictestlib.h b/test/helpers/quictestlib.h
index 430d4f71c30eb03770a37f67a1a110b97eceedc5..d5fe58900a675df0bea5c08cdda396e6072ececc 100644 (file)
--- a/test/helpers/quictestlib.h
+++ b/test/helpers/quictestlib.h
@@ -22,6 +22,8 @@ int qtest_create_quic_objects(SSL_CTX *clientctx, char *certfile, char *keyfile,
                               OSSL_QUIC_FAULT **fault);
 int qtest_create_quic_connection(QUIC_TSERVER *qtserv, SSL *clientssl);
 
+int qtest_check_server_protocol_err(QUIC_TSERVER *qtserv);
+
 void ossl_quic_fault_free(OSSL_QUIC_FAULT *fault);
 
 typedef int (*ossl_quic_fault_on_packet_plain_cb)(OSSL_QUIC_FAULT *fault,
@@ -81,11 +83,8 @@ int ossl_quic_fault_resize_message(OSSL_QUIC_FAULT *fault, size_t newlen);
  * Delete an extension from an extension block. |exttype| is the type of the
  * extension to be deleted. |ext| points to the extension block. On entry
  * |*extlen| contains the length of the extension block. It is updated with the
- * new length on exit. On entry |*msglen| is the length of the handshake message
- * (without the header). On exit it is updated with the new message length.
- * ossl_quic_fault_resize_handshake() is called automatically so there is no
- * need to call it explicitly.
+ * new length on exit.
  */
 int ossl_quic_fault_delete_extension(OSSL_QUIC_FAULT *fault,
                                      unsigned int exttype, unsigned char *ext,
-                                     size_t *extlen, size_t *msglen);
+                                     size_t *extlen);

diff --git a/test/quicfaultstest.c b/test/quicfaultstest.c
index 730851112a443c34501cf64488d2c4f0bff1bf00..7def61c4d18e21850c275598e1c0f4c9cab61ee3 100644 (file)
--- a/test/quicfaultstest.c
+++ b/test/quicfaultstest.c
@@ -117,7 +117,6 @@ static int test_unknown_frame(void)
     unsigned char buf[80];
     size_t byteswritten;
     OSSL_QUIC_FAULT *fault = NULL;
-    QUIC_TERMINATE_CAUSE cause;
 
     if (!TEST_ptr(cctx))
         goto err;
@@ -169,17 +168,62 @@ static int test_unknown_frame(void)
         goto err;
 #endif
 
-    ERR_clear_error();
+    if (!TEST_true(qtest_check_server_protocol_err(qtserv)))
+        goto err;
 
-    ossl_quic_tserver_tick(qtserv);
+    testresult = 1;
+ err:
+    ossl_quic_fault_free(fault);
+    SSL_free(cssl);
+    ossl_quic_tserver_free(qtserv);
+    SSL_CTX_free(cctx);
+    return testresult;
+}
+
+/*
+ * Test that a server that fails to provide transport params cannot be
+ * connected to.
+ */
+static int drop_transport_params_cb(OSSL_QUIC_FAULT *fault,
+                                    OSSL_QF_ENCRYPTED_EXTENSIONS *ee,
+                                    size_t eelen, void *encextcbarg)
+{
+    if (!ossl_quic_fault_delete_extension(fault,
+                                          TLSEXT_TYPE_quic_transport_parameters,
+                                          ee->extensions, &ee->extensionslen))
+        return 0;
+
+    return 1;
+}
+
+static int test_no_transport_params(void)
+{
+    int testresult = 0;
+    SSL_CTX *cctx = SSL_CTX_new(OSSL_QUIC_client_method());
+    QUIC_TSERVER *qtserv = NULL;
+    SSL *cssl = NULL;
+    OSSL_QUIC_FAULT *fault = NULL;
+
+    if (!TEST_ptr(cctx))
+        goto err;
+
+    if (!TEST_true(qtest_create_quic_objects(cctx, cert, privkey, &qtserv,
+                                             &cssl, &fault)))
+        goto err;
+
+    if (!TEST_true(ossl_quic_fault_set_hand_enc_ext_listener(fault,
+                                                             drop_transport_params_cb,
+                                                             NULL)))
+        goto err;
 
     /*
-     * Check that the server has received the protocol violation error
-     * connection close from the client
+     * We expect the connection to fail because the server failed to provide
+     * transport parameters
      */
-    if (!TEST_true(ossl_quic_tserver_is_term_any(qtserv, &cause))
-            || !TEST_true(cause.remote)
-            || !TEST_uint64_t_eq(cause.error_code, QUIC_ERR_PROTOCOL_VIOLATION))
+    if (!TEST_false(qtest_create_quic_connection(qtserv, cssl)))
+        goto err;
+
+    if (!TEST_true(qtest_check_server_protocol_err(qtserv)))
         goto err;
 
     testresult = 1;
@@ -189,6 +233,7 @@ static int test_unknown_frame(void)
     ossl_quic_tserver_free(qtserv);
     SSL_CTX_free(cctx);
     return testresult;
+
 }
 
 OPT_TEST_DECLARE_USAGE("certsdir\n")
@@ -216,6 +261,7 @@ int setup_tests(void)
 
     ADD_TEST(test_basic);
     ADD_TEST(test_unknown_frame);
+    ADD_TEST(test_no_transport_params);
 
     return 1;