Skip to content

dtls13: client ignores ACKs for earlier final-flight transmissions → Timeout(Handshake) after a late or lost ACK #172

Closed

Description

@xnorpx

Summary

After a DTLS 1.3 client has retransmitted its final flight (Certificate, CertificateVerify, Finished), it ignores any ACK that names the records of an earlier transmission of that flight. flight_resend overwrites each saved record's record number on every retransmission, and process_ack only matches the latest one. The client therefore keeps retransmitting until its retry budget is spent and returns Error::Timeout(TimeoutError::Handshake), even though it already emitted Output::Connected and the server has completed and acknowledged the flight.

Observed on main (0.7.6, 0c196c2) and on #162. Two ways to hit it:

  • Late ACK, no loss: the server's final ACK arrives after the client's retransmission timer has fired once. That ACK names the first transmission's records, so it is ignored.
  • Lost ACK: the server answers each retransmitted final flight with "a retransmit of its ACK" (RFC 9147 §5.8.1). Every copy names the first transmission's records, so none of them is ever matched.

Impact

  • A connected DTLS 1.3 client (Dtls::new_13, or Dtls::new_auto resolving to 1.3) fails with Timeout(Handshake) about half a minute later with default timing (31–41 s in the runs below), so the caller tears down a working connection. The server stays connected.
  • One late or lost ACK is enough. The late case needs no packet loss: the ACK only has to arrive after the client's first retransmission (0.75–1.25 s after the final flight with the default 1 s RTO).
  • If flight_start_rto is set below the path RTT (Start handshake clocks at ClientHello and honor timing settings consistently #161 prepares exposing it through str0m), the final ACK is typically late, so the client fails after most DTLS 1.3 handshakes.

Reproduction

Integration test using only the public API (drop into tests/, needs the rcgen feature)

It runs the handshake at one instant while holding back the server's final ACK, then drives both endpoints one second at a time, delivering every packet in both directions.

#![cfg(feature = "rcgen")]

use std::sync::Arc;
use std::time::{Duration, Instant};

use dimpl::certificate::generate_self_signed_certificate;
use dimpl::{Config, Dtls, Output};

fn drain(endpoint: &mut Dtls) -> (Vec<Vec<u8>>, bool) {
    let mut packets = Vec::new();
    let mut connected = false;
    let mut buffer = vec![0; 65536];
    loop {
        match endpoint.poll_output(&mut buffer) {
            Output::Packet(p) => packets.push(p.to_vec()),
            Output::Connected => connected = true,
            Output::Timeout(_) => return (packets, connected),
            _ => {}
        }
    }
}

/// Handshake at `base`, holding back the server's final ACK.
fn handshake_without_final_ack(base: Instant) -> (Dtls, Dtls, Vec<Vec<u8>>) {
    let config = Arc::new(
        Config::builder()
            .handshake_timeout(Duration::from_secs(120))
            .build()
            .unwrap(),
    );
    let mut client = Dtls::new_13(config.clone(), generate_self_signed_certificate().unwrap(), base);
    let mut server = Dtls::new_13(config, generate_self_signed_certificate().unwrap(), base);
    client.set_active(true);
    client.handle_timeout(base).unwrap();
    server.handle_timeout(base).unwrap();
    drain(&mut server);
    let mut to_server = drain(&mut client).0;
    for _ in 0..10 {
        let mut to_client = Vec::new();
        for packet in std::mem::take(&mut to_server) {
            server.handle_packet(&packet).unwrap();
            let (out, connected) = drain(&mut server);
            if connected {
                return (client, server, out);
            }
            to_client.extend(out);
        }
        for packet in to_client {
            client.handle_packet(&packet).unwrap();
            to_server.extend(drain(&mut client).0);
        }
    }
    panic!("server did not complete");
}

/// Deliver every client retransmission to the server and every server reply
/// back to the client, one second at a time.
fn run(label: &str, base: Instant, mut client: Dtls, mut server: Dtls) {
    let mut retransmissions = 0;
    for second in 3..=120u64 {
        let now = base + Duration::from_secs(second);
        if let Err(error) = client.handle_timeout(now) {
            eprintln!("{label}: {retransmissions} further client retransmissions, then {error:?} at {second}s");
            return;
        }
        server.handle_timeout(now).unwrap();
        let (packets, _) = drain(&mut client);
        if !packets.is_empty() {
            retransmissions += 1;
        }
        for packet in packets {
            server.handle_packet(&packet).unwrap();
            for reply in drain(&mut server).0 {
                client.handle_packet(&reply).unwrap();
            }
        }
    }
    eprintln!("{label}: {retransmissions} further client retransmissions, client still alive at 120s");
}

#[test]
fn dtls13_final_flight_ack_matching() {
    // Control: the final ACK arrives before the client retransmits.
    let base = Instant::now();
    let (mut client, server, final_ack) = handshake_without_final_ack(base);
    for packet in &final_ack {
        client.handle_packet(packet).unwrap();
    }
    client.handle_timeout(base).unwrap();
    drain(&mut client);
    run("control (ACK on time)", base, client, server);

    // Late ACK: no loss, the ACK just arrives after the first retransmission.
    let base = Instant::now();
    let (mut client, mut server, final_ack) = handshake_without_final_ack(base);
    client.handle_timeout(base).unwrap(); // arms the lazy timer on 0.7.6
    drain(&mut client);
    client.handle_timeout(base + Duration::from_secs(2)).unwrap();
    let (first_retransmission, _) = drain(&mut client);
    assert!(!first_retransmission.is_empty());
    for packet in &final_ack {
        client.handle_packet(packet).unwrap();
    }
    for packet in &first_retransmission {
        server.handle_packet(packet).unwrap();
        for reply in drain(&mut server).0 {
            client.handle_packet(&reply).unwrap();
        }
    }
    run("late ACK", base, client, server);

    // Lost ACK: the server's re-ACK of each retransmission is delivered.
    let base = Instant::now();
    let (mut client, server, _lost) = handshake_without_final_ack(base);
    client.handle_timeout(base).unwrap();
    drain(&mut client);
    run("lost ACK", base, client, server);
}

cargo test --test <file> -- --nocapture on main (0c196c2):

control (ACK on time): 0 further client retransmissions, then Timeout(Connect) at 120s
late ACK: 3 further client retransmissions, then Timeout(Handshake) at 34s
lost ACK: 4 further client retransmissions, then Timeout(Handshake) at 34s

On #162 (73cbfe1):

control (ACK on time): 0 further client retransmissions, client still alive at 120s
late ACK: 3 further client retransmissions, then Timeout(Handshake) at 31s
lost ACK: 4 further client retransmissions, then Timeout(Handshake) at 41s

The Timeout(Connect) in the main control run is a separate problem (the overall handshake deadline is not disabled after completion), which #162 fixes. Exact times vary between runs because the RTO jitter is random.

Root cause

  • flight_resend overwrites entry.send_seq with the record number of each new transmission (engine.rs#L727, #L747).
  • process_ack marks an entry acknowledged only when the ACK names that latest record number (engine.rs#L1390). ACKs for any earlier transmission are silently ignored, so the "all epoch-2 records ACKed" check never passes and the flight timer keeps running.
  • On the server, a retransmitted client final flight triggers flight_resend("dupe triggers resend") of the saved final ACK (engine.rs#L393), whose record numbers come from the first transmission. The retransmitted records are then dropped as old duplicates (engine.rs#L400) before they are added to the ACK list, so the server never ACKs the newer record numbers.

As a diagnostic I patched flight_resend to stop renumbering, so the client keeps matching its first transmission. The repro then gives late ACK: 0 further client retransmissions and lost ACK: 1 further client retransmissions, and the client stays connected. That patch is not a fix: it breaks matching of ACKs that name later transmissions.

Expected behavior (RFC 9147)

  • §7.2: "When an implementation receives an ACK, it SHOULD record that the messages or message fragments sent in the records being ACKed were received and omit them from any future retransmissions. [...] Once all the messages in a flight have been acknowledged, the implementation MUST cancel all retransmissions of that flight. Implementations MUST treat a record as having been acknowledged if it appears in any ACK".
  • §5.8.1: "when in the FINISHED state, the server MUST respond to retransmission of the client's final flight with a retransmit of its ACK." dimpl's server already does this; the client has to be able to use it.
  • §7.1: "Implementations MAY acknowledge the records corresponding to each transmission of each flight or simply acknowledge the most recent one."

Suggested fix

  1. Client (required): keep the record numbers of every transmission of each saved flight entry (bounded, e.g. a small fixed-capacity list per entry), and treat an ACK that names any of them as acknowledging the entry. KeyUpdate flights go through the same flight_saved_records / process_ack matching, so the fix should cover them too.
  2. Server (optional hardening, allowed by §7.1): when a retransmitted client final flight arrives after completion, also ACK the newly received record numbers. This lets dimpl servers work with clients that, like dimpl today, only match their latest transmission.

Acceptance tests

  • Late ACK: a DTLS 1.3 client that receives the ACK for its first final-flight transmission after retransmitting stops retransmitting and stays connected.
  • Lost ACK: with the first final ACK dropped, the client stops after the server answers one retransmission, and stays connected. Cover new_13 and new_auto resolving to 1.3.
  • An ACK naming a mix of earlier and latest transmissions marks every named record acknowledged, and selective retransmission of the rest still works.
  • The same late/lost ACK cases for a KeyUpdate flight.

Notes

Found while reviewing #162. The bug is independent of that PR and present on main.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions