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

From: Alexey Melnikov <[email-masked]>
Date: Wed, 18 Apr 2018 09:29:22 -0700

Alexey Melnikov has entered the following ballot position for
draft-ietf-tram-stunbis-16: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tram-stunbis/

----------------------------------------------------------------------
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?

   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?

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?

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.

   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.

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).

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.

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

----------------------------------------------------------------------
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?

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"?

---

From: Marc Petit-Huguenin <[email-masked]>
Date: Sat, 28 Apr 2018 14:29:53 -0700

Hi Alexey,

Thanks for the review, please see my responses inline.

> draft-ietf-tram-stunbis-16: Discuss
[...]

No reason really,

Changed to UsernameCasePreserved.

> I am confused by this statement: you already have 509 bytes 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.

> specifically designed for usernames?

Changed.

> 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.

> 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 long as 509 bytes when encoding them and as long as 763 bytes when

Fixed.

> 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
   [...]

> 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
   [...]

Moved to first paragraph of the section.

> This doesn't look right. Either drop "actual" or move it after "the"?

Fixed.

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