Re: [tram] Mirja Kühlewind's Yes on draft-ietf-tram-stunbis-16: (with COMMENT)

Marc Petit-Huguenin <marc@petit-huguenin.org> Sun, 22 April 2018 18:39 UTC

Return-Path: <marc@petit-huguenin.org>
X-Original-To: tram@ietfa.amsl.com
Delivered-To: tram@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 380BF126CBF; Sun, 22 Apr 2018 11:39:53 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.108
X-Spam-Level:
X-Spam-Status: No, score=-1.108 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RDNS_NONE=0.793, SPF_PASS=-0.001] autolearn=no autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id s_pZ_fXHLx7z; Sun, 22 Apr 2018 11:39:51 -0700 (PDT)
Received: from implementers.org (unknown [IPv6:2001:4b98:dc0:45:216:3eff:fe7f:7abd]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 59E17120227; Sun, 22 Apr 2018 11:39:51 -0700 (PDT)
Received: from [IPv6:2601:648:8301:730f:3134:3153:e7ee:f8f7] (unknown [IPv6:2601:648:8301:730f:3134:3153:e7ee:f8f7]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "Marc Petit-Huguenin", Issuer "implementers.org" (verified OK)) by implementers.org (Postfix) with ESMTPS id 8D29AAE844; Sun, 22 Apr 2018 20:39:47 +0200 (CEST)
To: =?UTF-8?Q?Mirja_K=c3=bchlewind?= <ietf@kuehlewind.net>, The IESG <iesg@ietf.org>
Cc: draft-ietf-tram-stunbis@ietf.org, Gonzalo.Camarillo@ericsson.com, Tolga Asveren <tasveren@rbbn.com>, tram-chairs@ietf.org, tram@ietf.org
References: <152390695800.19624.250040937113641569.idtracker@ietfa.amsl.com>
From: Marc Petit-Huguenin <marc@petit-huguenin.org>
Message-ID: <5d8e3d9e-46bd-57ba-5c80-ff414ef2aca5@petit-huguenin.org>
Date: Sun, 22 Apr 2018 11:39:40 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0
MIME-Version: 1.0
In-Reply-To: <152390695800.19624.250040937113641569.idtracker@ietfa.amsl.com>
Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="oFB9SoYimTa6EqYUex9JW4EzKuooVfeVd"
Archived-At: <https://mailarchive.ietf.org/arch/msg/tram/67hX4HEyN6yOBnAZcJGQ8HTh1rY>
Subject: Re: [tram] =?utf-8?q?Mirja_K=C3=BChlewind=27s_Yes_on_draft-ietf-tram?= =?utf-8?q?-stunbis-16=3A_=28with_COMMENT=29?=
X-BeenThere: tram@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: "Discussing the creation of a Turn Revised And Modernized \(TRAM\) WG, which goal is to consolidate the various initiatives to update TURN and STUN." <tram.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tram>, <mailto:tram-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tram/>
List-Post: <mailto:tram@ietf.org>
List-Help: <mailto:tram-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tram>, <mailto:tram-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 22 Apr 2018 18:39:53 -0000

Hi Mirja,

Thanks for the review, please see my responses inline.

On 04/16/2018 12:29 PM, Mirja Kühlewind wrote:
> Mirja Kühlewind has entered the following ballot position for
> draft-ietf-tram-stunbis-16: Yes
> 
[...]
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> 1) Maybe it would make sense to refer informationally to
> draft-ietf-tram-stun-pmtud in section 6.1?

Done, here's the new last paragraph:

   STUN provides no ability to handle the case where the request is
   under the MTU but the response would be larger than the MTU.  It is
   not envisioned that this limitation will be an issue for STUN.  The
   MTU limitation is a SHOULD, and not a MUST, to account for cases
   where STUN itself is being used to probe for MTU characteristics
   [RFC5780].  See also [I-D.ietf-tram-stun-pmtud] for a framework that
   use STUN to add Path MTU Discovery to protocols that lack one.
   Outside of this or similar applications, the MTU constraint MUST be
   followed.

> 
> 2)  sec 6.2.2: This sentence seems to assume that only one request is sent per
> TCP connection/each request sends out after a new SYN: "However, for a
> request/response transaction, if the client has not
>    received a response by Ti seconds after it sent the SYN to establish
>    the connection, it considers the transaction to have timed out.“
> i don’t think that assumption would be correct. Maybe rephrase this sentence…?

