]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Augment ossl_quic_wire_decode_pkt_hdr
authorNeil Horman <nhorman@openssl.org>
Fri, 15 Nov 2024 18:34:37 +0000 (13:34 -0500)
committerNeil Horman <nhorman@openssl.org>
Mon, 17 Feb 2025 16:27:33 +0000 (11:27 -0500)
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 <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25968)

include/internal/quic_wire_pkt.h
ssl/quic/quic_port.c
ssl/quic/quic_record_rx.c
ssl/quic/quic_trace.c
ssl/quic/quic_wire_pkt.c
test/helpers/pktsplitbio.c
test/helpers/quictestlib.c
test/quic_record_test.c
test/quic_wire_test.c

index 18a483fc2cc688a6725d854693f5ea870c152025..bcad134658c4c5a865bc90acbdfd065a4d67d893 100644 (file)
@@ -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.
index 83b0a591c49db7dbad113519220c91818dec26aa..3d8f311906dfdd83b032da09ed363d88b8042688 100644 (file)
@@ -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) {
index 722b957a437a6e791fd3c7d0b3d847c5db9bde31..cc201a6c2feb68ea18bc5170d9d065b80932aacd 100644 (file)
@@ -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;
     }
 
index cf337180ecc0e09858c9695121e31e0ebbde1546..000f743af0b87bfe838163396497be666845711d 100644 (file)
@@ -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");
index e0018377af2ddd8cebd5d380c17dcb4ea1e44318..78ee95eacf536708c2c95bd39357d30f48cfc2e7 100644 (file)
@@ -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;
 }
 
index 536e62f8a0030d1951e16807a7553befa7f0d11b..8b8ff364abe5166f84c3f6e6eeb7e64fe73bd627 100644 (file)
@@ -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) {
index dbad8afe597c3bd15764e5a141c7cd20e74a7a7f..db39d659b16d56b3147bbb893cf02404a8aa6e58 100644 (file)
@@ -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;
 
                 /*
index 4c1648dc6d0532a6912646d93dde852295b3d140..45be630dde0843915da9132461268b48265c4904 100644 (file)
@@ -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;
 
index 69f4cf2977d764bee09f53a662836f9ed8cafa5a..0a4d61acc86a2bd4fa95b25bea46909c8ace2a70 100644 (file)
@@ -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))