I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please resolve these comments along with any other Last Call comments
you may receive.


Document: draf-tietf-behave-rfc3489bis-11.txt
Reviewer: Miguel Garcia <[EMAIL PROTECTED]>
Review Date: 2007-10-26
IETF LC End Date: 2007-10-26

Summary: The document is almost ready for publication as a standards track RFC.

Comments: The document is very well written and has an outstanding quality. Congratulations to the authors.

I have a number of comments, mainly clarifications that may help implementors to better understand the text. Take these comments are your own discretion.



- Section 5, Definitions. The last term is "RTO", which is merely expanded. It would be nice to have a short description, like in the rest of the defined terms.

- Section 6, Figure 3. It would help if you add the bit counter on top of the figure, for example:

->                       0                 1
->                       2  3  4 5 6 7 8 9 0 1 2 3 4 5
                        +--+--+-+-+-+-+-+-+-+-+-+-+-+-+
                        |M |M |M|M|M|C|M|M|M|C|M|M|M|M|
                        |11|10|9|8|7|1|6|5|4|0|3|2|1|0|
                        +--+--+-+-+-+-+-+-+-+-+-+-+-+-+

- Section 7.1, the last sentence reads:

   All STUN requests (and responses) sent over UDP MUST be less than the
   path MTU, or 1500 bytes if the MTU is not known.

The draft does not offer a mechanism to fall back to TCP. I was wondering what happens if the request is sent over UDP and the response should be larger than the MTU. If there is no solution for this case, which is probably more theoretical than practical (due to the nature of STUN), it should be listed as a limitation of the protocol.

- Section 7.2.1 refers to a _configurable_ value of RTO:

   First, the initial value for RTO SHOULD be configurable (rather than
   the 3s recommended in RFC 2988) and SHOULD be greater than 100ms.

I was wondering of the nature of the configurable value. Does the word mean that the human user should be able to configure the initial RTO value? Or does it mean that STUN should offer applications the possibility of configure this value? The former is scary... I don't expect humans need to do something related to STUN. The latter is very reasonable, so, if the intention is to have the initial RTO value configurable to applications, it should be mentioned in the draft.

- Section 7.2.1 refers to the Karn's algorithm. I suggest to add an informative reference to Karn's algorithm.

- Section 7.2.1, typo:
  s/by an client/by a client

- Section 7.3, the 4th paragraph uses the "known-but-unexpected attributes" and "comprehension-optional attributes" terms. While it is obvious what they mean, they haven't been defined earlier, so, I would suggest to add the two terms to the Definitions section.

- Section 7.3.1, third paragraph, talks about idempotent requests. I am sure if the reader who is not familiar with RFC 2616 will get right what it means, although the same sentence tries to get a definition. Perhaps this is another term to add to the definitions, and add a reference to RFC 2616.

