Re: [tram] Publication has been requested for draft-ietf-tram-stun-pmtud-07

[Marc Petit-Huguenin <[e-mail]>]

Hi Spencer,

Thank you for the review.  Please see inline.

On 05/08/2018 08:31 PM, Spencer Dawkins at IETF wrote:
> Dear Authors,
> 
> On Fri, Apr 20, 2018 at 4:32 AM, Gonzalo Camarillo <
> [e-mail]> wrote:
> 
>> Gonzalo Camarillo has requested publication of
>> draft-ietf-tram-stun-pmtud-07 as Proposed Standard on behalf of the TRAM
>> working group.
>>
>> Please verify the document's state at https://datatracker.ietf.org/
>> doc/draft-ietf-tram-stun-pmtud/
> 
> 
> This document was fairly clear to me. I did have some questions I'd like to
> work through before requesting Last Call, but none of them are showstoppers
> - they shouldn't even slow the show down much.
> 
> Thanks,
> 
> Spencer
> 
> I've been wrong before, but I'm thinking that
> 
>   Not all UDP-based protocols implement the Path MTU discovery
>    mechanism described in [RFC4821].
> 
> is an understatement - I'm thinking of several widely used UDP-based
> protocols that try to use heuristics including just pick a low MTU size, or
> switch to TCP transport, or some other workaround. This statement seems to
> say that most UDP-based protocols do RFC 4821. Is it fair to say that many
> UDP-based protocols don't do RFC 4821 PMTU discovery?

New text:

   Many UDP-based protocols do not implement the Path MTU discovery
   mechanism described in [RFC4821].  These protocols can make use of
   [...]

> 
> For this section,
> 
> 3.  Terminology
> 
>    The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>    "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
>    document are to be interpreted as described in [RFC2119].  When these
>    words are not in ALL CAPS (such as "must" or "Must"), they have their
>    usual English meanings, and are not to be interpreted as RFC 2119 key
>    words.
> 
> you might consider replacing it with the corresponding paragraph in
> https://tools.ietf.org/html/rfc8174 - I'm seeing an increasing number of
> comments on IESG ballots suggesting this, and it's likely that the new BCP
> text will more familiar to most readers over time.

Done.

> 
> This may be a dumb question, but in this text,
> 
>    Implementations supporting this specification MUST implement the
>    server side of both the Simple Probing mechanism (Section 4.1) and
>    the Complete Probing mechanism (Section 4.2).
> 
>    Implementations supporting this specification MUST implement the
>    client side of the Complete Probing mechanism.  They MAY implement
>    the client side of the Simple Probing mechanism.
> 
> does this mean "Server implementations" and "Client implementations"?

No, a compliant implementation must support both the client and the server side so to discover the PMTU in both directions.  The only choice is to not support the client side of the simple probing mechanism.

> 
> I'm a bit confused about this text:
> 
>    Some application layer protocols may already have a way of
>    identifying each individual UDP packet, in which case these
>    identifiers SHOULD be used in the IDENTIFIERS attribute of the Report
>    Response.  While there are other possible packet identification
>    schemes, this document describes two different ways to identify a
>    specific packet.
> 
>    In the first packet identification mechanism, the server computes a
>    checksum over each packet received and sends back to the sender the
>    list of checksums ordered chronologically.  The client compares this
>    list to its own list of checksums.
> 
>    In the second packet identification mechanism, the client prepends
>    the UDP data with a header that provides a sequence number.  The
>    server sends back the chronologically ordered list of sequence
>    numbers received that the client then compares with its own list.
> 
> It wasn't clear to me from this text whether these two packet
> identification mechanism are descriptions of what applications do if they
> DON'T already have a way of identifying packets. If that's what's intended,
> perhaps
> 
>   Some application layer protocols may already have a way of
>    identifying each individual UDP packet, in which case these
>    identifiers SHOULD be used in the IDENTIFIERS attribute of the Report
>    Response.  While there are other possible packet identification
>    schemes, this document describes two different ways to identify a
>    specific packet, when no application layer protocol-specific
> identification
> 
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>    mechanism is available.
>    ^^^^^^^^^^^^^^^^^^^^^^
> ?

Applied.

> 
> When I read this text,
> 
>   Then the client waits half the RTO, if it is known, or 250 ms after
>    sending the last Probe Indication and then sends the Report Request
>    to the server over UDP.
> 
> I was confused - isn't there's a 500-ms default RTO value in 4.1.1? Is this
> different from
> 
>   Then the client waits half the RTO after
>    sending the last Probe Indication and then sends the Report Request
>    to the server over UDP.
> 
> or am I missing your point?

Replaced by your text.

> 
> In this text,
> 
>   The algorithm used to calculate the checksum is similar to the
>    algorithm used for the FINGERPRINT attribute (i.e., the CRC-32 of the
>    payload XOR'ed with the 32-bit value 0x5354554e).
> 
> is there a reference you could provide for the "algorithm used for the
> FINGERPRINT attribute" here? I'm pretty sure I could find the right one,
> but googling for "STUN FINGERPRINT checksum" is a lot less helpful than I
> expected, and making sure that you understand what something is "similar
> to" seems important if that's going to be helpful information for
> implementers.

Added reference to ITU V42.

> 
> If using authentication in
> 
>   UDP-based protocols that want to use any of these mechanisms,
>    including the PMTUD-SUPPORTED attribute, to signal PMTUD capabilities
>    MUST ensure that it cannot be used to launch an amplification attack.
>    For example, using authentication can ensure this.
> 
> is only one way to ensure prevention of amplification attacks, is there any
> guidance or reference you could point to that would help implementers
> evaluate other approaches?

It's a good question.  I have to think about that.

> 
> Actually, after reading
> 
>   The Probe Request or Indication that are used to implicitly signal
>    probing support in the reverse direction MUST be authenticated to
>    prevent amplification attacks.
> 
> I wonder why authentication isn't a MUST for UDP-based protocols using
> STUN-PMTUD, but that's just me wondering ...

When STUN-PMTUD is used as an add-on to an existing protocol, it is difficult to impose to make the whole protocol authenticated.  When it is for a new protocol I agree that it should be secure, but then it is up to the protocol designer to do that (and to the IESG to enforce it).

> 
> I was a bit confused by
> 
>   This specification defines two new STUN methods and two new STUN
>    attributes.  IANA added these new protocol elements to the "STUN
>    Parameters Registry" created by [RFC5389].
> 
> because they haven't been added to the registry yet, right?
> 

Right.  I changed the whole IANA section to look like the one in STUNBis.

-- 
Marc Petit-Huguenin
Email: [e-mail]
Blog: https://marc.petit-huguenin.org
Profile: https://www.linkedin.com/in/petithug