QUIC ACKM: Clarify probe types

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #19925)
openssl · Jan 30, 2023 · 8ca3baa · 8ca3baa
1 parent 51cf034
commit 8ca3baa
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 12 deletions.
diff --git a/include/internal/quic_ackm.h b/include/internal/quic_ackm.h
@@ -205,8 +205,25 @@ int ossl_ackm_is_ack_desired(OSSL_ACKM *ackm, int pkt_space);
 int ossl_ackm_is_rx_pn_processable(OSSL_ACKM *ackm, QUIC_PN pn, int pkt_space);
 
 typedef struct ossl_ackm_probe_info_st {
-    uint32_t handshake;
-    uint32_t padded_initial;
+    /*
+     * The following two probe request types are used only for anti-deadlock
+     * purposes in relation to the anti-amplification logic, by generating
+     * packets to buy ourselves more anti-amplification credit with the server
+     * until a client address is verified. Note that like all Initial packets,
+     * any Initial probes are padded.
+     *
+     * Note: The ACKM will only ever increase these by one at a time,
+     * as only one probe packet should be generated for these cases.
+     */
+    uint32_t anti_deadlock_initial, anti_deadlock_handshake;
+
+    /*
+     * Send an ACK-eliciting packet for each count here.
+     *
+     * Note: The ACKM may increase this by either one or two for each probe
+     * request, depending on how many probe packets it thinks should be
+     * generated.
+     */
     uint32_t pto[QUIC_PN_SPACE_NUM];
 } OSSL_ACKM_PROBE_INFO;
 

diff --git a/ssl/quic/quic_ackm.c b/ssl/quic/quic_ackm.c
@@ -1250,18 +1250,22 @@ int ossl_ackm_on_handshake_confirmed(OSSL_ACKM *ackm)
     return 1;
 }
 
-static void ackm_queue_probe_handshake(OSSL_ACKM *ackm)
+static void ackm_queue_probe_anti_deadlock_handshake(OSSL_ACKM *ackm)
 {
-    ++ackm->pending_probe.handshake;
+    ++ackm->pending_probe.anti_deadlock_handshake;
 }
 
-static void ackm_queue_probe_padded_initial(OSSL_ACKM *ackm)
+static void ackm_queue_probe_anti_deadlock_initial(OSSL_ACKM *ackm)
 {
-    ++ackm->pending_probe.padded_initial;
+    ++ackm->pending_probe.anti_deadlock_initial;
 }
 
 static void ackm_queue_probe(OSSL_ACKM *ackm, int pkt_space)
 {
+    /*
+     * TODO(QUIC): We are allowed to send either one or two probe packets here.
+     * Determine a strategy for when we should send two probe packets.
+     */
     ++ackm->pending_probe.pto[pkt_space];
 }
 
@@ -1289,9 +1293,9 @@ int ossl_ackm_on_timeout(OSSL_ACKM *ackm)
          * ownership.
          */
         if (ackm->discarded[QUIC_PN_SPACE_INITIAL])
-            ackm_queue_probe_handshake(ackm);
+            ackm_queue_probe_anti_deadlock_handshake(ackm);
         else
-            ackm_queue_probe_padded_initial(ackm);
+            ackm_queue_probe_anti_deadlock_initial(ackm);
     } else {
         /*
          * PTO. The user of the ACKM should send new data if available, else

diff --git a/test/quic_ackm_test.c b/test/quic_ackm_test.c
@@ -309,15 +309,15 @@ enum {
 };
 
 static int test_probe_counts(const OSSL_ACKM_PROBE_INFO *p,
-                             uint32_t handshake,
-                             uint32_t padded_initial,
+                             uint32_t anti_deadlock_handshake,
+                             uint32_t anti_deadlock_initial,
                              uint32_t pto_initial,
                              uint32_t pto_handshake,
                              uint32_t pto_app)
 {
-    if (!TEST_uint_eq(p->handshake, handshake))
+    if (!TEST_uint_eq(p->anti_deadlock_handshake, anti_deadlock_handshake))
         return 0;
-    if (!TEST_uint_eq(p->padded_initial, padded_initial))
+    if (!TEST_uint_eq(p->anti_deadlock_initial, anti_deadlock_initial))
         return 0;
     if (!TEST_uint_eq(p->pto[QUIC_PN_SPACE_INITIAL], pto_initial))
         return 0;