Back off on generating noise in the event of a PING frame
authorMatt Caswell <matt@openssl.org>
Mon, 2 Oct 2023 10:47:08 +0000 (11:47 +0100)
committerPauli <pauli@openssl.org>
Tue, 3 Oct 2023 23:51:51 +0000 (10:51 +1100)
If either endpoint issues a PING frame while we are introducing noise
into the communication then there is a danger that the connection itself
will fail. We detect the PING and then back off on generating noise for a
short while. It should be sufficient to just ensure that the next datagram
does not get dropped for each endpoint.

Fixes #22199

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22243)

test/helpers/noisydgrambio.c
test/helpers/quictestlib.c
test/helpers/quictestlib.h
test/quicapitest.c

index 8b68726dd2b4c4fa1751c4588d43850fd4786195..33cf84c3c6c0a86f493ed54b2892015ecff9bd1f 100644 (file)
@@ -17,6 +17,7 @@ struct noisy_dgram_st {
     uint64_t this_dgram;
     BIO_MSG msg;
     uint64_t reinject_dgram;
+    int backoff;
 };
 
 static long noisy_dgram_ctrl(BIO *bio, int cmd, long num, void *ptr)
@@ -31,6 +32,16 @@ static long noisy_dgram_ctrl(BIO *bio, int cmd, long num, void *ptr)
     case BIO_CTRL_DUP:
         ret = 0L;
         break;
+    case BIO_CTRL_NOISE_BACK_OFF: {
+            struct noisy_dgram_st *data;
+
+            data = BIO_get_data(bio);
+            if (!TEST_ptr(data))
+                return 0;
+            data->backoff = 1;
+            ret = 1;
+            break;
+        }
     default:
         ret = BIO_ctrl(next, cmd, num, ptr);
         break;
@@ -195,6 +206,17 @@ static int noisy_dgram_recvmmsg(BIO *bio, BIO_MSG *msg, size_t stride,
         }
 
         get_noise(&reinject, &should_drop);
+        if (data->backoff) {
+            /*
+             * We might be asked to back off on introducing too much noise if
+             * there is a danger that the connection will fail. In that case
+             * we always ensure that the next datagram does not get dropped so
+             * that the connection always survives. After that we can resume
+             * with normal noise
+             */
+            should_drop = 0;
+            data->backoff = 0;
+        }
 
         /*
          * We ignore reinjection if a message is already waiting to be
index 514bb6422a79016fc97816223f495e1494a066e1..0ae05e5701e29a92f53758c48f9130b452570ca6 100644 (file)
 
 #define GROWTH_ALLOWANCE 1024
 
+struct noise_args_data_st {
+    BIO *cbio;
+    BIO *sbio;
+    BIO *tracebio;
+    int flags;
+};
+
 struct qtest_fault {
     QUIC_TSERVER *qtserv;
 
@@ -62,6 +69,7 @@ struct qtest_fault {
     BIO_MSG msg;
     /* Allocated size of msg data buffer */
     size_t msgalloc;
+    struct noise_args_data_st noiseargs;
 };
 
 static void packet_plain_finish(void *arg);
@@ -75,6 +83,41 @@ static OSSL_TIME fake_now_cb(void *arg)
     return fake_now;
 }
 
