Skip to main content

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 revision No specific revision (document currently at 12)
Type Last Call Review
Team Transport Area Review Team (tsvart)
Deadline 2021-08-19
Requested 2021-08-05
Authors Benjamin M. Schwartz , Mike Bishop , Erik Nygren
I-D last updated 2021-08-17
Completed reviews Genart Last Call review of -07 by Dale R. Worley (diff)
Artart Last Call review of -07 by Cullen Fluffy Jennings (diff)
Opsdir Last Call review of -07 by Joe Clarke (diff)
Tsvart Last Call review of -07 by Kyle Rose (diff)
Intdir Telechat review of -08 by Carlos Pignataro (diff)
Dnsdir Last Call review of -12 by Matt Brown
Assignment Reviewer Kyle Rose
State Completed
Request Last Call review on draft-ietf-dnsop-svcb-https by Transport Area Review Team Assigned
Posted at https://mailarchive.ietf.org/arch/msg/tsv-art/KFFF1V89n6KFm2lMZdVNmx3ABIQ
Reviewed revision 07 (document currently at 12)
Result Ready w/issues
Completed 2021-08-17
review-ietf-dnsop-svcb-https-07-tsvart-lc-rose-2021-08-17-00
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".