Re: [DNSOP] Tsvart last call review of draft-ietf-dnsop-svcb-https-07

[Ben Schwartz <[e-mail]>]

-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

On Tue, Aug 17, 2021 at 12:29 PM Kyle Rose via Datatracker <[e-mail]>
wrote:

> 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.
>

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.

 * 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.
>

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.

 * 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.
>

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.

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.
>

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


>  * 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".
>

True.  Removed.