Observation
In Server::handle_inner at ntp-proto/src/server.rs:228, NtpPacket::deserialize(message, self.keyset.as_ref()) is invoked for inbound messages that have already been classified as ServerResponse::Deny by the denylist policy gate above it. For NTS-shaped packets from denylisted clients, this performs cookie decryption work whose result is then discarded by the existing branch:
Err(PacketParsingError::DecryptError(packet)) => {
// Don't care about decryption errors when denying anyway
if action != ServerResponse::Deny {
action = ServerResponse::NTSNak;
reason = ServerReason::InvalidCrypto;
}
(packet, None)
}The comment shows you already know about this; I wanted to ask whether the asymmetry is intentional (i.e., the small wasted work is the better tradeoff vs. complicating the parse path) or whether it would be welcome to skip parsing entirely on the deny branch.
The asymmetry would be relevant to denial-of-service amplification only at high rates of malformed NTS-shaped traffic from denylisted sources, where the server pays cookie-decode cost per packet. I don't have measurements of how much that matters in practice.
Caveat
I'm not opening a PR alongside this issue because the natural fix (a header-only deserialize path on the deny branch) would touch NtpPacket's public surface and that's a design call I'd rather defer to you. If you'd like a PR exploring a specific approach, happy to follow up.
Provenance
This came out of an automated defensive code-review sweep; the observation is mine, not from an exploit. Closing this as "intentional" is a totally reasonable answer.
Observation
In
Server::handle_inneratntp-proto/src/server.rs:228,NtpPacket::deserialize(message, self.keyset.as_ref())is invoked for inbound messages that have already been classified asServerResponse::Denyby the denylist policy gate above it. For NTS-shaped packets from denylisted clients, this performs cookie decryption work whose result is then discarded by the existing branch:The comment shows you already know about this; I wanted to ask whether the asymmetry is intentional (i.e., the small wasted work is the better tradeoff vs. complicating the parse path) or whether it would be welcome to skip parsing entirely on the deny branch.
The asymmetry would be relevant to denial-of-service amplification only at high rates of malformed NTS-shaped traffic from denylisted sources, where the server pays cookie-decode cost per packet. I don't have measurements of how much that matters in practice.
Caveat
I'm not opening a PR alongside this issue because the natural fix (a header-only deserialize path on the deny branch) would touch
NtpPacket's public surface and that's a design call I'd rather defer to you. If you'd like a PR exploring a specific approach, happy to follow up.Provenance
This came out of an automated defensive code-review sweep; the observation is mine, not from an exploit. Closing this as "intentional" is a totally reasonable answer.