From ed75eb32f3712b80a47ad783d0082c66164c732f Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Wed, 26 Jul 2023 18:10:16 +0100 Subject: QUIC TEST: Test NEW_CONN_ID frames Fixes https://github.com/openssl/project/issues/86 Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/21565) --- include/internal/quic_channel.h | 5 ++ ssl/quic/quic_channel.c | 5 ++ ssl/quic/quic_txp.c | 21 ++++--- test/quic_multistream_test.c | 125 ++++++++++++++++++++++++++++++++++++++-- 4 files changed, 143 insertions(+), 13 deletions(-) diff --git a/include/internal/quic_channel.h b/include/internal/quic_channel.h index 0a033fa67e..e36acb4eeb 100644 --- a/include/internal/quic_channel.h +++ b/include/internal/quic_channel.h @@ -390,6 +390,11 @@ void ossl_quic_channel_set_inhibit_tick(QUIC_CHANNEL *ch, int inhibit); */ uint16_t ossl_quic_channel_get_diag_num_rx_ack(QUIC_CHANNEL *ch); +/* + * Diagnostic use only. Gets the current local CID. + */ +void ossl_quic_channel_get_diag_local_cid(QUIC_CHANNEL *ch, QUIC_CONN_ID *cid); + # endif #endif diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c index 52ec3d749e..6931f79d01 100644 --- a/ssl/quic/quic_channel.c +++ b/ssl/quic/quic_channel.c @@ -3337,3 +3337,8 @@ uint16_t ossl_quic_channel_get_diag_num_rx_ack(QUIC_CHANNEL *ch) { return ch->diag_num_rx_ack; } + +void ossl_quic_channel_get_diag_local_cid(QUIC_CHANNEL *ch, QUIC_CONN_ID *cid) +{ + *cid = ch->cur_local_cid; +} diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c index 1107a0778d..bc017478fe 100644 --- a/ssl/quic/quic_txp.c +++ b/ssl/quic/quic_txp.c @@ -2846,20 +2846,23 @@ static int txp_pkt_commit(OSSL_QUIC_TX_PACKETISER *txp, if (!ossl_quic_fifd_pkt_commit(&txp->fifd, tpkt)) return 0; - /* Send the packet. */ - if (!ossl_qtx_write_pkt(txp->args.qtx, &txpkt)) - return 0; - /* - * Post-Packet Generation Bookkeeping - * ================================== + * Transmission and Post-Packet Generation Bookkeeping + * =================================================== * - * No backing out anymore - we have sent the packet and need to record this - * fact. + * No backing out anymore - at this point the ACKM has recorded the packet + * as having been sent, so we need to increment our next PN counter, or + * the ACKM will complain when we try to record a duplicate packet with + * the same PN later. At this point actually sending the packet may still + * fail. In this unlikely event it will simply be handled as though it + * were a lost packet. */ - ++txp->next_pn[pn_space]; + /* Send the packet. */ + if (!ossl_qtx_write_pkt(txp->args.qtx, &txpkt)) + return 0; + /* * Record FC and stream abort frames as sent; deactivate streams which no * longer have anything to do. diff --git a/test/quic_multistream_test.c b/test/quic_multistream_test.c index 4bf14a5040..c5cf6479c9 100644 --- a/test/quic_multistream_test.c +++ b/test/quic_multistream_test.c @@ -2845,20 +2845,60 @@ static int script_39_inject_plain(struct helper *h, QUIC_PKT_HDR *hdr, WPACKET wpkt; unsigned char frame_buf[64]; size_t i, written; + uint64_t seq_no = 0, retire_prior_to = 0; + QUIC_CONN_ID new_cid = {0}; + QUIC_CHANNEL *ch = ossl_quic_tserver_get_channel(h->s); - if (h->inject_word1 == 0) + switch (h->inject_word1) { + case 0: return 1; + case 1: + new_cid.id_len = 0; + break; + case 2: + new_cid.id_len = 21; + break; + case 3: + new_cid.id_len = 1; + new_cid.id[0] = 0x55; + + seq_no = 0; + retire_prior_to = 1; + break; + case 4: + /* Cheese it by using our actual CID so we don't break connectivity. */ + ossl_quic_channel_get_diag_local_cid(ch, &new_cid); + + seq_no = 2; + retire_prior_to = 2; + break; + case 5: + /* + * Use a bogus CID which will need to be ignored if connectivity is to + * be continued. + */ + new_cid.id_len = 8; + new_cid.id[0] = 0x55; + + seq_no = 1; + retire_prior_to = 1; + break; + } if (!TEST_true(WPACKET_init_static_len(&wpkt, frame_buf, sizeof(frame_buf), 0))) return 0; if (!TEST_true(WPACKET_quic_write_vlint(&wpkt, OSSL_QUIC_FRAME_TYPE_NEW_CONN_ID)) - || !TEST_true(WPACKET_quic_write_vlint(&wpkt, 0)) /* seq no */ - || !TEST_true(WPACKET_quic_write_vlint(&wpkt, 0)) /* retire prior to */ - || !TEST_true(WPACKET_put_bytes_u8(&wpkt, 0))) /* len */ + || !TEST_true(WPACKET_quic_write_vlint(&wpkt, seq_no)) /* seq no */ + || !TEST_true(WPACKET_quic_write_vlint(&wpkt, retire_prior_to)) /* retire prior to */ + || !TEST_true(WPACKET_put_bytes_u8(&wpkt, new_cid.id_len))) /* len */ goto err; + for (i = 0; i < new_cid.id_len; ++i) + if (!TEST_true(WPACKET_put_bytes_u8(&wpkt, new_cid.id[i]))) + goto err; + for (i = 0; i < QUIC_STATELESS_RESET_TOKEN_LEN; ++i) if (!TEST_true(WPACKET_put_bytes_u8(&wpkt, 0x42))) goto err; @@ -3631,6 +3671,80 @@ static const struct script_op script_54[] = { OP_END }; +/* 55. Fault injection - NEW_CONN_ID with >20 byte CID */ +static const struct script_op script_55[] = { + OP_S_SET_INJECT_PLAIN (script_39_inject_plain) + OP_C_SET_ALPN ("ossltest") + OP_C_CONNECT_WAIT () + OP_C_SET_DEFAULT_STREAM_MODE(SSL_DEFAULT_STREAM_MODE_NONE) + + OP_C_NEW_STREAM_BIDI (a, C_BIDI_ID(0)) + OP_C_WRITE (a, "apple", 5) + OP_S_BIND_STREAM_ID (a, C_BIDI_ID(0)) + OP_S_READ_EXPECT (a, "apple", 5) + + OP_SET_INJECT_WORD (0, 2) + OP_S_WRITE (a, "orange", 5) + + OP_C_EXPECT_CONN_CLOSE_INFO(QUIC_ERR_FRAME_ENCODING_ERROR,0,0) + + OP_END +}; + +/* 56. Fault injection - NEW_CONN_ID with seq no < retire prior to */ +static const struct script_op script_56[] = { + OP_S_SET_INJECT_PLAIN (script_39_inject_plain) + OP_C_SET_ALPN ("ossltest") + OP_C_CONNECT_WAIT () + OP_C_SET_DEFAULT_STREAM_MODE(SSL_DEFAULT_STREAM_MODE_NONE) + + OP_C_NEW_STREAM_BIDI (a, C_BIDI_ID(0)) + OP_C_WRITE (a, "apple", 5) + OP_S_BIND_STREAM_ID (a, C_BIDI_ID(0)) + OP_S_READ_EXPECT (a, "apple", 5) + + OP_SET_INJECT_WORD (0, 3) + OP_S_WRITE (a, "orange", 5) + + OP_C_EXPECT_CONN_CLOSE_INFO(QUIC_ERR_FRAME_ENCODING_ERROR,0,0) + + OP_END +}; + +/* 57. Fault injection - NEW_CONN_ID with lower seq no ignored */ +static const struct script_op script_57[] = { + OP_S_SET_INJECT_PLAIN (script_39_inject_plain) + OP_C_SET_ALPN ("ossltest") + OP_C_CONNECT_WAIT () + OP_C_SET_DEFAULT_STREAM_MODE(SSL_DEFAULT_STREAM_MODE_NONE) + + OP_C_NEW_STREAM_BIDI (a, C_BIDI_ID(0)) + OP_C_WRITE (a, "apple", 5) + OP_S_BIND_STREAM_ID (a, C_BIDI_ID(0)) + OP_S_READ_EXPECT (a, "apple", 5) + + OP_SET_INJECT_WORD (0, 4) + OP_S_WRITE (a, "orange", 5) + OP_C_READ_EXPECT (a, "orange", 5) + + OP_C_WRITE (a, "Strawberry", 10) + OP_S_READ_EXPECT (a, "Strawberry", 10) + + /* + * Now we send a NEW_CONN_ID with a bogus CID. However the sequence number + * is old so it should be ignored and we should still be able to + * communicate. + */ + OP_SET_INJECT_WORD (0, 5) + OP_S_WRITE (a, "raspberry", 9) + OP_C_READ_EXPECT (a, "raspberry", 9) + + OP_C_WRITE (a, "peach", 5) + OP_S_READ_EXPECT (a, "peach", 5) + + OP_END +}; + static const struct script_op *const scripts[] = { script_1, script_2, @@ -3686,6 +3800,9 @@ static const struct script_op *const scripts[] = { script_52, script_53, script_54, + script_55, + script_56, + script_57, }; static int test_script(int idx) -- cgit v1.2.3