From 8ca3baa9bdf972b963a70769780db67ebcbdf779 Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Fri, 16 Dec 2022 10:11:10 +0000 Subject: [PATCH] QUIC ACKM: Clarify probe types Reviewed-by: Matt Caswell Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/19925) --- include/internal/quic_ackm.h | 21 +++++++++++++++++++-- ssl/quic/quic_ackm.c | 16 ++++++++++------ test/quic_ackm_test.c | 8 ++++---- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/include/internal/quic_ackm.h b/include/internal/quic_ackm.h index a255b754e3..2d5de7fd64 100644 --- 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 index 53eb51cfc8..2c01757494 100644 --- 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 index 83da7098bf..6036d0d0a9 100644 --- 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; -- 2.34.1