Last Call Review of draft-ietf-dnsop-svcb-https-07
review-ietf-dnsop-svcb-https-07-tsvart-lc-rose-2021-08-17-00

Request Review of draft-ietf-dnsop-svcb-https
Requested rev. no specific revision (document currently at 08)
Type Last Call Review
Team Transport Area Review Team (tsvart)
Deadline 2021-08-19
Requested 2021-08-05
Authors Benjamin Schwartz, Mike Bishop, Erik Nygren
Draft last updated 2021-08-17
Completed reviews Genart Last Call review of -07 by Dale Worley (diff)
Artart Last Call review of -07 by Cullen Jennings (diff)
Opsdir Last Call review of -07 by Joe Clarke (diff)
Tsvart Last Call review of -07 by Kyle Rose (diff)
Assignment Reviewer Kyle Rose 
State Completed
Review review-ietf-dnsop-svcb-https-07-tsvart-lc-rose-2021-08-17
Posted at https://mailarchive.ietf.org/arch/msg/tsv-art/KFFF1V89n6KFm2lMZdVNmx3ABIQ
Reviewed rev. 07 (document currently at 08)
Review result Ready with Issues
Review completed: 2021-08-17

Review
review-ietf-dnsop-svcb-https-07-tsvart-lc-rose-2021-08-17

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 tsv-art@ietf.org 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".