Re: [tram] Alexey Melnikov's Discuss on draft-ietf-tram-stunbis-16: (with DISCUSS and COMMENT)

Marc Petit-Huguenin <petithug@acm.org> Sat, 28 April 2018 21:30 UTC

Return-Path: <petithug@acm.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 0B2DC127601; Sat, 28 Apr 2018 14:30:08 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.441
X-Spam-Level:
X-Spam-Status: No, score=-0.441 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RDNS_NONE=0.793, SPF_SOFTFAIL=0.665, URIBL_BLOCKED=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 vekeo0YAgAnB; Sat, 28 Apr 2018 14:30:05 -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 7DD5112778D; Sat, 28 Apr 2018 14:30:05 -0700 (PDT)
Received: from [IPv6:2601:648:8301:730f:9171:1104:b81b:2778] (unknown [IPv6:2601:648:8301:730f:9171:1104:b81b:2778]) (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 71042AE844; Sat, 28 Apr 2018 23:30:00 +0200 (CEST)
To: Alexey Melnikov <aamelnikov@fastmail.fm>, The IESG <iesg@ietf.org>
Cc: tram-chairs@ietf.org, tasveren@rbbn.com, Gonzalo.Camarillo@ericsson.com, draft-ietf-tram-stunbis@ietf.org, tram@ietf.org
References: <152406896286.13830.7440801444130192741.idtracker@ietfa.amsl.com>
From: Marc Petit-Huguenin <petithug@acm.org>
Message-ID: <cd8c63e1-f6b8-bc02-1255-302bf8b8c7b3@acm.org>
Date: Sat, 28 Apr 2018 14:29:53 -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: <152406896286.13830.7440801444130192741.idtracker@ietfa.amsl.com>
Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="qesjHgN64czY8k04CibpKdqMPpaln8h7A"
Archived-At: <https://mailarchive.ietf.org/arch/msg/tram/kxnBxDiQpAuRdqt2miu0mJ_Zlq8>
Subject: Re: [tram] Alexey Melnikov's Discuss on draft-ietf-tram-stunbis-16: (with DISCUSS and COMMENT)
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: Sat, 28 Apr 2018 21:30:08 -0000

Hi Alexey,

Thanks for the review, please see my responses inline.

On 04/18/2018 09:29 AM, Alexey Melnikov wrote:
> Alexey Melnikov has entered the following ballot position for
> draft-ietf-tram-stunbis-16: Discuss
> 
[...]
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> I would like to discuss several issues with the document (which look easy to
> fix to me) before recommending its approval for publication as an RFC:
> 
> 1) 14.3.  USERNAME
> 
>    The USERNAME attribute is used for message integrity.  It identifies
>    the username and password combination used in the message-integrity
>    check.
> 
>    The value of USERNAME is a variable-length value containing the
>    authentication username.  It MUST contain a UTF-8 [RFC3629] encoded
>    sequence of less than 509 bytes, and MUST have been processed using
>    the OpaqueString profile [RFC8265].
> 
> I am trying to understand if there was a particular reason why you didn't use
> UsernameCasePreserved (or its case insensitive alternative) profile here which
> was specifically designed for usernames?

No reason really,

Changed to UsernameCasePreserved.

> 
>    A compliant implementation MUST
>    be able to parse UTF-8 encoded sequence of less than 763.
> 
> I am confused by this statement: you already have 509 bytes above.
> Is "no" missing above?

It has been pointed out (in a review that was not sent to the mailing-list) that RFC 3629 can encode characters into only up to 4 bytes.  So 509 bytes (127 characters + the eventual 0) is the actual maximum.  But RFC 5389 said 763 (127 characters encoded into 6 bytes each, + the eventual 0), which, although contradicting RFC 3629, may let implementers think that they could encode with 6 bytes.

Thus I am keeping the decoding requirements to 763 bytes to be compliant with RFC 5389, but restricting the encoding to 503 bytes to be - at last- fully compliant with RFC 3629.

So I modified the text there to:

   The value of USERNAME is a variable-length value containing the
   authentication username.  It MUST contain a UTF-8 [RFC3629] encoded
   sequence of less than 509 bytes, and MUST have been processed using
   the UsernameCasePreserved profile [RFC8265].  A compliant
   implementation MUST be able to parse UTF-8 encoded sequence of less
   than 763 bytes, to be compatible with [RFC5389] that mistakenly
   assumed up to 6 bytes per characters encoded.


