QUIC LCIDM: Enforce and document ODCID peculiarities

Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #22673)
openssl · Dec 6, 2023 · a35956b · a35956b
1 parent 1184157
commit a35956b
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 3 deletions.
diff --git a/include/internal/quic_lcidm.h b/include/internal/quic_lcidm.h
@@ -70,6 +70,29 @@
  *
  * Both (2) and (3) are retired normally via RETIRE_CONNECTION_ID frames, as it
  * has a sequence number of 0.
+ *
+ *
+ * ODCID Peculiarities
+ * -------------------
+ *
+ * Almost all LCIDs are issued by the receiver responsible for routing them,
+ * which means that almost all LCIDs will have the same length (specified in
+ * lcid_len below). The only exception to this is (1); the ODCID is the only
+ * case where we recognise an LCID we didn't ourselves generate. Since an ODCID
+ * is chosen by the peer, it can be any length and doesn't necessarily match the
+ * length we use for LCIDs we generate ourselves.
+ *
+ * Since DCID decoding for short-header packets requires an implicitly known
+ * DCID length, it logically follows that an ODCID can never be used in a 1-RTT
+ * packet. This is fine as by the time the 1-RTT EL is reached the peer should
+ * already have switched away from the ODCID to a CID we generated ourselves,
+ * and if this is not happened we can consider that a protocol violation.
+ *
+ * In any case, this means that the LCIDM must necessarily support LCIDs of
+ * different lengths, even if it always generates LCIDs of a given length.
+ *
+ * An ODCID has no sequence number associated with it. It is the only CID to
+ * lack one.
  */
 typedef struct quic_lcidm_st QUIC_LCIDM;
 
@@ -109,7 +132,9 @@ size_t ossl_quic_lcidm_get_num_active_lcid(const QUIC_LCIDM *lcidm,
  * LCIDM_ODCID_SEQ_NUM internally for our purposes.
  *
  * Note that this is the *only* circumstance where we recognise an LCID we did
- * not generate ourselves.
+ * not generate ourselves, or allow an LCID with a different length to lcid_len.
+ *
+ * An ODCID MUST be at least 8 bytes in length (RFC 9000 s. 7.2).
  *
  * This function may only be called once for a given connection.
  * Returns 1 on success or 0 on failure.

diff --git a/include/internal/quic_types.h b/include/internal/quic_types.h
@@ -73,6 +73,7 @@ static ossl_unused ossl_inline int ossl_quic_pn_valid(QUIC_PN pn)
 
 /* QUIC connection ID representation. */
 #  define QUIC_MAX_CONN_ID_LEN   20
+#  define QUIC_MIN_ODCID_LEN     8   /* RFC 9000 s. 7.2 */
 
 typedef struct quic_conn_id_st {
     unsigned char id_len, id[QUIC_MAX_CONN_ID_LEN];

diff --git a/ssl/quic/quic_lcidm.c b/ssl/quic/quic_lcidm.c
@@ -318,7 +318,8 @@ int ossl_quic_lcidm_enrol_odcid(QUIC_LCIDM *lcidm,
     QUIC_LCIDM_CONN *conn;
     QUIC_LCID key, *lcid_obj;
 
-    if (initial_odcid == NULL)
+    if (initial_odcid == NULL || initial_odcid->id_len < QUIC_MIN_ODCID_LEN
+        || initial_odcid->id_len > QUIC_MAX_CONN_ID_LEN)
         return 0;
 
     if ((conn = lcidm_upsert_conn(lcidm, opaque)) == NULL)

diff --git a/test/quic_lcidm_test.c b/test/quic_lcidm_test.c
@@ -22,7 +22,7 @@ static int test_lcidm(void)
 {
     int testresult = 0;
     QUIC_LCIDM *lcidm;
-    size_t lcid_len = 8;
+    size_t lcid_len = 10; /* != ODCID len */
     QUIC_CONN_ID lcid_1, lcid_dummy, lcid_init;
     OSSL_QUIC_FRAME_NEW_CONN_ID ncid_frame_1, ncid_frame_2, ncid_frame_3;
     void *opaque = NULL;