+static void noise_msg_callback(int write_p, int version, int content_type,
+                               const void *buf, size_t len, SSL *ssl,
+                               void *arg)
+{
+    struct noise_args_data_st *noiseargs = (struct noise_args_data_st *)arg;
+
+    if (content_type == SSL3_RT_QUIC_FRAME_FULL) {
+        PACKET pkt;
+        uint64_t frame_type;
+
+        if (!PACKET_buf_init(&pkt, buf, len))
+            return;
+
+        if (!ossl_quic_wire_peek_frame_header(&pkt, &frame_type, NULL))
+            return;
+
+        if (frame_type == OSSL_QUIC_FRAME_TYPE_PING) {
+            /*
+             * If either endpoint issues a ping frame then we are in danger
+             * of our noise being too much such that the connection itself
+             * fails. We back off on the noise for a bit to avoid that.
+             */
+            BIO_ctrl(noiseargs->cbio, BIO_CTRL_NOISE_BACK_OFF, 0, NULL);
+            BIO_ctrl(noiseargs->sbio, BIO_CTRL_NOISE_BACK_OFF, 0, NULL);
+        }
+    }
+
+#ifndef OPENSSL_NO_SSL_TRACE
+    if ((noiseargs->flags & QTEST_FLAG_CLIENT_TRACE) != 0
+            && !SSL_is_server(ssl))
+        SSL_trace(write_p, version, content_type, buf, len, ssl,
+                  noiseargs->tracebio);
+#endif
+}
+
 int qtest_create_quic_objects(OSSL_LIB_CTX *libctx, SSL_CTX *clientctx,
                               SSL_CTX *serverctx, char *certfile, char *keyfile,
                               int flags, QUIC_TSERVER **qtserv, SSL **cssl,
@@ -89,15 +132,18 @@ int qtest_create_quic_objects(OSSL_LIB_CTX *libctx, SSL_CTX *clientctx,
     BIO *tmpbio = NULL;
 
     *qtserv = NULL;
-    if (fault != NULL)
-        *fault = NULL;
-
     if (*cssl == NULL) {
         *cssl = SSL_new(clientctx);
         if (!TEST_ptr(*cssl))
             return 0;
     }
 
+    if (fault != NULL) {
+        *fault = OPENSSL_zalloc(sizeof(**fault));
+        if (*fault == NULL)
+            goto err;
+    }
+
 #ifndef OPENSSL_NO_SSL_TRACE
     if ((flags & QTEST_FLAG_CLIENT_TRACE) != 0) {
         tmpbio = BIO_new_fp(stdout, BIO_NOCLOSE);
@@ -170,7 +216,15 @@ int qtest_create_quic_objects(OSSL_LIB_CTX *libctx, SSL_CTX *clientctx,
     }
 
     if ((flags & QTEST_FLAG_NOISE) != 0) {
-        BIO *noisebio = BIO_new(bio_f_noisy_dgram_filter());
+        BIO *noisebio;
+
+        /*
+         * It is an error to not have a QTEST_FAULT object when introducing noise
+         */
+        if (!TEST_ptr(fault))
+            goto err;
+
+        noisebio = BIO_new(bio_f_noisy_dgram_filter());
 
         if (!TEST_ptr(noisebio))
             goto err;
@@ -181,6 +235,14 @@ int qtest_create_quic_objects(OSSL_LIB_CTX *libctx, SSL_CTX *clientctx,
         if (!TEST_ptr(noisebio))
             goto err;
         sbio = BIO_push(noisebio, sbio);
+
+        (*fault)->noiseargs.cbio = cbio;
+        (*fault)->noiseargs.sbio = sbio;
+        (*fault)->noiseargs.tracebio = tmpbio;
+        (*fault)->noiseargs.flags = flags;
+
+        SSL_set_msg_callback(*cssl, noise_msg_callback);
+        SSL_set_msg_callback_arg(*cssl, &(*fault)->noiseargs);
     }
 
     SSL_set_bio(*cssl, cbio, cbio);
@@ -192,12 +254,6 @@ int qtest_create_quic_objects(OSSL_LIB_CTX *libctx, SSL_CTX *clientctx,
     if (!TEST_true(SSL_set1_initial_peer_addr(*cssl, peeraddr)))
         goto err;
 
-    if (fault != NULL) {
-        *fault = OPENSSL_zalloc(sizeof(**fault));
-        if (*fault == NULL)
-            goto err;
-    }
-
     fisbio = BIO_new(qtest_get_bio_method());
     if (!TEST_ptr(fisbio))
         goto err;
@@ -237,6 +293,10 @@ int qtest_create_quic_objects(OSSL_LIB_CTX *libctx, SSL_CTX *clientctx,
     sbio = NULL;
     fisbio = NULL;
 
+    if ((flags & QTEST_FLAG_NOISE) != 0)
+        ossl_quic_tserver_set_msg_callback(*qtserv, noise_msg_callback,
+                                           &(*fault)->noiseargs);
+
     if (fault != NULL)
         (*fault)->qtserv = *qtserv;
 
index f090299b22798a1263913524d07cea8828859d63..d1ac350c244610c3c2ae5bc50fcb0f6cc5b7b68d 100644 (file)
@@ -245,6 +245,8 @@ int qtest_fault_resize_datagram(QTEST_FAULT *fault, size_t newlen);
 /* Copy a BIO_MSG */
 int bio_msg_copy(BIO_MSG *dst, BIO_MSG *src);
 
+#define BIO_CTRL_NOISE_BACK_OFF 1001
+
 /* BIO filter for simulating a noisy UDP socket */
 const BIO_METHOD *bio_f_noisy_dgram_filter(void);
 
index 273f1421e7926e83116425782e67a3972e5373ab..7739cbcb24b0ed4a6577c535f2bfc8b28b4c3c66 100644 (file)
@@ -1410,6 +1410,7 @@ static int test_noisy_dgram(int idx)
     size_t msglen = strlen(msg), written, readbytes, i, j;
     unsigned char buf[80];
     int flags = QTEST_FLAG_NOISE | QTEST_FLAG_FAKE_TIME;
+    QTEST_FAULT *fault = NULL;
 
     if (idx == 1)
         flags |= QTEST_FLAG_PACKET_SPLIT;
@@ -1418,7 +1419,7 @@ static int test_noisy_dgram(int idx)
             || !TEST_true(qtest_create_quic_objects(libctx, cctx, NULL, cert,
                                                     privkey, flags,
                                                     &qtserv,
-                                                    &clientquic, NULL, NULL)))
+                                                    &clientquic, &fault, NULL)))
         goto err;
 
     if (!TEST_true(qtest_create_quic_connection(qtserv, clientquic)))
@@ -1492,6 +1493,7 @@ static int test_noisy_dgram(int idx)
     SSL_free(stream[1]);
     SSL_free(clientquic);
     SSL_CTX_free(cctx);
+    qtest_fault_free(fault);
 
     return testresult;
 }