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

From: Kyle Rose via Datatracker <[email-masked]>
Date: Tue, 17 Aug 2021 09:29:57 -0700

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
[email-masked] 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".

---

From: Ben Schwartz <[email-masked]>
Date: Wed, 18 Aug 2021 13:10:36 -0400

-last-call

Thanks for the review, Kyle.  I've opened a PR to track proposed changes
based on your comments: https://github.com/MikeBishop/dns-alt-svc/pull/336

>  attack surface for no good reason.

I assume you're referring to the case of an AliasMode record whose
TargetName does not have a SVCB record.  draft-07 explains this scenario as
follows:

   SVCB-optional
   clients SHALL append an alternative endpoint consisting of the final
   value of $QNAME, the authority endpoint's port number, and no
   SvcParams, to the list of alternative endpoints, which is attempted
   before falling back to non-SVCB connection modes.  This ensures that
   SVCB-optional clients will make use of an AliasMode record whose
   TargetName has A and/or AAAA records but no SVCB records.

This is valuable in the HTTPS context, where it allows a CDN customer to
alias their apex domain to an unmodified CDN.

I do not believe this behavior introduces any additional security issues,
as a SVCB-optional client can always be induced to fall back to a standard
origin connection by denying it access to the SVCB records or interfering
with connections to the alternative endpoints.

>  that are protocol-independent.

I've attempted to reformulate this section a bit to emphasize that we are
talking about additional restrictions that apply only when "cryptographic
protection" is in place.

>  should be explicitly noted under Security Considerations.

I'm not sure what you're referring to here, but it seems like the problem
might be that we have an unstated assumption that SVCB does not attempt to
protect clients from cross-transport downgrade attacks (e.g. QUIC to
TLS/TCP).  I've proposed some text to make that assumption explicit.

>  format that are unrecognized by a particular implementation..." But this

I've adjusted the phrasing here to talk about "arbitrary keys", since there
is in fact no requirement that they key be unrecognized.

>  "Difference from the SRV RR type".

True.  Removed.