Subject: [BEHAVE] Minor comments on draft-ietf-behave-3489bis-05

From: Philip Matthews <[email-masked]>
Date: Mon, 27 Nov 2006 09:25:48 -0500

Jonathan:

This e-mail contains comments on draft-ietf-behave-3489bis-05
which are quite minor in nature. Most of them can be considered
to be editorial.  I have  placed my more interesting
comments in a separate e-mail.

To help you figure out what a comment refers to, I have inserted
section headers to break up the comments. I also frequently
quote from the text of the document.

- Philip

General
Suggest adding a note that the discussion of TCP transport also
pertains to TLS/TCP transport, except where explicitly noted.

Title
    "Simple Traversal Underneath Network Address Translators (NAT)  
(STUN)"
The "(NAT) (STUN)" looks a bit awkward. Perhaps remove the "(NAT)"?

Abstract
    "Simple Traversal Underneath NATs (STUN) is a lightweight protocol
     that serves as a tool for application protocols in dealing with NAT
     traversal. "
Perhaps reword to something like:
   "Simple Traversal Underneath NATs (STUN) is a lightweight protocol
     which can be used by application protocols as one part of a NAT  
traversal scheme."

1. Applicability Statement
    "In particular, the STUN binding usage by itself does not enable
    incoming UDP packets through NATs whose mapping property is address
    dependent or address and port dependent[14].  Furthermore, the
    binding usage, when used by itself, does not work when a client is
    communicating with a peer which happens to be behind the same NAT.
    Nor will it work when the STUN server is not in a common shared
    address realm ."
  First sentence, do you mean “filtering” and not “mapping”? It is  
not clear to me exactly what message exchange  you are envisioning.
Second sentence: Depending on exactly what yoo mean, it WILL work if  
the NAT supports hairpinning.
General comment: I don’t think that anyone not already intimately  
familiar with NATs is going to really understand the problems you  
describe here without pictures and more text than you are providing.  
So it might be better to just state the big picture, and not go into  
the details. This text, coming as it does right at the beginning of  
the document, is rather intimidating to someone who is just getting  
into NATs. Also, not stated in all the text here is that what doesn't  
work is communication with a peer, as opposed to communication with  
the STUN server.

4.  Definitions
Under "mapped address", suggest stating that this is an example of a  
reflexive address.

   "... enrolles in a service and persists ..."
s/enrolles/enrolls/

Add "RTO" and "RTT" to this section?

5.  Overview of Operation
    "in which client"
s/client/the client/

    "There are no indications defined by this specification."
Add "(but see [TURN])"?

      "...mapped address created by the NAT closest to the server.   
The STUN
       server copies that source transport address into a STUN Binding
       Response and sends it back to the source transport address of the
       STUN request. Every type of NAT will route that response so  
that it
Middle sentence: Add “and sends the binding response back to the  
client”?
Also, use the term “reflexive transport address" around here somewhere
to help people understand this term?

Under the discussion of the binding discover usage, suggest rewording
the text to be a bit more general. For example:
     "The client can convey these transport addresses to a remote  
application
       to allow that remote application to send messages to the client"
and then
    "For example, these messages can be conveyed in the SDP body of a  
SIP message ..."

Similarly, the binding keepalive usage does not have to be as complex
as you describe. It can simply be used by an endpoint to keep one or  
more NAT
bindings alive so that nodes on the public side of the NAT can use  
it, either
now or in the future. The details given here are a "for instance",  
but do not
define the usage. Also: is it really important to give so many SIP  
details, since
this is a general mechanism?

      "The STUN binding responses also inform the client
       that the server is still responsive[PM2] , and also inform the  
client if
       its transport address towards the server have changed (its  
reflexive
       transport address),
  Too many “also”s in this sentence?
  As noted in the new NAT discovery document, there are advantages if  
the keepalives do not go all the way to the server. So reword to make  
this part a “may”?

   "These two usages all utilize the same Binding Request message, and
    all require the same basic processing on the server."
