cloudflare / cloudflare/quiche

pkt_num_len can return 1 byte, leading to AEAD decryption failures under packet reorder

Open
#2,448 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
11.8k
Forks
1.1k
Avg merge
21h 9m
Merged PRs (30d)
6

Description

### Summary

`quiche/src/packet.rs::pkt_num_len` will choose a 1-byte truncated packet number when fewer than 128 packets are unacked. RFC 9000 §17.1 permits this, but it is unsafe in practice: if the receiver observes more than 128 packets reordered, the entire valid range of a 1-byte truncation collapses, and the decoded full packet number lands on the wrong candidate. The AEAD nonce is derived from the full packet number, so the receiver fails to decrypt an otherwise-good packet.

### Source

[`quiche/src/packet.rs::pkt_num_len`](https://github.com/cloudflare/quiche/blob/master/quiche/src/packet.rs#L569-L575):

```rust
pub fn pkt_num_len(pn: u64, largest_acked: u64) -> usize {
let num_unacked: u64 = pn.saturating_sub(largest_acked) + 1;
let min_bits = u64::BITS - num_unacked.leading_zeros() + 1;
min_bits.div_ceil(8) as usize // returns 1 when num_unacked < 128
}
```

The reference Go implementation, [quic-go](https://github.com/quic-go/quic-go), refuses 1-byte truncation for exactly this reason. From [`internal/protocol/packet_number.go#L41-L42`](https://github.com/quic-go/quic-go/blob/master/internal/protocol/packet_number.go#L41-L42):

> // it never chooses a PacketNumberLen of 1 byte, since this is too short under certain circumstances

### Field observation

Hit in production at https://viatrail.app — Cloudflare R2-served HTTP/3 tile fetches stalling on mobile. qlog shows clusters of `packet_dropped` with `trigger=payload_decrypt_error` on 1-RTT packets, with decoded PN offset from actual PN by exactly 256·*k* — the precise signature of 1-byte truncation ambiguity.

### Reproducer

https://github.com/dave/quiche-bug — Docker-based server (stock or patched via `--build-arg APPLY_PATCH=0|1`) plus Go HTTP/3 client using quic-go.

| Server | AEAD failures (90s, 8 workers) |
|---|---|
| Stock | **6** |
| With `pkt_num_len.max(2)` | **0** |

### Proposed fix

Floor `pkt_num_len` at 2 bytes (1-line change). PR to follow.

Cost: 1 byte per outgoing 1-RTT packet during the brief window where `num_unacked < 128`. quic-go has shipped this trade-off since 2017.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.