Right,  the Ti timer should start when the unique request per transaction is sent, not when the connection is established.  Here's the new text:

   Reliability of STUN over TCP and TLS-over-TCP is handled by TCP
   itself, and there are no retransmissions at the STUN protocol level.
   However, for a request/response transaction, if the client has not
   received a response by Ti seconds after it sent the request message,
   it considers the transaction to have timed out.  Ti SHOULD be
   configurable and SHOULD have a default of 39.5s.  This value has been
   chosen to equalize the TCP and UDP timeouts for the default initial
   RTO.


> 
> 3) Also section 6.2.2: Should the client send keep-alives if a connection is
> hold open but currently not used? If not, I guess further recommendation is
> needed to address the case where a transmission fails because an existing idle
> TCP connection was used that doesn’t work anymore. In this case, I'd say the
> recommendation would be to send a RST and try to open a new TCP connection.

I feel that keep-alives recommendations are dependent on the specificity of a STUN Usage, and should be left to the document specifying it.  For example in the TURN case there is probably enough frequent messages with the allocation and permission refresh to not mandate any kind of keep-alive. OTOH for Basic STUN Usage (NAT discovery) a client will probably immediately close the TCP connection after retrieving the NAt transport address.

So I added TCP keep-alive to the list of what STUN Usages should discuss (Section 13):

   o  The keep-alive mechanism if STUN is run over TCP or TLS-over-TCP.

I also added the following paragraph to section 6.2.2:

   The details of an eventual keep-alive mechanism are left to each STUN
   Usage.  In any case if a transaction fails because an idle TCP
   connection doesn't work anymore the client SHOULD send a RST and try
   to open a new TCP connection.

and I updated the Basic Usage to follow our own recommendations:

   [...]
   ALTERNATE-SERVER mechanism for the same reason.  It MUST support UDP
   and TCP.  It MAY support STUN over TCP/TLS or   ALTERNATE-SERVER mechanism for the same reason.  It MUST support UDP
   and TCP.  It MAY support STUN over TCP/TLS or STUN over UDP/DTLS;
   however, DTLS and TLS provide minimal security benefits in this basic
   mode of operation.  It does not require a keep-alive mechanism as a
   TCP or TLS-over-TLS connection is closed after the end of the Binding
   transaction.  It MAY utilize the FINGERPRINT mechanism but MUST NOT
 STUN over UDP/DTLS;
   however, DTLS and TLS provide minimal security benefits in this basic
   mode of operation.  It does not require a keep-alive mechanism as a
   TCP or TLS-over-TLS connection is closed after the end of the Binding
   transaction.  It MAY utilize the FINGERPRINT mechanism but MUST NOT
   [...]

> 
> 4) Should section 9.2 maybe provide further guidance on the lifetime of the
> (server-selected) nonce?

I added this in the Security Considerations Section:

   Implementations and deployments of a STUN usage using the Long-Term
   Credential Mechanism (Section 9.2) MUST follow the recommendations in
   Section 5 of [RFC7616].

> 
> 5) I'm sure, I just missed something but how does a server know if a first
> request intends to use the Long-Term Credential Mechanism or not (see 9.2.3.1.
> and 9.2.4 I guess). Or is this pre-configured?

Each STUN Usage defines what authentication mechanism it uses (see bullet point 3 of section 13), so a server implementing a specific Usage knows what to do with the first request.

> 
> 6) Section 6.3 says that this doc only specifies the procedures for the new
> spec in this document and subsequently says: "It checks [...] that the magic
> cookie field has the correct value..." However, given this spec obsoletes
> RFC5389, I think that section 11 should provide more guidance on how to handle
> "old" STUN messages. Or is the intention that an upgraded STUN server does not
> handle old requests anymore? If so that should be spelled out as well.

STUNBis still contains the processing of "classic" STUN (aka RFC5389), which is what "old" STUN messages are (not RFC5389 messages).

> 
> 7) sec 14.5: "The value of the MESSAGE-INTEGRITY attribute is set to a dummy
> value." Should the dummy value be further specified? Also it looks like there
> was a compile problem on page 39. Is there text missing?

Already responded to that point in Dale Worley review:

"[...] the input is up to and including the attribute *preceding* the MESSAGE-INTEGRITY attribute.  The message attribute is only counted when calculating the length in the header [...]
[...] the content of the MESSAGE-INTEGRITY is not used when computing the hash, so the content itself is irrelevant."

The text says:

  The text used as input to HMAC is the STUN message, up to and
  including the attribute preceding the MESSAGE-INTEGRITY-SHA256
  attribute.  

> 
> 8) sec 17.6: Isn't "stuns  5349/upd" used for DTLS? If so, it should be
> registered!
> 
> 

Answered by Adam Roach.

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