Remove “all” (both occurrences)?

    "Some usages (such as the binding keepalive) require STUN  
messages to
    be sent on the same transport address as some application protocol,
    such as RTP or SIP.  To facilitate the demultiplexing of the two,
    STUN defines a special field in the message called the magic cookie,
    which is a fixed 32 bit value that identifies STUN traffic .  STUN
    requests also contain a fingerprint, which is a cryptographic  
hash of
    the message, that allow for validation that the message was a STUN
    request and not a data packet that happened to have the same 32 bit
    value in the right place in the message. "
  There is also a two-bit field of zeros at the start of the message.
  Need to be consistent in number throughout the last sentence.

6.  STUN Message Structure

    "STUN messages are TLV (type-length-value) encoded using big endian
    (network ordered) binary.  STUN messages are encoded using binary
    fields.  All integer fields are carried in network byte order, that
    is, most significant byte (octet) first.  This byte order is  
commonly
    known as big-endian.  The transmission order is described in  
detail ..."
These four sentences are somewhat repetitive.

    "All STUN messages consist of a 20 byte header:"
  s/consist/start/

    "The most significant two bits of every STUN message are both  
zeroes.
    This, combined with the magic cookie and the fingerprint attribute,
    aid[PM1]  in differentiating STUN packets from other protocols  
when STUN is
    multiplexed with other protocols on the same port."
  [PM1] s/aid/aids/  (since it needs to agree with "this")

                          +-+-+-+-+-+-+-+-+-+-+-+-+-+-+
                           |M|M|M|M|M|C|M|M|M|C|M|M|M|M|
                           |1|1|9|8|7|1|6|5|4|0|3|2|2|0|
                           |1|0| | | | | | | | | | | | |
                           +-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  ]The numbering here is the reverse of the standard IETF bit numbering,
which increases from left to right (e.g., figure 2).  In addition, M2  
is repeated twice.
Also, I believe there was already an agreement to add an explanation  
of why
the C bits were located where they are.

    "The magic cookie additionally indicates the STUN client is  
compliant
    with this specification."
As opposed to the previous version of the specification.

   "A reponse  to the STUN request,
s/reponse/response/

   "... 0 and 2**96 - 1.As with requests, ..."
Need two spaces after the period.

   "attributes whose content is not a
    multiple of 4 bytes are padded with 1, 2 or 3 bytes of padding so
    that they are a multiple of 4 bytes."
Just to be clear, say padded at the end?

7.1.  Request/Response Transactions

    "STUN clients are allowed to pipeline STUN requests.  That is, a  
STUN
    client MAY have multiple outstanding STUN requests with different
    transaction IDs and not wait for completion of a STUN request/
    response exchange before sending another STUN request.
Just to be clear, state that this multiple outstanding requests to  
the same server.

   "When running STUN over UDP it is possible that the STUN request or
     its response might be dropped by the network."
This statement is true for TCP as well. Suggest that what you want to  
say
is that, when running over UDP, STUN itself is responsible for handling
retransmissions (or handling lost packets).

