From: Hugo Landau Date: Mon, 3 Jul 2023 14:45:25 +0000 (+0100) Subject: QUIC ACKM: RFC 9000 s. 13.2.1: max_ack_delay taken as 0 in INITIAL/HANDSHAKE X-Git-Tag: openssl-3.2.0-alpha1~394 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=76908b45823f958f29b6bdf11efab6eac47f61ca;p=thirdparty%2Fopenssl.git QUIC ACKM: RFC 9000 s. 13.2.1: max_ack_delay taken as 0 in INITIAL/HANDSHAKE Reviewed-by: Tomas Mraz Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/21349) --- diff --git a/ssl/quic/quic_ackm.c b/ssl/quic/quic_ackm.c index 025b7cc7bd3..facd34176a0 100644 --- a/ssl/quic/quic_ackm.c +++ b/ssl/quic/quic_ackm.c @@ -1466,6 +1466,8 @@ static void ackm_on_rx_ack_eliciting(OSSL_ACKM *ackm, OSSL_TIME rx_time, int pkt_space, int was_missing) { + OSSL_TIME tx_max_ack_delay; + if (ackm->rx_ack_desired[pkt_space]) /* ACK generation already requested so nothing to do. */ return; @@ -1507,15 +1509,24 @@ static void ackm_on_rx_ack_eliciting(OSSL_ACKM *ackm, * Not emitting an ACK yet. * * Update the ACK flush deadline. + * + * RFC 9000 s. 13.2.1: "An endpoint MUST acknowledge all ack-eliciting + * Initial and Handshake packets immediately"; don't delay ACK generation if + * we are using the Initial or Handshake PN spaces. */ + tx_max_ack_delay = ackm->tx_max_ack_delay; + if (pkt_space == QUIC_PN_SPACE_INITIAL + || pkt_space == QUIC_PN_SPACE_HANDSHAKE) + tx_max_ack_delay = ossl_time_zero(); + if (ossl_time_is_infinite(ackm->rx_ack_flush_deadline[pkt_space])) ackm_set_flush_deadline(ackm, pkt_space, - ossl_time_add(rx_time, ackm->tx_max_ack_delay)); + ossl_time_add(rx_time, tx_max_ack_delay)); else ackm_set_flush_deadline(ackm, pkt_space, ossl_time_min(ackm->rx_ack_flush_deadline[pkt_space], ossl_time_add(rx_time, - ackm->tx_max_ack_delay))); + tx_max_ack_delay))); } int ossl_ackm_on_rx_packet(OSSL_ACKM *ackm, const OSSL_ACKM_RX_PKT *pkt) diff --git a/test/quic_ackm_test.c b/test/quic_ackm_test.c index b92e153d1a1..7e7c35816b7 100644 --- a/test/quic_ackm_test.c +++ b/test/quic_ackm_test.c @@ -618,7 +618,8 @@ enum { RX_OPK_CHECK_STATE, /* check is_desired/deadline */ RX_OPK_CHECK_ACKS, /* check ACK ranges */ RX_OPK_TX, /* TX packet */ - RX_OPK_RX_ACK /* RX ACK frame */ + RX_OPK_RX_ACK, /* RX ACK frame */ + RX_OPK_SKIP_IF_PN_SPACE /* skip for a given PN space */ }; struct rx_test_op { @@ -685,6 +686,12 @@ struct rx_test_op { 0, 0, NULL, 0, 0 \ }, +#define RX_OP_SKIP_IF_PN_SPACE(pn_space) \ + { \ + RX_OPK_SKIP_IF_PN_SPACE, 0, (pn_space), 0, \ + 0, 0, NULL, 0, 0 \ + }, + #define RX_OP_END \ { RX_OPK_END } @@ -715,7 +722,7 @@ static const struct rx_test_op rx_script_1[] = { RX_OP_END }; -/* RX 2. Simple Test with ACK Not Yet Desired (Packet Threshold) */ +/* RX 2. Simple Test with ACK Not Yet Desired (Packet Threshold) (1-RTT) */ static const OSSL_QUIC_ACK_RANGE rx_ack_ranges_2a[] = { { 0, 0 } }; @@ -725,6 +732,14 @@ static const OSSL_QUIC_ACK_RANGE rx_ack_ranges_2b[] = { }; static const struct rx_test_op rx_script_2[] = { + /* + * We skip this for INITIAL/HANDSHAKE and use a separate version + * (rx_script_4) for those spaces as those spaces should not delay ACK + * generation, so a different RX_OP_CHECK_STATE test is needed. + */ + RX_OP_SKIP_IF_PN_SPACE(QUIC_PN_SPACE_INITIAL) + RX_OP_SKIP_IF_PN_SPACE(QUIC_PN_SPACE_HANDSHAKE) + RX_OP_CHECK_STATE (0, 0, 0) /* no threshold yet */ RX_OP_CHECK_PROC (0, 0, 3) @@ -826,10 +841,57 @@ static const struct rx_test_op rx_script_3[] = { RX_OP_END }; +/* + * RX 4. Simple Test with ACK Not Yet Desired (Packet Threshold) + * (Initial/Handshake) + */ +static const OSSL_QUIC_ACK_RANGE rx_ack_ranges_4a[] = { + { 0, 1 } +}; + +static const struct rx_test_op rx_script_4[] = { + /* The application PN space is tested in rx_script_2. */ + RX_OP_SKIP_IF_PN_SPACE(QUIC_PN_SPACE_APP) + + RX_OP_CHECK_STATE (0, 0, 0) /* no threshold yet */ + RX_OP_CHECK_PROC (0, 0, 3) + + /* First packet always generates an ACK so get it out of the way. */ + RX_OP_PKT (0, 0, 1) + RX_OP_CHECK_UNPROC (0, 0, 1) + RX_OP_CHECK_PROC (0, 1, 1) + RX_OP_CHECK_STATE (0, 1, 0) /* first packet always causes ACK */ + RX_OP_CHECK_ACKS (0, rx_ack_ranges_2a) /* clears packet counter */ + RX_OP_CHECK_STATE (0, 0, 0) /* desired state should have been cleared */ + + /* + * Second packet should cause ACK-desired state because we are + * INITIAL/HANDSHAKE (RFC 9000 s. 13.2.1) + */ + RX_OP_PKT (0, 1, 1) /* just one packet, threshold is 2 */ + RX_OP_CHECK_UNPROC (0, 0, 2) + RX_OP_CHECK_PROC (0, 2, 1) + RX_OP_CHECK_STATE (0, 1, 1) + RX_OP_CHECK_ACKS (0, rx_ack_ranges_4a) + RX_OP_CHECK_STATE (0, 0, 0) /* desired state should have been cleared */ + + /* At this point we would generate e.g. a packet with an ACK. */ + RX_OP_TX (0, 0, 1) /* ACKs all */ + RX_OP_CHECK_ACKS (0, rx_ack_ranges_4a) /* not provably ACKed yet */ + RX_OP_RX_ACK (0, 0, 1) /* TX'd packet is ACK'd */ + + RX_OP_CHECK_NO_ACKS (0) /* nothing more to ACK */ + RX_OP_CHECK_UNPROC (0, 0, 2) /* still unprocessable */ + RX_OP_CHECK_PROC (0, 2, 1) /* still processable */ + + RX_OP_END +}; + static const struct rx_test_op *const rx_test_scripts[] = { rx_script_1, rx_script_2, - rx_script_3 + rx_script_3, + rx_script_4 }; static void on_ack_deadline_callback(OSSL_TIME deadline, @@ -850,6 +912,7 @@ static int test_rx_ack_actual(int tidx, int space) struct pkt_info *pkts = NULL; OSSL_ACKM_TX_PKT *txs = NULL, *tx; OSSL_TIME ack_deadline[QUIC_PN_SPACE_NUM]; + size_t opn = 0; for (i = 0; i < QUIC_PN_SPACE_NUM; ++i) ack_deadline[i] = ossl_time_infinite(); @@ -880,7 +943,7 @@ static int test_rx_ack_actual(int tidx, int space) goto err; /* Run script. */ - for (s = script; s->kind != RX_OPK_END; ++s) { + for (s = script; s->kind != RX_OPK_END; ++s, ++opn) { fake_time = ossl_time_add(fake_time, ossl_ticks2time(s->time_advance)); switch (s->kind) { @@ -992,6 +1055,14 @@ static int test_rx_ack_actual(int tidx, int space) break; + case RX_OPK_SKIP_IF_PN_SPACE: + if (space == s->pn) { + testresult = 1; + goto err; + } + + break; + default: goto err; } @@ -999,6 +1070,9 @@ static int test_rx_ack_actual(int tidx, int space) testresult = 1; err: + if (!testresult) + TEST_error("error in ACKM RX script %d, op %zu", tidx + 1, opn + 1); + helper_destroy(&h); OPENSSL_free(pkts); OPENSSL_free(txs);