]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
QUIC: Prevent incoming oversize tokens
authorHugo Landau <hlandau@openssl.org>
Thu, 19 Oct 2023 08:27:11 +0000 (09:27 +0100)
committerMatt Caswell <matt@openssl.org>
Fri, 20 Oct 2023 15:31:40 +0000 (16:31 +0100)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22436)

include/internal/quic_txp.h
ssl/quic/quic_channel.c
ssl/quic/quic_txp.c
test/quic_multistream_test.c
test/quic_txp_test.c

index 64efedc27f385e8b754f04bc6987aba8ed227d6c..ae508f2393bd41bc63fd388ed3fd1deff020d706 100644 (file)
@@ -112,13 +112,15 @@ OSSL_TIME ossl_quic_tx_packetiser_get_deadline(OSSL_QUIC_TX_PACKETISER *txp);
 /*
  * Set the token used in Initial packets. The callback is called when the buffer
  * is no longer needed; for example, when the TXP is freed or when this function
- * is called again with a new buffer.
+ * is called again with a new buffer. Fails returning 0 if the token is too big
+ * to ever be reasonably encapsulated in an outgoing packet based on our current
+ * understanding of our PMTU.
  */
-void ossl_quic_tx_packetiser_set_initial_token(OSSL_QUIC_TX_PACKETISER *txp,
-                                               const unsigned char *token,
-                                               size_t token_len,
-                                               ossl_quic_initial_token_free_fn *free_cb,
-                                               void *free_cb_arg);
+int ossl_quic_tx_packetiser_set_initial_token(OSSL_QUIC_TX_PACKETISER *txp,
+                                              const unsigned char *token,
+                                              size_t token_len,
+                                              ossl_quic_initial_token_free_fn *free_cb,
+                                              void *free_cb_arg);
 
 /* Change the DCID the TXP uses to send outgoing packets. */
 int ossl_quic_tx_packetiser_set_cur_dcid(OSSL_QUIC_TX_PACKETISER *txp,
index 8e75eda539f44ec7c6c958ff758a1095fd48c519..9e5b84162234c469a75accf22f68b97f11d823a2 100644 (file)
@@ -2816,8 +2816,18 @@ static int ch_retry(QUIC_CHANNEL *ch,
     if ((buf = OPENSSL_memdup(retry_token, retry_token_len)) == NULL)
         return 0;
 
-    ossl_quic_tx_packetiser_set_initial_token(ch->txp, buf, retry_token_len,
-                                              free_token, NULL);
+    if (!ossl_quic_tx_packetiser_set_initial_token(ch->txp, buf,
+                                                   retry_token_len,
+                                                   free_token, NULL)) {
+        /*
+         * This may fail if the token we receive is too big for us to ever be
+         * able to transmit in an outgoing Initial packet.
+         */
+        ossl_quic_channel_raise_protocol_error(ch, QUIC_ERR_INVALID_TOKEN, 0,
+                                               "received oversize token");
+        OPENSSL_free(buf);
+        return 0;
+    }
 
     ch->retry_scid  = *retry_scid;
     ch->doing_retry = 1;
index 0f1e9b8f25618bde8ddf31cead956293d0211bb1..1045e79e0fc05471bd43fd4c13f2d41e6169f3e3 100644 (file)
@@ -510,12 +510,63 @@ void ossl_quic_tx_packetiser_free(OSSL_QUIC_TX_PACKETISER *txp)
     OPENSSL_free(txp);
 }
 
-void ossl_quic_tx_packetiser_set_initial_token(OSSL_QUIC_TX_PACKETISER *txp,
-                                               const unsigned char *token,
-                                               size_t token_len,
-                                               ossl_quic_initial_token_free_fn *free_cb,
-                                               void *free_cb_arg)
+/*
+ * Determine if an Initial packet token length is reasonable based on the
+ * current MDPL, returning 1 if it is OK.
+ *
+ * The real PMTU to the peer could differ from our (pessimistic) understanding
+ * of the PMTU, therefore it is possible we could receive an Initial token from
+ * a server in a Retry packet which is bigger than the MDPL. In this case it is
+ * impossible for us ever to make forward progress and we need to error out
+ * and fail the connection attempt.
+ *
+ * The specific boundary condition is complex: for example, after the size of
+ * the Initial token, there are the Initial packet header overheads and then
+ * encryption/AEAD tag overheads. After that, the minimum room for frame data in
+ * order to guarantee forward progress must be guaranteed. For example, a crypto
+ * stream needs to always be able to serialize at least one byte in a CRYPTO
+ * frame in order to make forward progress. Because the offset field of a CRYPTO
+ * frame uses a variable-length integer, the number of bytes needed to ensure
+ * this also varies.
+ *
+ * Rather than trying to get this boundary condition check actually right,
+ * require a reasonable amount of slack to avoid pathological behaviours. (After
+ * all, transmitting a CRYPTO stream one byte at a time is probably not
+ * desirable anyway.)
+ *
+ * We choose 160 bytes as the required margin, which is double the rough
+ * estimation of the minimum we would require to guarantee forward progress
+ * under worst case packet overheads.
+ */
+#define TXP_REQUIRED_TOKEN_MARGIN       160
+
+static int txp_check_token_len(size_t token_len, size_t mdpl)
+{
+    if (token_len == 0)
+        return 1;
+
+    if (token_len >= mdpl)
+        return 0;
+
+    if (TXP_REQUIRED_TOKEN_MARGIN >= mdpl)
+        /* (should not be possible because MDPL must be at least 1200) */
+        return 0;
+
+    if (token_len > mdpl - TXP_REQUIRED_TOKEN_MARGIN)
+        return 0;
+
+    return 1;
+}
+
+int ossl_quic_tx_packetiser_set_initial_token(OSSL_QUIC_TX_PACKETISER *txp,
+                                              const unsigned char *token,
+                                              size_t token_len,
+                                              ossl_quic_initial_token_free_fn *free_cb,
+                                              void *free_cb_arg)
 {
+    if (!txp_check_token_len(token_len, txp_get_mdpl(txp)))
+        return 0;
+
     if (txp->initial_token != NULL && txp->initial_token_free_cb != NULL)
         txp->initial_token_free_cb(txp->initial_token, txp->initial_token_len,
                                    txp->initial_token_free_cb_arg);
@@ -524,6 +575,7 @@ void ossl_quic_tx_packetiser_set_initial_token(OSSL_QUIC_TX_PACKETISER *txp,
     txp->initial_token_len          = token_len;
     txp->initial_token_free_cb      = free_cb;
     txp->initial_token_free_cb_arg  = free_cb_arg;
+    return 1;
 }
 
 int ossl_quic_tx_packetiser_set_cur_dcid(OSSL_QUIC_TX_PACKETISER *txp,
index e4663ece172f7564b68fdaf3781df9d578fae278..958dd30fc1033360957e2edd9ed6d4452c4fc815 100644 (file)
@@ -4902,6 +4902,7 @@ static const struct script_op script_76[] = {
     OP_END
 };
 
+/* 77. Ensure default stream popping operates correctly */
 static const struct script_op script_77[] = {
     OP_C_SET_ALPN           ("ossltest")
     OP_C_CONNECT_WAIT       ()
index 4682483acc3d10bbc56c3fc8358371d8dbab830d..423d28ddcbb4747b0f00b397340262a5e8ed8177 100644 (file)
@@ -1198,6 +1198,54 @@ static const struct script_op script_17[] = {
     OP_END
 };
 
+/* 18. Big Token Rejection */
+static const unsigned char big_token[1950];
+
+static int try_big_token(struct helper *h)
+{
+    size_t i;
+
+    /* Ensure big token is rejected */
+    if (!TEST_false(ossl_quic_tx_packetiser_set_initial_token(h->txp,
+                                                              big_token,
+                                                              sizeof(big_token),
+                                                              NULL,
+                                                              NULL)))
+        return 0;
+
+    /*
+     * Keep trying until we find an acceptable size, then make sure
+     * that works for generation
+     */
+    for (i = sizeof(big_token) - 1;; --i) {
+        if (!TEST_size_t_gt(i, 0))
+            return 0;
+
+        if (ossl_quic_tx_packetiser_set_initial_token(h->txp, big_token, i,
+                                                      NULL, NULL))
+            break;
+    }
+
+    return 1;
+}
+
+static const struct script_op script_18[] = {
+    OP_PROVIDE_SECRET(QUIC_ENC_LEVEL_INITIAL, QRL_SUITE_AES128GCM, secret_1)
+    OP_TXP_GENERATE_NONE()
+    OP_CHECK(try_big_token)
+    OP_TXP_GENERATE_NONE()
+    OP_CRYPTO_SEND(QUIC_PN_SPACE_INITIAL, crypto_1)
+    OP_TXP_GENERATE()
+    OP_RX_PKT()
+    OP_EXPECT_DGRAM_LEN(1200, 1200)
+    OP_NEXT_FRAME()
+    OP_EXPECT_FRAME(OSSL_QUIC_FRAME_TYPE_CRYPTO)
+    OP_EXPECT_NO_FRAME()
+    OP_RX_PKT_NONE()
+    OP_TXP_GENERATE_NONE()
+    OP_END
+};
+
 static const struct script_op *const scripts[] = {
     script_1,
     script_2,
@@ -1215,7 +1263,8 @@ static const struct script_op *const scripts[] = {
     script_14,
     script_15,
     script_16,
-    script_17
+    script_17,
+    script_18
 };
 
 static void skip_padding(struct helper *h)