Perhaps explain the difference between RTO and RTT?

    "Regardless of whether TCP or UDP was used for the transaction, if a
    failure occurs and the client has other servers it can reach (as a
    consequence of an SRV query which provides a multiplicity of STUN
    servers Section 8.1, for example), the client SHOULD create a new
    request, which is identical to the previous, but has a different
    transaction ID (and consequently a different MESSAGE INTEGRITY  
and/or
    FINGERPRINT attribute."
The last sentence seems to end too soon - what should the client do with
this new request?
Also, this discussion doesn't apply to the connectivity-check usage
or the nat-keepalive usage
(depending on how you define  "server" in these cases).

    "Regardless of whether TCP or UDP was used for the indication, if a
    failure occurs (due to a fatal ICMP error or TCP error), and the
    client has other servers it can reach (as a consequence of an SRV
    query which provides a multiplicity of STUN servers Section 8.1, for
    example), the client SHOULD create a new indication, which is
    identical to the previous, but has a different transaction ID (and
    consequently a different MESSAGE INTEGRITY and/or FINGERPRINT
    attribute).
Same comment as above about the last sentence.
More generally, what usage does this make sense for? Surely we don't
want to do this for a data-indication or send-indication?

8.1.  Discovery

    "The mechanism for configuring the STUN client with the domain  
name to
    look up is not in scope of this document."
s/scope/the scope/

When talking about discovering STUN servers, it says
           "Different ports SHOULD be used
            for TCP and TCP/TLS, so that the server can determine  
whether the
            first message it will receive after the TCP connection is  
set up is a
           STUN message or a TLS message."
Cannot this determination be made using the two zero bits and the  
magic cookie?
Isn't the real reason the fact that TLS is often built into the OS  
layer, while
STUN runs on top? And isn't this whole discussion really an  
implementation detail
so the "SHOULD" is not appropriate? Perhaps reword along the lines of
         "A server may wish to run TCP and TLS/TCP on different  
ports, especially if TLS
           support is built into the OS layer while STUN is  
implemented as a user application,
          because the OS layer will be confused if it receives STUN  
messages when it is
          expecting the TLS handshake messages".

8.3.1.  Formulating the Request Message

    "The client follows the syntax rules defined in Section 6 and the
    transmission rules of Section 7.  The message class MUST be a
    request.
    The client creates a STUN message following the STUN message
    structure described in Section 6.  The client SHOULD add a MESSAGE-
    INTEGRITY and USERNAME attribute to the Request message if the usage
    employs authentication.  The specific credentials to use are ..."
It is perhaps obvious, but the client often needs to add other  
attributes beyond the
authentication-related attributes. Suggest mentioning what other  
attributes need
to be added.

8.3.2.  Processing Responses

     "...response to an oustanding request"
s/oustanding/outstanding/

    "... or it had included the NONCE  but received a 438, it is a  
request .."
Add "(Stale Nonce)" since this has not been explained recently?

     "Responses containing unknown
      mandatory attributions ..."
s/attributions/attributes/
(this typo is rather amusing!)

    "...the same server,  identified by transport address
Insert "as"?

8.4.  Indication Transactions

    "This section applies to client and server behavior[PM1]  for  
sending an
    Indication message."
Suggest rewording as
     "This section describes client behavior for sending an  
indication message."
since server behavior is described later.
[This rewording assumes you are not making other changes that I have  
suggested elsewhere.]

9.  Server Behavior

      "As with clients, server behavior depends on whether it is a  
request/
       response transaction or indication."
  s/indication/indication transaction/     (for consistency).

      "If the request is a retransmission of a request for which the  
server
       has already generated a response within the last 10 seconds, the
       server MUST retransmit the response.  A server can do this  
either by
       remembering the response it transmitted, or by re-processing the
       request and computing the response.  The latter technique can  
only be
       applied to requests which are idempotent and would result in  
the same
       response for the same request.  This is the case for the Binding
       Request, but not for the Shared Secret Request.  Extensions to  
STUN
       SHOULD state whether their request types have this property or  
not."
Doesn't this paragraph only apply to requests sent over UDP?
And what does a server do if the request was received more than 10  
seconds ago? Ignore? Reply with a 600?
Perhaps this should be covered as part of the enhanced description of  
transaction id processing?

       "... consequence of authentication problems (error codes 401,  
432, 434,
        435, 430 and 436, and the REALM is present in the response ..."
  Missing”)”.

       "includes an ERROR-CORE attribute"
s/CORE/CODE/

9.1.2.  Constructing the Response

       "The server MUST copy the transaction ID from the request to  
the response."
If you don't take my suggestion (made elsewhere) to move the backward  
compatibility discussion
into the usage to which it applies, then you need to note that an  
"old-style" transaction id is 32 bits
longer than the new, so this statement needs to be qualified.

10.  Demultiplexing of STUN and Application Traffic

          "All STUN messages have the value 0x2112A442 as the second  