> 
> 14.4.  USERHASH
> 
>    The USERHASH attribute is used as a replacement for the USERNAME
>    attribute when username anonymity is supported.
> 
>    The value of USERHASH has a fixed length of 32 bytes.  The username
>    and the realm MUST have been processed using the OpaqueString profile
>    [RFC8265] before hashing.
> 
> As above: why didn't you use UsernameCasePreserved profile which was
> specifically designed for usernames?

Changed.

> 
> 2) 14.9.  REALM
> 
>    The REALM attribute may be present in requests and responses.  It
>    contains text that meets the grammar for "realm-value" as described
>    in [RFC3261] but without the double quotes and their surrounding
>    whitespace.  That is, it is an unquoted realm-value (and is therefore
>    a sequence of qdtext or quoted-pair).  It MUST be a UTF-8 [RFC3629]
>    encoded sequence of less than 128 characters (which can be as long as
>    509 bytes when encoding them and a long as 763 bytes when decoding
>    them),
> 
> (Here and similar text in several other sections)
> Can you please elaborate on how you came up with values 509 and 763?
> And why you need more space for decoding than for encoding of UTF-8.

See above.  I put the full explanation only on its first occurrence though.

> 
>    and MUST have been processed using the OpaqueString profile
>    [RFC8265].
> 
> 3) 14.16.  ALTERNATE-DOMAIN
> 
>    The value of ALTERNATE-DOMAIN is variable length.  It MUST be a UTF-8
>    [RFC3629] encoded sequence of less than 128 characters (which can be
> 
> Ekr already pointed this out, but I want to expand on this: you need to say
> whether this allows U-label (which are UTF-8) or A-labels (which are ASCII
> only). If you only allow Punycode encoded A-labels, this field doesn't need to
> be UTF-8, it only need to be ASCII.

I supposed that using Punycode ensure maximum compatibility with DNS client that may not fully support Unicode, so I chose that.

   The value of ALTERNATE-DOMAIN is variable length.  It MUST be a valid
   DNS name [RFC1123] (including A-labels [RFC5890]) of 255 or less
   ASCII characters.

> 
> As far as I remember ASCII domain names are limited to 255 bytes.
> 
>    as long as 509 bytes when encoding them and as long as 763 bytes when
>    decoding them).

Fixed.

> 
> 4)
> 
> 10.  ALTERNATE-SERVER Mechanism
> 
>    If the transport protocol uses TLS or DTLS, then the
>    client looks for an ALTERNATE-DOMAIN attribute.  If the attribute is
>    found, the domain MUST be used to validate the certificate using the
>    recommendations in [RFC6125].
> 
> When you reference RFC 6125 you need to provide more details:
> 
> a) Are you expecting support for DNS-ID, CN-ID or both? I assume you don't
> support SRV-ID/URI-ID (saying so explicitly would be great too). b) Are you
> expected to allow wildcards in DNS-IDs/CN-IDs? If yes, you need to say so.

New text:

   [...]
   recommendations in [RFC6125].  The certificate MUST contain an
   identifier of type DNS-ID or CN-ID, eventually with wildcards, but
   not of type SRV-ID or URI-ID.  If the attribute is not found, the
   [...]

> 
> There is one more reference to RFC 6125 in the document, you should make a
> similar change there as well.

New text:

   [...]
   MUST verify the identity of the server.  To do that, it follows the
   identification procedures defined in [RFC6125], with certificate
   containing an identifier of type DNS-ID or CN-ID, eventually with
   wildcards, but not of type SRV-ID or URI-ID.  Alternatively, a client
   [...]

> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> I am agreeing with DISCUSS/comments from Adam.
> 
> Additionally, I have the following comments:
> 
> 9.2.4.  Receiving a Request
> 
>    o  If the value of the USERNAME or USERHASH attribute is not valid,
>       the server MUST generate an error response with an error code of
>       401 (Unauthenticated).  This response MUST include a REALM value.
>       It is RECOMMENDED that the REALM value be the domain name of the
>       provider of the STUN server.
> 
> You include this sentence here and at the beginning of this section,
> but not above (in at least 2 other places) where REALM is also mentioned?
> Just mention the advice once earlier in this section?

Moved to first paragraph of the section.

> 
> 14.10.  NONCE
> 
>    Note that this means that the NONCE attribute
>    will not contain actual the surrounding quote characters.
> 
> This doesn't look right. Either drop "actual" or move it after "the"?
> 
> 

Fixed.

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