From: Neil Horman Date: Fri, 15 Nov 2024 18:34:37 +0000 (-0500) Subject: Augment ossl_quic_wire_decode_pkt_hdr X-Git-Tag: openssl-3.5.0-alpha1~323 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2784112e9a5d844b7a54ae6de5ab9d06a9822ef6;p=thirdparty%2Fopenssl.git Augment ossl_quic_wire_decode_pkt_hdr In preparation for supporting the handling of version negotiation, we need to be able to detect why the decoding of quic header failed. Specifically, ossl_quic_wire_decode_pkt_hdr fails if the version provided in the header isn't QUIC_VERSION_1. We want to keep that, as we don't support anything else, but the server code needs to differentiate when we fail decode because of a version problem, vs some other more fatal malforming issue. So add a uint64_t *fail_cause pointer that gets filled out with a failure cause. We only use VERSION failures right now, but we can expand this later if needed Reviewed-by: Tomas Mraz Reviewed-by: Saša Nedvědický (Merged from https://github.com/openssl/openssl/pull/25968) --- diff --git a/include/internal/quic_wire_pkt.h b/include/internal/quic_wire_pkt.h index 18a483fc2cc..bcad134658c 100644 --- a/include/internal/quic_wire_pkt.h +++ b/include/internal/quic_wire_pkt.h @@ -465,14 +465,23 @@ struct quic_pkt_hdr_ptrs_st { * QUIC_MAX_CONN_ID_LEN), this function fails when trying to decode a short * packet, but succeeds for long packets. * + * fail_cause is a bitmask of the reasons decode might have failed + * as defined below, useful when you need to interrogate parts of + * a header even if its otherwise undecodeable. May be NULL. + * * Returns 1 on success and 0 on failure. */ + +# define QUIC_PKT_HDR_DECODE_DECODE_ERR (1 << 0) +# define QUIC_PKT_HDR_DECODE_BAD_VERSION (1 << 1) + 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); + QUIC_PKT_HDR_PTRS *ptrs, + uint64_t *fail_cause); /* * Encodes a packet header. The packet is written to pkt. diff --git a/ssl/quic/quic_port.c b/ssl/quic/quic_port.c index 83b0a591c49..3d8f311906d 100644 --- a/ssl/quic/quic_port.c +++ b/ssl/quic/quic_port.c @@ -849,7 +849,7 @@ static void port_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, 0, &hdr, NULL)) + if (!ossl_quic_wire_decode_pkt_hdr(&pkt, SIZE_MAX, 1, 0, &hdr, NULL, NULL)) goto undesirable; switch (hdr.version) { diff --git a/ssl/quic/quic_record_rx.c b/ssl/quic/quic_record_rx.c index 722b957a437..cc201a6c2fe 100644 --- a/ssl/quic/quic_record_rx.c +++ b/ssl/quic/quic_record_rx.c @@ -815,7 +815,8 @@ 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, 0, &rxe->hdr, &ptrs)) + need_second_decode, 0, &rxe->hdr, &ptrs, + NULL)) goto malformed; /* @@ -951,7 +952,7 @@ 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, 0, &rxe->hdr, NULL) != 1) + 0, 0, &rxe->hdr, NULL, NULL) != 1) goto malformed; } diff --git a/ssl/quic/quic_trace.c b/ssl/quic/quic_trace.c index cf337180ecc..000f743af0b 100644 --- a/ssl/quic/quic_trace.c +++ b/ssl/quic/quic_trace.c @@ -588,7 +588,8 @@ int ossl_quic_trace(int write_p, int version, int content_type, * TODO(QUIC SERVER): 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, 1, &hdr, NULL) != 1) + if (ossl_quic_wire_decode_pkt_hdr(&pkt, 0, 0, 1, &hdr, NULL, + 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 e0018377af2..78ee95eacf5 100644 --- a/ssl/quic/quic_wire_pkt.c +++ b/ssl/quic/quic_wire_pkt.c @@ -187,12 +187,16 @@ int ossl_quic_wire_decode_pkt_hdr(PACKET *pkt, int partial, int nodata, QUIC_PKT_HDR *hdr, - QUIC_PKT_HDR_PTRS *ptrs) + QUIC_PKT_HDR_PTRS *ptrs, + uint64_t *fail_cause) { unsigned int b0; unsigned char *pn = NULL; size_t l = PACKET_remaining(pkt); + if (fail_cause != NULL) + *fail_cause = QUIC_PKT_HDR_DECODE_DECODE_ERR; + if (ptrs != NULL) { ptrs->raw_start = (unsigned char *)PACKET_data(pkt); ptrs->raw_sample = NULL; @@ -332,6 +336,8 @@ int ossl_quic_wire_decode_pkt_hdr(PACKET *pkt, if (!PACKET_forward(pkt, hdr->len)) return 0; } else if (version != QUIC_VERSION_1) { + if (fail_cause != NULL) + *fail_cause |= QUIC_PKT_HDR_DECODE_BAD_VERSION; /* Unknown version, do not decode. */ return 0; } else { @@ -451,6 +457,12 @@ int ossl_quic_wire_decode_pkt_hdr(PACKET *pkt, } } + /* + * Good decode, clear the generic DECODE_ERR flag + */ + if (fail_cause != NULL) + *fail_cause &= ~QUIC_PKT_HDR_DECODE_DECODE_ERR; + return 1; } diff --git a/test/helpers/pktsplitbio.c b/test/helpers/pktsplitbio.c index 536e62f8a00..8b8ff364abe 100644 --- a/test/helpers/pktsplitbio.c +++ b/test/helpers/pktsplitbio.c @@ -92,7 +92,7 @@ static int pkt_split_dgram_recvmmsg(BIO *bio, BIO_MSG *msg, size_t stride, * TODO(QUIC SERVER): 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, 0, &hdr, NULL) != 1) + if (ossl_quic_wire_decode_pkt_hdr(&pkt, 0, 0, 0, &hdr, NULL, NULL) != 1) return 0; remain = PACKET_remaining(&pkt); if (remain > 0) { diff --git a/test/helpers/quictestlib.c b/test/helpers/quictestlib.c index dbad8afe597..db39d659b16 100644 --- a/test/helpers/quictestlib.c +++ b/test/helpers/quictestlib.c @@ -1117,7 +1117,7 @@ static int pcipher_sendmmsg(BIO *b, BIO_MSG *msg, size_t stride, */ 0, 1, - 0, &hdr, NULL)) + 0, &hdr, NULL, NULL)) goto out; /* diff --git a/test/quic_record_test.c b/test/quic_record_test.c index 4c1648dc6d0..45be630dde0 100644 --- a/test/quic_record_test.c +++ b/test/quic_record_test.c @@ -2808,7 +2808,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, 0, &hdr, &ptrs), + 0, 0, &hdr, &ptrs, NULL), !expect_fail)) goto err; diff --git a/test/quic_wire_test.c b/test/quic_wire_test.c index 69f4cf2977d..0a4d61acc86 100644 --- a/test/quic_wire_test.c +++ b/test/quic_wire_test.c @@ -1542,7 +1542,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, 0, &hdr, NULL))) + if (!TEST_true(ossl_quic_wire_decode_pkt_hdr(&pkt, 0, 0, 0, &hdr, NULL, NULL))) goto err; if (!TEST_int_eq(hdr.type, QUIC_PKT_TYPE_RETRY))