32 bit word."
Is it worthwhile inserting "consistent with this specification" after  
"messages"?
(Perhaps this is too cryptic and/or not helpful?)

           "....  If this probability
            is deemed sufficiently small for the application at hand  
(for
            example, it is considered adequate for Voice over IP  
applications)
            then any packet with this value in its second 32 bit word is
            processed as a STUN packet.
Why the reference to VoIP?  I thought that STUN did not collide with  
RTP and SIP,
so FINGERPRINT is not needed for demuxing in these cases?

       "Thus, if a packet appears to be a STUN message because it has  
a value
         of 0x2112A442 in its second 32 bit word, a client or server  
then
        assumes the message is a STUN message, and computes the value  
for the
        fingerprint.  It then looks for the FINGERPRINT attribute 
[PM1]  in the
        message, and ... "
  Actually, the order needs to be reversed. You have to look for the  
FINGERPRINT attribute first,
since the fingerprint is only over the message up to the FINGERPRINT  
attribute.

11.1.  MAPPED-ADDRESS

       "The port is a network byte ordered representation of the port  
the
         request arrived from."
Would it be clearer to state "the source port in the TCP or UDP  
header of the message when it arrived."?

       "The first 8 bits of the MAPPED-ADDRESS are ignored for the  
purposes
        of aligning parameters on natural 32 bit boundaries."
Suggest rewording as
       "The first 8 bits of the MAPPED-ADDRESS are ignored when  
received. They are present
         only for the purpose of aligning the attribute on a natural  
32 bit boundary."

        "It is possible for an IPv4 host to receive a MAPPED-ADDRESS
         containing an IPv6 address, or for an IPv6 host to receive a  
MAPPED-
         ADDRESS containing an IPv4 address.  Clients MUST be  
prepared for
         this case."
  Perhaps briefly explain why this can happen, and/or say a bit more?
For example “if the client doesn’t like this, it should try a  
different server”?
(This paragraph also appears in section 11.11, but perhaps you only need
to  say this once?)

11.3.  PASSWORD

      "Note that, as described above, if the USERNAME is not a
        multiple of four bytes it is padded for encoding into the STUN
        message,
  s/USERNAME/PASSWORD/

11.4.  MESSAGE-INTEGRITY

      "With the exception of the FINGERPRINT attribute, which appears  
after
       MESSAGE-INTEGRITY, elements MUST ignore all other attributes that
       follow MESSAGE-INTEGRITY."
s/elements/endpoints/  ?  Or however you are handling the client/ 
server terminology question.

11.5 FINGERPRINT

       "When present, the FINGERPRINT attribute MUST be
        the last attribute in the message."
What error code is returned if there are attributes after the  
FINGERPRINT attribute?
Or are the attributes just ignored a la MESSAGE-INTEGRITY?

11.7.  REALM

Mention that this may be padded to a 32 bit boundary (like a USERNAME  
or PASSWORD)?

11.10.  XOR-MAPPED-ADDRESS

     "It provides the same information that would present in the MAPPED-
      ADDRESS attribute but because[PM1]  the NAT's public IP address  
is ..."
Need a comma here.

     "This attribute MUST always be present in a Binding Response and  
may
       be used in other responses as well."
Section 9.1.2 implies that a server inserts just one of MAPPED- 
ADDRESS or XOR-MAPPED-ADDRESS.

11.13.  REFRESH-INTERVAL

    "...interval measured in millseconds."
s/millseconds/milliseconds   (if this section is not just removed)

12.  STUN Usages

    "New STUN usages MUST describe their applicability, client discovery
      of the STUN server, how the server determines the usage, new  
message
      types (requests or indications), new message attributes, new error
      response codes, and new client and server procedures."
The wording here suggests that new STUN must define new message  
attributes,
when I think you mean that they must simply describe any new message  
attributes
they invent.

12.1.  Binding Discovery

     "The previous version of this specification, RFC3489 [15],  
