Thanks for the review comments. Responses below:
Miguel Garcia wrote:
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.
added:
<t hangText="RTO:">Retransmission TimeOut, which defines the
initial period of time between transmission of a request and
the first retransmit of that request.</t>
- 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|
+--+--+-+-+-+-+-+-+-+-+-+-+-+-+
done.
- 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.
There is no solution. No TCP fallback is provided as this is
fundamentally not possible, given NAT pinholes are specific to UDP and TCP.
Text says:
<t>All STUN requests (and responses) sent over UDP MUST be
less than the path MTU, or 1500 bytes if the MTU is not
known. 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.</t>
- 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.
It means configurable using whatever means are used to configure the UA.
Usually its automated, but STUN doesnt REQUIRE automated configuration,
in which case this could be user adjusted, likely through hidden config
settings I suspect. I don't think we can or should mandate automated
configuration.
- Section 7.2.1 refers to the Karn's algorithm. I suggest to add an
informative reference to Karn's algorithm.
ok.
- Section 7.2.1, typo:
s/by an client/by a client
fixed.
- 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.
these terms are actually used nowhere else; just here. I think the
definition is clear from the words.
- 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.
I added an inline definition of idempotent. Its not used enough to
warrant definition in the terminology section.
- 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".
well it does indicate when the check is performed. I changed the text to:
correct value. <xref target="sec:receiving-a-stun-message"/>
describes when in the overall processing of a STUN message the
FINGERPRINT check is performed. This additional check helps
- Section 10.2.1.2
s/INTEGRITY attributed/INTEGRITY attributes
fixed
- 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).
fixed.
Additionally, a reference to the "NAT Behavior Discovery usage" should
be added.
done.
- Section 13, last sentence, add a reference to RFC 3424.
done.
- 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.
good catch, fixed.
- 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.
fixed.
- 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.
fixed.
- 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".
fixed
- 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
wow - good catch. fixed.
- Section 14.6, the figures does not have a figure number neither a
caption.
fixed
- 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.
This got discussed in the group. There is a DoS attack possible if you
include a specific error code for invalid username. It allows for
probing of valid usernames without authenticating yourself. So a generic
401 is used to cover all cases. The text in 14.6 needs to be adjusted.
- 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.
I don't follow. Hex is just a way of representing binary values in a
string format. The protocol fields are binary.
- 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.
fixed
- 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.
fixed.
- 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.
you don't need to understand rfc3261; you just need to pull the BNF
definition from there.
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.
I dislike repeating the same grammar in multiple places; it introduces
the possibility of making a mistake and then the definition is really
unclear.
- 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".
Yes, great catch.
Thanks,
Jonathan R.
--
Jonathan D. Rosenberg, Ph.D. 600 Lanidex Plaza
Cisco Fellow Parsippany, NJ 07054-2711
Cisco Systems
[EMAIL PROTECTED] FAX: (973) 952-5050
http://www.jdrosen.net PHONE: (973) 952-5000
http://www.cisco.com
_______________________________________________
Gen-art mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/gen-art
