From: Hugo Landau Date: Thu, 19 Oct 2023 08:27:11 +0000 (+0100) Subject: QUIC: Prevent incoming oversize tokens X-Git-Tag: openssl-3.2.0-beta1~59 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=461d41174b33e365677d21bf176d6959b15c2468;p=thirdparty%2Fopenssl.git QUIC: Prevent incoming oversize tokens Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/22436) --- diff --git a/include/internal/quic_txp.h b/include/internal/quic_txp.h index 64efedc27f3..ae508f2393b 100644 --- a/include/internal/quic_txp.h +++ b/include/internal/quic_txp.h @@ -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, diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c index 8e75eda539f..9e5b8416223 100644 --- a/ssl/quic/quic_channel.c +++ b/ssl/quic/quic_channel.c @@ -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; diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c index 0f1e9b8f256..1045e79e0fc 100644 --- a/ssl/quic/quic_txp.c +++ b/ssl/quic/quic_txp.c @@ -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, diff --git a/test/quic_multistream_test.c b/test/quic_multistream_test.c index e4663ece172..958dd30fc10 100644 --- a/test/quic_multistream_test.c +++ b/test/quic_multistream_test.c @@ -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 () diff --git a/test/quic_txp_test.c b/test/quic_txp_test.c index 4682483acc3..423d28ddcbb 100644 --- a/test/quic_txp_test.c +++ b/test/quic_txp_test.c @@ -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)