described
      only this binding discovery usage."
It also defined the short-term credentials usage.

12.1.1.  Applicability

     "Binding discovery is used to learn reflexive addresses from  
servers
      on the network, generally the public Internet."
The last part of this sentence doesn't quite read right.

      "That is, it is used
       by a client to determine its dynamically-bound 'public' UDP  
transport
       address that is assigned by a NAT between a STUN client and a  
STUN
       server."
Or TCP.

More general comment. We know that the simplistic approach described in
this section (i.e., determine a reflexive transport address and then  
use it as
your contact address) often doesn't work. So I suggest rewriting this  
section
to say that the Binding-Discovery usage is just used to determine one  
or more
reflexive transport addresses, which the application can then use in  
whatever
way it wishes (e.g., for ICE). I suggest mentioning that it can be  
used to discover
multiple reflexive transport addresses.
I believe there are  other documents that describe the problems with  
the simplistic
approach, if you feel you want to reference them.

12.1.2.  Client Discovery of Server

    "Clients SHOULD be configured with a domain name for a STUN  
server to use."
s/for/of/  ?

12.1.7.  Client Procedures

As mentioned above, suggest rewriting this section to that the usage  
is simply about
getting one or more reflexive transport addresses.

     "...obtained is then using [PM1] in Binding Request messages."
s/using/used/

12.1.8.  Server Procedures

    "It is RECOMMENDED that servers utilize short term credentials,
    obtained by the client from a Shared Secret request, for
    authentication and message integrity.  Consequently, if a Binding
    Request is generated without a short term credential, the server
    SHOULD challenge for one."
Except if the binding request is set over TLS/TCP?

12.2.  NAT Keepalives
12.2.1.  Applicability
This section is written with the viewpoint that a client is connected  
to a server,
and just the client sends keepalives. But ICE doesn't work this way.

     "Since the connection between the client and server is very long  
lived..."
In the previous paragraph, it was just "long-lived".

     "The client will periodically send a Binding Request ..."
How often? Or is this left outside of the specification?
Of course, it differs between TCP and UDP, and perhaps various other  
factors.
If you don't want to specify, then suggest at least saying that.

    "... server using the magic cookie and possibly FINGERPRINT."
And the  two initial zero bits. (Perhaps just reference the demux  
section?).

12.2.3.  Server Determination of Usage
Suggest rewording this section to say that the two endpoints agree on  
this usage
using some mechanism outside this specification. For example, there  
may be some
  application-layer messaging whereby they agree to use this  
mechanism (e.g., sip-outbound or ICE).

12.2.7.  Client Procedures
     "... the client SHOULD inform other applications"
  s/other/the remote/  ? Other might refer to applications on the  
other host.

12.3.  Short-Term Password
12.3.8.  Server Procedures
     "Extremely high probability means that the likelihood of
       collision SHOULD be better than 1 in 2**64."
  s/better/ess than/

12.3.9.  Security Considerations for Short-Term Password
    "To prevent being swamped with traffic, a STUN server SHOULD  
limit the
     number of simultaneous TLS connections it will hold open by  
dropping
     an existing connection when a new connection request arrives (based
     on an Least Recently Used (LRU) policy, for example)."
If the connection is also being used for binding requests, then this  
will cause difficulties for the client,
because the clients wants to keep these connections open until it can  
use the learned
reflexive transport address.

Regarding the mechanism to construct the username and password.
Is it worthwhile explaining why this works and how to  check things?
Not sure why you round the time to 20 minutes – it seems simpler to  
use the exact time.
That way, the server can refuse the username once 30 minutes has  
expired.

13.1.3.  Attack III: Assuming the Identity of a Client
     "the attacker themself."
  s/themself/itself/

16.  Changes Since RFC 3489
     "...in this document"
  s/this/that/  (or perhaps just say "RFC 3489").

18.2.  Informational References
Isn't Rohan now a co-author of the TURN document?

_______________________________________________
Behave mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/behave