[Tsv-art] Tsvart last call review of draft-ietf-dnsop-svcb-https-07

[Kyle Rose via Datatracker <[e-mail]>]

Reviewer: Kyle Rose
Review result: Ready with Issues

This document has been reviewed as part of the transport area review team's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the document's
authors and WG to allow them to address any issues raised and also to the IETF
discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
[e-mail] if you reply to or forward this review.

This document is Ready with some minor issues and nits.

Issues:

 * 3: Is there a downgrade attack vector resulting from the implicit fallback
 to the alternative endpoint? I wonder if a better approach is to have
 SVCB-aware clients fail when the SVCB chain is incomplete, with SVCB-unaware
 clients ignoring all such records and implementing legacy rendezvous. The
 question I'd use to justify this change is: is there any particular reason
 (supporting partial implementation, incremental/partial activation, etc.) to
 support such fallback? If not, then it seems like it introduces complexity and
 attack surface for no good reason.

 * 3.1: The downgrade issue (obviously) isn't limited to clients with
 cryptographically-protected connections to resolvers. The issue is more that
 such connections are reliable and integrity-protected, so if they fail it's
 the result of either misconfiguration or an attack. Traditional UDP-based DNS
 will frequently and naturally encounter transport failures, notably dropped
 packets, and of course active attackers can manipulate cleartext TCP at will.
 That said, DNSSEC validation failures should result in connection abandonment
 irrespective of the type of client<->resolver communication. It may be that
 this section should have two subsections, one for transport related failures
 that apply only to secure connections, and one for other classes of failures
 that are protocol-independent.

 * 5.1: The prescribed behavior does not prevent all classes of downgrade
 attacks, particularly those that might result from delaying SVCB responses to
 force fallback to a less secure application-layer protocol, and seems to
 directly contradict the recommendation in section 3.1. This may be a practical
 necessity to keep the internet working, albeit in a degraded state, in the
 presence of anomalous (but to-be-expected) network behavior, and if so this
 should be explicitly noted under Security Considerations.

Nits:

 * 2.1: What's here might be idiomatic for DNS RFCs, but it seems like
 "Unrecognized keys" is insufficiently precise. The problem is an
 implementation that does not have a registered key number -> presentation name
 mapping for a particular number. Maybe something like "Key numbers in the wire
 format that are unrecognized by a particular implementation..." But this might
 be way too pedantic.

 * C.1: "SVCB records use 16 bit for SvcPriority for consistency with SRV and
 other RR types that also use 16 bit priorities" does not describe a
 "Difference from the SRV RR type".