From e8528c95a0543a218b432d2ea02e6bd0c1e7ab19 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 5 May 2023 16:51:43 +0100 Subject: [PATCH] Enable tracing of packets that have been sent Reviewed-by: Tomas Mraz Reviewed-by: Hugo Landau (Merged from https://github.com/openssl/openssl/pull/20914) --- include/internal/quic_record_rx.h | 3 --- include/internal/quic_record_tx.h | 5 +++++ include/internal/quic_types.h | 3 +++ include/internal/quic_wire_pkt.h | 4 ++++ ssl/quic/quic_channel.c | 6 +++++- ssl/quic/quic_record_rx.c | 13 +++++++------ ssl/quic/quic_record_tx.c | 19 ++++++++++++++++--- ssl/quic/quic_trace.c | 2 +- ssl/quic/quic_wire_pkt.c | 19 +++++++++++++------ test/helpers/quictestlib.c | 2 +- test/quic_record_test.c | 2 +- test/quic_wire_test.c | 2 +- 12 files changed, 57 insertions(+), 23 deletions(-) diff --git a/include/internal/quic_record_rx.h b/include/internal/quic_record_rx.h index 4817347993..f372887f1b 100644 --- a/include/internal/quic_record_rx.h +++ b/include/internal/quic_record_rx.h @@ -18,9 +18,6 @@ # ifndef OPENSSL_NO_QUIC -typedef void (*ossl_msg_cb)(int write_p, int version, int content_type, - const void *buf, size_t len, SSL *ssl, void *arg); - /* * QUIC Record Layer - RX * ====================== diff --git a/include/internal/quic_record_tx.h b/include/internal/quic_record_tx.h index 2cc7333c02..21bfb9d01b 100644 --- a/include/internal/quic_record_tx.h +++ b/include/internal/quic_record_tx.h @@ -46,6 +46,11 @@ typedef struct ossl_qtx_args_st { /* Maximum datagram payload length (MDPL) for TX purposes. */ size_t mdpl; + + /* Message callback related arguments */ + ossl_msg_cb msg_callback; + void *msg_callback_arg; + SSL *msg_callback_s; } OSSL_QTX_ARGS; /* Instantiates a new QTX. */ diff --git a/include/internal/quic_types.h b/include/internal/quic_types.h index 2cf8ebefc8..2fa36d46e9 100644 --- a/include/internal/quic_types.h +++ b/include/internal/quic_types.h @@ -95,6 +95,9 @@ static ossl_unused ossl_inline int ossl_quic_conn_id_eq(const QUIC_CONN_ID *a, # define QUIC_STATELESS_RESET_TOKEN_LEN 16 +typedef void (*ossl_msg_cb)(int write_p, int version, int content_type, + const void *buf, size_t len, SSL *ssl, void *arg); + # endif #endif diff --git a/include/internal/quic_wire_pkt.h b/include/internal/quic_wire_pkt.h index ce3c63057e..5979a5ceb5 100644 --- a/include/internal/quic_wire_pkt.h +++ b/include/internal/quic_wire_pkt.h @@ -425,6 +425,9 @@ struct quic_pkt_hdr_ptrs_st { * If partial is 0, the input is assumed to have already had header protection * removed, and all header fields are decoded. * + * If nodata is 1, the input is assumed to have no payload data in it. Otherwise + * payload data must be present. + * * On success, the logical decode of the packet header is written to *hdr. * hdr->partial is set or cleared according to whether a partial decode was * performed. *ptrs is filled with pointers to various parts of the packet @@ -441,6 +444,7 @@ struct quic_pkt_hdr_ptrs_st { int ossl_quic_wire_decode_pkt_hdr(PACKET *pkt, size_t short_conn_id_len, int partial, + int nodata, QUIC_PKT_HDR *hdr, QUIC_PKT_HDR_PTRS *ptrs); diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c index cb24bd0c1a..76546e2bd3 100644 --- a/ssl/quic/quic_channel.c +++ b/ssl/quic/quic_channel.c @@ -130,6 +130,10 @@ static int ch_init(QUIC_CHANNEL *ch) /* We plug in a network write BIO to the QTX later when we get one. */ qtx_args.libctx = ch->libctx; qtx_args.mdpl = QUIC_MIN_INITIAL_DGRAM_LEN; + /* Callback related arguments */ + qtx_args.msg_callback = ch->msg_callback; + qtx_args.msg_callback_arg = ch->msg_callback_arg; + qtx_args.msg_callback_s = ch->msg_callback_s; ch->rx_max_udp_payload_size = qtx_args.mdpl; ch->qtx = ossl_qtx_new(&qtx_args); @@ -1602,7 +1606,7 @@ static void ch_default_packet_handler(QUIC_URXE *e, void *arg) * operation to fail if we get a 1-RTT packet. This is fine since we only * care about Initial packets. */ - if (!ossl_quic_wire_decode_pkt_hdr(&pkt, SIZE_MAX, 1, &hdr, NULL)) + if (!ossl_quic_wire_decode_pkt_hdr(&pkt, SIZE_MAX, 1, 0, &hdr, NULL)) goto undesirable; switch (hdr.version) { diff --git a/ssl/quic/quic_record_rx.c b/ssl/quic/quic_record_rx.c index 40c76a6bc1..266dee9d31 100644 --- a/ssl/quic/quic_record_rx.c +++ b/ssl/quic/quic_record_rx.c @@ -726,7 +726,7 @@ static int qrx_process_pkt(OSSL_QRX *qrx, QUIC_URXE *urxe, need_second_decode = !pkt_is_marked(&urxe->hpr_removed, pkt_idx); if (!ossl_quic_wire_decode_pkt_hdr(pkt, qrx->short_conn_id_len, - need_second_decode, &rxe->hdr, &ptrs)) + need_second_decode, 0, &rxe->hdr, &ptrs)) goto malformed; /* @@ -838,18 +838,19 @@ static int qrx_process_pkt(OSSL_QRX *qrx, QUIC_URXE *urxe, /* Decode the now unprotected header. */ if (ossl_quic_wire_decode_pkt_hdr(pkt, qrx->short_conn_id_len, - 0, &rxe->hdr, NULL) != 1) + 0, 0, &rxe->hdr, NULL) != 1) goto malformed; - - if (qrx->msg_callback != NULL) - qrx->msg_callback(0, OSSL_QUIC1_VERSION, SSL3_RT_QUIC_PACKET, sop, - eop - sop, qrx->msg_callback_s, qrx->msg_callback_arg); } /* Validate header and decode PN. */ if (!qrx_validate_hdr(qrx, rxe)) goto malformed; + if (qrx->msg_callback != NULL) + qrx->msg_callback(0, OSSL_QUIC1_VERSION, SSL3_RT_QUIC_PACKET, sop, + eop - sop - rxe->hdr.len, qrx->msg_callback_s, + qrx->msg_callback_arg); + /* * The AAD data is the entire (unprotected) packet header including the PN. * The packet header has been unprotected in place, so we can just reuse the diff --git a/ssl/quic/quic_record_tx.c b/ssl/quic/quic_record_tx.c index 28ebc436bb..9040f2f904 100644 --- a/ssl/quic/quic_record_tx.c +++ b/ssl/quic/quic_record_tx.c @@ -94,6 +94,11 @@ struct ossl_qtx_st { ossl_mutate_packet_cb mutatecb; ossl_finish_mutate_cb finishmutatecb; void *mutatearg; + + /* Message callback related arguments */ + ossl_msg_cb msg_callback; + void *msg_callback_arg; + SSL *msg_callback_s; }; /* Instantiates a new QTX. */ @@ -112,6 +117,9 @@ OSSL_QTX *ossl_qtx_new(const OSSL_QTX_ARGS *args) qtx->propq = args->propq; qtx->bio = args->bio; qtx->mdpl = args->mdpl; + qtx->msg_callback = args->msg_callback; + qtx->msg_callback_arg = args->msg_callback_arg; + qtx->msg_callback_s = args->msg_callback_s; return qtx; } @@ -432,9 +440,9 @@ static int qtx_write_hdr(OSSL_QTX *qtx, const QUIC_PKT_HDR *hdr, TXE *txe, { WPACKET wpkt; size_t l = 0; + unsigned char *data = txe_data(txe) + txe->data_len; - if (!WPACKET_init_static_len(&wpkt, txe_data(txe) + txe->data_len, - txe->alloc_len - txe->data_len, 0)) + if (!WPACKET_init_static_len(&wpkt, data, txe->alloc_len - txe->data_len, 0)) return 0; if (!ossl_quic_wire_encode_pkt_hdr(&wpkt, hdr->dst_conn_id.id_len, @@ -443,9 +451,14 @@ static int qtx_write_hdr(OSSL_QTX *qtx, const QUIC_PKT_HDR *hdr, TXE *txe, WPACKET_finish(&wpkt); return 0; } + WPACKET_finish(&wpkt); + + if (qtx->msg_callback != NULL) + qtx->msg_callback(1, OSSL_QUIC1_VERSION, SSL3_RT_QUIC_PACKET, data, l, + qtx->msg_callback_s, qtx->msg_callback_arg); txe->data_len += l; - WPACKET_finish(&wpkt); + return 1; } diff --git a/ssl/quic/quic_trace.c b/ssl/quic/quic_trace.c index 025e8189ef..aa25e8f028 100644 --- a/ssl/quic/quic_trace.c +++ b/ssl/quic/quic_trace.c @@ -495,7 +495,7 @@ int ossl_quic_trace(int write_p, int version, int content_type, * TODO(QUIC): We need to query the short connection id len here, * e.g. via some API SSL_get_short_conn_id_len() */ - if (ossl_quic_wire_decode_pkt_hdr(&pkt, 0, 0, &hdr, NULL) != 1) + if (ossl_quic_wire_decode_pkt_hdr(&pkt, 0, 0, 1, &hdr, NULL) != 1) return 0; BIO_puts(bio, write_p ? "Sent" : "Received"); diff --git a/ssl/quic/quic_wire_pkt.c b/ssl/quic/quic_wire_pkt.c index e0180ec060..d63101095f 100644 --- a/ssl/quic/quic_wire_pkt.c +++ b/ssl/quic/quic_wire_pkt.c @@ -162,6 +162,7 @@ int ossl_quic_hdr_protector_encrypt_fields(QUIC_HDR_PROTECTOR *hpr, int ossl_quic_wire_decode_pkt_hdr(PACKET *pkt, size_t short_conn_id_len, int partial, + int nodata, QUIC_PKT_HDR *hdr, QUIC_PKT_HDR_PTRS *ptrs) { @@ -369,8 +370,10 @@ int ossl_quic_wire_decode_pkt_hdr(PACKET *pkt, hdr->pn_len = partial ? 0 : (b0 & 3) + 1; if (!PACKET_get_quic_vlint(pkt, &len) - || len < sizeof(hdr->pn) - || len > PACKET_remaining(pkt)) + || len < sizeof(hdr->pn)) + return 0; + + if (!nodata && len > PACKET_remaining(pkt)) return 0; /* @@ -393,11 +396,15 @@ int ossl_quic_wire_decode_pkt_hdr(PACKET *pkt, hdr->len = (size_t)(len - hdr->pn_len); } - hdr->data = PACKET_data(pkt); + if (nodata) { + hdr->data = NULL; + } else { + hdr->data = PACKET_data(pkt); - /* Skip over packet body. */ - if (!PACKET_forward(pkt, hdr->len)) - return 0; + /* Skip over packet body. */ + if (!PACKET_forward(pkt, hdr->len)) + return 0; + } } } } diff --git a/test/helpers/quictestlib.c b/test/helpers/quictestlib.c index c973a8cc65..c33518805d 100644 --- a/test/helpers/quictestlib.c +++ b/test/helpers/quictestlib.c @@ -737,7 +737,7 @@ static int pcipher_sendmmsg(BIO *b, BIO_MSG *msg, size_t stride, do { if (!ossl_quic_wire_decode_pkt_hdr(&pkt, 0 /* TODO(QUIC): Not sure how this should be set*/, 1, - &hdr, NULL)) + 0, &hdr, NULL)) goto out; /* diff --git a/test/quic_record_test.c b/test/quic_record_test.c index 1dc0eec5e5..088e730af1 100644 --- a/test/quic_record_test.c +++ b/test/quic_record_test.c @@ -2522,7 +2522,7 @@ static int test_wire_pkt_hdr_actual(int tidx, int repeat, int cipher, goto err; if (!TEST_int_eq(ossl_quic_wire_decode_pkt_hdr(&pkt, t->short_conn_id_len, - 0, &hdr, &ptrs), + 0, 0, &hdr, &ptrs), !expect_fail)) goto err; diff --git a/test/quic_wire_test.c b/test/quic_wire_test.c index d6eef296a3..04e287fbf7 100644 --- a/test/quic_wire_test.c +++ b/test/quic_wire_test.c @@ -1513,7 +1513,7 @@ static int test_wire_retry_integrity_tag(void) if (!TEST_true(PACKET_buf_init(&pkt, retry_encoded, sizeof(retry_encoded)))) goto err; - if (!TEST_true(ossl_quic_wire_decode_pkt_hdr(&pkt, 0, 0, &hdr, NULL))) + if (!TEST_true(ossl_quic_wire_decode_pkt_hdr(&pkt, 0, 0, 0, &hdr, NULL))) goto err; if (!TEST_int_eq(hdr.type, QUIC_PKT_TYPE_RETRY)) -- 2.34.1