Re: [MBONED] Benjamin Kaduk's No Objection on draft-ietf-mboned-driad-amt-discovery-11: (with COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Thu, Dec 19, 2019 at 06:54:27PM +0000, Holland, Jake wrote:
> Hi Ben,
> 
> Thanks for the review.  Responses below:
> 
> On 2019-12-18, 21:57, "Benjamin Kaduk via Datatracker" <[e-mail]> wrote:
> > Section 1
> > 
> >    This document updates Section 5.2.3.4 of [RFC7450] by adding a new
> >    extension to the relay discovery procedure.
> > 
> > I know that there is not a universal usage of "updates", but note that
> > in other protocols with similar scenarios (multiple possible discovery
> > methods), the core protocol document is not always in an "Updated by"
> > relationship with the new discovery methods.  (That said, there seem to
> > be plenty of other ways in which this document updates RFC 7540, so this
> > particular instsance isn't a big deal.)
> 
> Now that you mention it, this phrasing does seem a bit awkward.  As Eric
> mentioned, this is not the only update.  I think maybe this is slightly
> better:
> 
> NEW:
>    This document extends the relay discovery procedure described in
>    Section 5.2.3.4 of [RFC7450].

I concur.

> > Section 1.2.2
> > 
> >    |     L flag | The "Limit" flag described in Section 5.1.1.4 of     |
> >    |            | [RFC7450]                                            |
> > 
> > nit: s/5.1.1.4/5.1.4.4/
> > 
> >    The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
> >    "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
> >    "OPTIONAL" in this document are to be interpreted as described in
> >    [RFC2119] and [RFC8174] when, and only when, they appear in all
> >    capitals, as shown here.
> >
> > nit: this is not quite the prescribed form from RFC 8174.
> 
> Thanks for catching these!  Fixes upcoming.
> 
> > Section 2.2
> > 
> > Perhaps it's worth a brief note that the UDP unicast tunnel is over IPv6
> > even though the multicast traffic being conveyed is native IPv4 in both
> > multicast networks?
> 
> Sure, good point.  Added this at the bottom of the section:
> 
> NEW:
>    Note that the address family of the AMT tunnel is independent of the
>    address family for the multicast traffic.

(Maybe this is OBE after Adam's suggestions?)

> > Section 2.3.2
> > 
> > I don't think I understand what "connecting to Figure 6" means.
> 
> Ah, sorry.  You're right, this is not terribly clear.  A hopefully-
> improved rephrasing:
> 
> OLD:
>       A related motivating example in the sending-side network is
>       provided by considering a sender that needs to instruct the
>       gateways on how to select between connecting to Figure 6 or
>       Figure 7 (from Section 3.2), in order to manage load and failover
>       scenarios in a manner that operates well with the sender's
>       provisioning strategy for horizontal scaling of AMT relays.
> 
>       In this example about the sending-side network, the precedence
>       field described in Section 4.2.1 is a critical method of control
>       so that senders can provide the appropriate guidance to gateways
>       during the discovery process.
> 
> NEW:
>       A related motivating example is provided by considering a sender
>       whose traffic can be forwarded by relays in a network like
>       Figure 6 in Section 3.2.1, and also by relays in a network like
>       Figure 7 in Section 3.2.2, with different cost and scalability
>       profiles for the different options.
> 
>       In this example about the sending-side network, the precedence
>       field described in Section 4.2.1 is a critical method of control
>       so that senders can provide the appropriate guidance to gateways
>       during the discovery process, in order to manage load and failover
>       scenarios in a manner that operates well with the sender's
>       provisioning strategy for horizontal scaling of AMT relays.
> 
> Does that work?

I think so; thanks!

> >    Among relay addresses that have an equivalent preference as described
> >    above, a Happy Eyeballs algorithm for AMT SHOULD use use the
> >    Destination Address Selection defined in Section 6 of [RFC6724].
> >
> >    Among relay addresses that still have an equivalent preference after
> >    the above orderings, a gateway SHOULD make a non-deterministic choice
> > 
> > side note: I think that the way RFC 6724 is written (as a series of
> > comparison rules), it doesn't have an "are equivalent preference"
> > option, just a "leave the order unchanged" one.  But that's probably a
> > useless pedantic distinction and I can't see an actionable change that
> > would result from it.
> 
> At least having this quoted to me made me notice the "use use" nit.
> 
> I'm not sure I fully agree though; to me Section 6 of RFC 6724 phrases
> the rules like:
> "   Rule 1: Prefer same address."
> 
> To me, I think the advice in the driad draft falls under "If the eight
> rules fail to choose a single address, the tiebreaker is
> implementation-specific." clause from 6724, but specifies some rules
> for the tiebreaker.
> 
> I think I agree that it's a pedantic point that doesn't need a change,
> but if this makes you think of any ideas that would improve the clarity,
> suggestions are of course welcome.  I haven't made a change here yet.
> 
> > Section 2.7
> >
> >    The DNS query functionality is expected to follow ordinary standards
> >    and best practices for DNS clients.  A gateway MAY use an existing
> >    DNS client implementation that does so, and MAY rely on that client's
> >    retry logic to determine the timeouts between retries.
> >
> > The first part of this seems to be a duplication of Section 2.4.2, but
> > the latter part (and following paragraph/sentences) diverges.  There's
> > probably some room for consolidation and/or harmonization of procedures
> > here.
> 
> That occurred to me briefly, in the form of writing a section that said
> something brief and referring to it.
> 
> I ended up deciding it's short enough it's just better to highlight right
> next to the couple of key requirements, so that anybody looking at that
> point specifically will see it.  I'm open to updating if you have strong
> feelings about this, but I thought the gains marginal for this case, and
> offset by the marginal increase in the structural complexity of the doc,
> such that for just the 2 cases it didn't seem worthwhile to me.

No strong feelings; I just wanted to check that it was known.

> > Section 4.2.4
> > 
> > (Presumably we ignore entires with unrecognized 'type'; I forget whether
> > this is standard DNS usage or we should mention it explicitly, though.)
> 
> A fair point, I have no objection to adding some text here.
> 
> NEW:
>    RRs with an undefined value in the Type field SHOULD NOT be
>    considered by receiving gateways for AMT relay discovery.
> 
> (I got a little shy about "MUST be ignored" phrasing after seeing it
> quoted, right where a packet to be ignored was abandoned in the linux
> kernel without logging or producing any kind of error or warning
> dmesg output that might have saved me a day of debugging once when I
> tried to use a non-link-local interface address for joining an IPv6
> (S,G) with MLD, IIRC.)

Thanks, and understood; that sounds like it was not a fun debugging
exercise.

> > Section 5
> > 
> > Is there any guidance to give to the Designated Experts in addition to
> > the default rules from RFC 8126?
> 
> Nothing occurs to me, I'd expect the default guidance to be sufficient
> here.  I think it already sort of includes "please don't approve obviously
> terrible ideas", which is the only thing I really want. 
> 
> > Section 6
> >
> > I like that the relay-discovery precedence rules minimize the
> > opportunity for an attacker to disrupt discovery and try to force a
> > different relay to be used (whether to afford an opportunity to tamper
> > with the traffic going to a target recipient or other reasons).  Since
> > we are updating the generic AMT relay discovery procedure, we could
> > reasonably mention that (and the generic risks with discovery procedures
> > that involve attempting to contact a relay and failing over if a timely
> > response does not appear); RFC 7450's Section 6.2 provides only a
> > minimal mention.  That said, most of the security considerations
> > relevant here seem to be ones that apply to stock AMT, and are tolerably
> > covered in RFC 7450.  I'm a little surprised that Happy Eyeballs doesn't
> > cover this sort of disruption in its security considerations; I was
> > going to suggest referencing that as well.
> 
> Not sure if you're asking for an edit here?  I think I'm reading this
> as "it would be great if this were slightly better, but it seems adequate"?

The potential edit (not quite a suggestion) was to note that when we're
using a relay-discovery procedure that involves "try one, and fall over to
another one if it doesn't work", an attacker can disrupt one (or more)
attempts early in the procedure to try to force a particular relay to be
used.  (That might be with the ultimate goal to intercept traffic, to alter
traffic, to gain the ability to selectively drop traffic, or otherwise, but
it's not terribly interesting for this point.)  Furthermore, the particular
procedure that we've chosen reduces the ability of an attacker to do so,
since we try the relays that are "closer" (and thus harder to attack)
first.

(The other things that we might want to say, in terms of the usual sorts of
things we say in security considerations sections, apply equally to stock
AMT, and thus probably don't need to be repeated here.)

> > We briefly mention active/active failover in Section 2.3.3, and such a
> > scheme poses some risk of (additional) traffic duplication around a
> > failover event, but (1) that can happen with UDP anyway, so it will
> > already be handled, and (2) it's a pretty tenuous hook to say that we
> > need to talk about the security considerations of such a situation.
> > 
> > Also on the borderline of worth mentioning, an attacker might attempt to
> > force a gateway to repeatedly go through the relay discovery process; I
> > don't think this process is sufficiently resource-intensive that it
> > would be a usable DoS attack, though, so there's not really much there
> > other than the generic "disruption" that is already covered in 7450.
> 
> I agree, I think.  I concluded I didn't think these added enough to be
> worth pulling any focus away from the points that were mentioned, so
> I left it out.

Sure.

> > Section 6.2
> > 
> > Even though not all of the listed mechanisms are currently specified for
> > recursive-to-authoritative queries, I think it's fine to list them here,
> > as they are expected to become defined in the future and would make
> > sense as options, when available.
> > 
> >    response from the trusted server.  The connection to the trusted
> >    server can use any secure channel, such as with a TSIG [RFC2845] or
> >    SIG(0) [RFC2931] channel, a secure local channel on the host, DNS
> >    over TLS [RFC7858], DNS over HTTPS [RFC8484], or some other mechanism
> >    that provides authentication of the RR.
> > 
> > I don't think that it's really "authentication" that we're providing for
> > the RR itself; what we want is more of "source authentication" for the
> > provenance of the RR (and integrity protection for its contents).
> 
> I'm fine with that wording if that makes good sense to security experts:
> 
> NEW:
>          ... or some other mechanism
>     that provides source authentication for the provenance of the RR and
>     integrity protection for its contents.

I think that's okay, though more verbose than we sometimes see.  (I think
just "provides source authentication" is a common shorthand.)

> >    If an AMT gateway accepts a maliciously crafted AMTRELAY record, the
> >    result could be a Denial of Service, or receivers processing
> >    multicast traffic from a source under the attacker's control.
> > 
> > Even for an honest AMTRELAY record, isn't there a chance that the
> > multicast traffic's contents could also be modified or injected by the
> > attacker?
> 
> Yes, but this is generic to AMT and is mentioned in the 2nd paragraph of
> Section 6.2 of RFC 7450.  I guess it's a fair point that it's not clear
> the warning there is entirely adequate--should I try to write a section
> about this on the grounds that I'm updating 7450 and making it more
> widely deployable?

I don't insist on it, so maybe give it a quick go and see if it looks good.

> > Section 8.2
> > 
> > Arguably RFC 8499 would be normative, since we defer to its definition
> > of FQDN, but I am not really very concerned about it.
> 
> Sure, I can do that.
> 
> > Appendix A
> > 
> >      $ ./translate.py amtrelays.example.com
> >      24
> >      09616d7472656c617973076578616d706c6503636f6d
> >    <CODE ENDS>
> > 
> >    The length and the hex string for the domain name
> >    "amtrelays.example.com" are the outputs of this program, yielding a
> >    length of 22 and the above hex string.
> 
> > I'm having a hard time parsing this in a consistent way where the
> > yielded length is 22 and the literal command output is 24.
> 
> Oops, I updated the program in an earlier version and didn't update the
> explanation properly, thanks for catching this.
> 
> NEW:
>    The length of the RData and the hex string for the domain name
>    "amtrelays.example.com" are the outputs of this program.
> 
>    22 is the length of the wire-encoded domain name, so 2 was added to
>    that value (1 for the precedence field and 1 for the combined D-bit
>    and relay type fields) to get the full length 24 of the RData.  For
>    the 2 octets ahead of the domain name, we encode the precedence,
>    D-bit, and relay type fields, as described in Section 4.
> 
>    This results in a zone file entry like this:
>    ...

Thanks!

-Ben