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
- [tram] Alexey Melnikov's Discuss on draft-ietf-tr… Alexey Melnikov
- Re: [tram] Alexey Melnikov's Discuss on draft-iet… Marc Petit-Huguenin