- Section 8, last paragraph, the text reads:

   the agent also checks that the
   message contains a FINGERPRINT attribute and that the attribute
   contains the correct value (see Section 7.3.

The comment is that Section 7.3 does not have additional information, so I suggest to remove the "(see Section 7.3".

- Section 10.2.1.2
 s/INTEGRITY attributed/INTEGRITY attributes

- Section 12, last bullet point:

   o  There were two comprehension-required attributes, RESPONSE-ADDRESS
      and CHANGE-REQUEST, that have been removed from this
      specification;

      *  These attributes are now part of the NAT Behavior Discovery
         usage.


I noticed that in addition to RESPONSE-ADDRESS and CHANGE-REQUEST, also CHANGE-ADDRESS has been removed from the original 3489, although I don't know if CHANGE-ADDRESS is comprehension-required or not. In any case, it should be also mentioned that the attribute has been removed (and is nw part of the NAT Behavior Discovery).

Additionally, a reference to the "NAT Behavior Discovery usage" should be added.

- Section 13, last sentence, add a reference to RFC 3424.

- Section 14.2, the forth paragraph reads:

   If the IP address
   family is IPv6, X-Address is computed by taking the mapped IP address
   in host byte order, XOR'ing it with the magic cookie and the 96-bit
   transaction ID, and converting the result to network byte order.

I am not sure how to XOR the mapped IP address. I suspect the text wants to say "XOR'ing it with the *concatenation of the* magic cookie and the 96-bit transaction ID". I hope it does not mean doing a double XOR operation.

- In many places in the spec, starting in Section 14.3, the term UTF-8 is mentioned. I am missing a normative reference RFC 3629 in every occurrence.

- Similarly, in Section 14.4, the term MD5 is written briefly to explain how to calculate the key. It would be nice to have some text around that includes a normative reference to RFC 1321.

- Section 14.5 reads:

   The FINGERPRINT attribute may be present in all STUN messages.

  I think the "may" can be promoted to a normative "MAY".

- Section 14.5, there is a reference to ITU-V42.1994. I tried to download that version of the specification, but it is no longer available. It has been superseded by "200203",

http://www.itu.int/rec/T-REC-V.42/en

- Section 14.6, the figures does not have a figure number neither a caption.


- Section 14.6. I have trouble to follow-up on the 401 code, I think the draft contradicts itself. The text in Section 14.6 reads:

   401  Unauthorized: The request did not contain the expected MESSAGE-
        INTEGRITY attribute.  The server MAY include the MESSAGE-
        INTEGRITY attribute in its error response.


However, in Section 10.2.2, 4th bullet point, the text reads:

   o  If the username in the USERNAME attribute is not valid, the server
      MUST generate an error response with an error code of 401
      (Unauthorized).

So, it seems that the 401 response does not really mean that the MESSAGE-INTEGRITY attribute is not present or is wrong. It may also mean that the USERNAME attribute is not valid.

Is this the desired behavior though? Essentially, it will be difficult to inspect a response and analyze what the problem is without correlating it with the request.


- Section 14.6, below the figure, the text reads:

   The Reserved bits SHOULD be 0, and are for alignment on 32-bit
   boundaries.  Receivers MUST ignore these bits.  The Class represents
   the hundreds digit of the error code.  The value MUST be between 3
   and 6.  The number represents the error code modulo 100, and its
   value MUST be between 0 and 99.


It would be nice to indicate that these values are all written in binary. I hope none makes a mistake and codes them in hexadecimal.

- Section 14.6, the following paragraph reads:

   The following error codes, along with their recommended reason
   phrases (in brackets) are defined:

But the reason phrases are not written "in brackets", so delete it.


- Section 14.8 Unlike Section 14.7, the text in 14.8 does not say whether the NONE attribute includes the quotes or not. Bearing in mind that RFC 2617 apparently requires the usage of quotes in nones, but 'qdtext' in RFC3261 excludes them, I would suggest to clearly say, as in Section 14.7, whether the NONCE attribute includes quotes or not.

- Section 14.8. The text reuses 'qdtext' and 'quoted-pair' from RFC 3261. If the implementor is familiar with SIP, will most likely appreciate it, but if the implementor is not so familiar with SIP, it won't help. I wonder if there is a way to express the exactly what it is, without referring to RFC 3261.

Furthermore. I noticed that the 'nonce' value in RFC 3261 is defined as follows:

nonce               =  "nonce" EQUAL nonce-value
nonce-value         =  quoted-string

So, notice that nonce-value in RFC 3261 does not allow to include a 'quoted-pair', as indicated in STUN. Therefore, I think the reference to RFC 3261 will just confuse people. The BNF of the nonce value attribute should be self-described in STUN.

- Section 17.2

In the initial STUN Attributes types, it came to my attention that the values 0 and 7 are reserved, because they were used by the original RFC 3489. I noticed that the values 2, 3, 4, and 5 are not mentioned. But then I noticed that values 3, 4, and 5 are registered by the NAT Behavior Discovery usage. So, what about value 2, which RFC 3489 assigns to the RESPONSE-ADDRESS attribute, and that nowadays seems to be deprecated? I think it should be registered by this draft as "reserved; was RESPONSE-ADDRESS".


/Miguel



--
Miguel A. Garcia           tel:+358-50-4804586
Nokia Siemens Networks     Espoo, Finland



_______________________________________________
Gen-art mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/gen-art

Reply via email to