Subject: [BEHAVE] WGLC for draft-ietf-behave-rfc3489bis-05 starts now

From: "Dan Wing" <[email-masked]>
Date: Mon, 13 Nov 2006 17:53:00 -0800

As agreed at IETF67 in San Diego, the working group last call for STUNbis, 
http://www.ietf.org/internet-drafts/draft-ietf-behave-rfc3489bis-05.txt
begins now and runs for three weeks, ending at 5pm Pacific Time on Monday,
December 4th.

Please submit your comments to the BEHAVE mailing list, [email-masked].  

-d

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

---

From: Magnus Westerlund <[email-masked]>
Date: Fri, 24 Nov 2006 13:54:10 +0100

Hi,

I have made a WGLC review of the document as an early AD review. I have 
found some 23 issues that needs to be straighten out. There are also 
some minor nits at the end of the message.

1.  It is unclear what this document does to RFC 3489. I assume the 
purpose is to obsolete that document. If that is correct please do the 
following. Otherwise you will get comments from the IESG:
A. add "Obsoletes: RFC 3489 (if approved)" to the header
B. Add "This document obsoletes RFC 3489" to the abstract.
C. In the introduction please tell that it obsoletes RFC 3489 and some 
high level points about the interoperability with old RFC 3489 
implementations.
D. Update the first sentence of section 16.

2. Section 2, first paragraph, page 6:
"ALGs have serious limitations, including scalability,
    reliability, and speed of deploying new applications."

I would include "security" in the above list.

3. Section 4, Transport Address definition:

    "Transport Address: The combination of an IP address and (UDP or TCP)
       port."

As I see it STUN could be extended in the future to be used over DCCP 
and SCTP for some usages. Based on that I would like to change the 
definition to the following:

    Transport Address: The combination of an IP address and Transport
    protocol (UDP or TCP) port.

4. Section 5, Page 10:
    In the binding keepalive usage, a client sends an application
    protocol message (such as a SIP REGISTER message) to a server.  The
    server notes the source transport address of the request, and
    remembers it.  Later on, if it needs to reach the client, it sends a
    message to that transport address.

I think the above description of what the server does is not "binding 
keepalive usage". To me it is in fact another usage of STUN that isn't 
really covered in the document. Even if one can use STUN for this 
particular method of achieving NAT-traversal, it is simply an 
alternative to inband signalling of the received information in binding 
discovery response. Thus I would like to have it in at least a separate 
paragraph. One should also discuss this security implications of doing 
this. To my knowledge it will be substantially less secure, as an 
attacker only need to spoof the source-address to achieve an attack on 
anyone. The topological restraints that having the binding discovery 
reply go back to the initiator and then having them signal the 
application server on where to send is more secure.

5. Section 6, page 13, Third paragraph:

"Each STUN attribute ends on a 32 bit boundary:"

I think there should be a SHALL here to make it clear that the alignment 
question is not up for discussion.

"Each STUN attribute SHALL end on a 32 bit boundary:"

6. Section 8.3.2, Second paragraph:

  Then the client SHOULD check the
    response for a MESSAGE-INTEGRITY attribute.  If not present, and the
    client placed a MESSAGE-INTEGRITY attribute into the associated
    request, it MUST discard the response.

This processing has some implication on how a client can react to a 
particular kind of error message. Errors where the client included the 
message integrity but failed to include information that makes it 
possible for the server to include its own message integrity attribute. 
These will be dropped on the floor, and I think correctly so based on 
the security implications of doing otherwise. Fortunately I think the 
errors that will occur of this kind is implementation related rather 
than usage dependent. However I think there should be a warning about 
this in the text. Because the client will perform its 7 retransmission 
and then timeout due to this issue.

7. Section 9. How is the server allowed to handle the TCP connections 
and when may it close it down? I think there needs to be some rules 
here. Otherwise we will see different behavior from different servers.

8. Section 9.1.2, third paragraph: This maybe solved by a better 
introduction explaining what RFC 3489 implementations that are supported 
by server/clients according to this spec. However I think there is need 
to explain why the server should perform this check.

9. Section 10 and 12.2. Demultiplexing STUN and application data over TCP:
I think there is to little written about how you use STUN over TCP. 
Especially about the issues with multiplexing STUN and application data 
in the keep-alive usage. As the STUN messages would need to be sent 
framed together with the application data in such a way that the STUN 
messages can be detected this is somewhat problematic and result in 
limited applicability. The usage over UDP or other packet based 
transport is much simpler. Therefore I think the applicability of STUN 
over TCP for the different usages needs to be expanded. In addition 
section 10 should be updated to discuss the specific TCP issues. Also 
the usage of "STUN packet" should be investigated. Because over TCP 
there are no way of ensuring that STUN is sent in a single STUN packet.

10. Section 11.1: The bits marked as "x" in Figure 6: These are either 
reserved or padding bits. Why not use normative language to indicate 
what they actually should be used. I am a great fan of using:
"x: Reserved bits SHALL be set to 0 and SHALL be ignored on reception." 
The reason I like mandate this is that if one ever for some reasons like 
to use them then it would work to define all values except 0 to mean 
something.

11, Section 11.4, and 13.1 are the only two places the word "elements" 
are used to describe either server or clients and it is not defined in 
either place. I think this should be replaced with "server or client"

12. Section 11.6, 11.12: No where where I can find is it specified if 
the 300 response is required or not to contain an ALTERNATE-SERVER. I 
think this should be defined. And if it is allowed to not contain one, 
then the procedure on what to do should be defined.

13. Section 11.8:
A. Are there any limitation on the length of the NONCE? It might be good 
to recommend and provide some limits to the size of the NONCE.
B. I think the pointer into RFC 2617 should be a bit more specific. 
Point at least to the relevant sections. People shouldn't need to read 
the whole document to find the relevant parts.
C. I also think the explanation on why the NONCE exist is quite thin. 
The best text in the whole document seems to be in section 9.1.1. Maybe 
add something about this in the overview section.

14. Section 11.10:
    This attribute MUST always be present in a Binding Response and may
    be used in other responses as well.

This MUST is in contradiction with the backwards compatibility rules 
present in section 9.1.2 that allows RFC 3489 clients to get the 
response. I think the MUST can be removed here and the exception noted.

15. Section 11.10:
"X-Port is the mapped port, exclusive or'd with most significant 16
    bits of the magic cookie."

Are there a reason why the most significant bits where chosen? Using the 
least significant would have saved a shift or using another constant in 
the XOR operation.

16. Section 12.2.7:
"  The client SHOULD NOT include a MESSAGE-INTEGRITY attribute unless
    prompted for one by the server, since authentication is not generally
    used with this STUN usage."

I think the SHOULD NOT needs some qualifications. There should be some 
explanation when it is reasonable to break this SHOULD NOT.

17. Section 12.2.9:

"Such
    a re-registration would not use the transport address obtained from
    the Binding Response. "

I think there needs to be some explanation why this shouldn't be done. 
To me it seems obvious that some applications will use this short cut in 
performing the application layer update to get things working again. 
Thus opening the security vulnerability to spoofed requests. Thus I 
think more explanation on this and maybe even normative language at 
least RECOMMENDING against using the mapped address present in the 
request the server receives.

18. Section 12.3.1
"  To thwart some on-path attacks described in Section 13, it is
    necessary for the STUN client and STUN server to integrity protect
    the information they exchange over UDP."

"exchanges over UDP." Is this only a inconsistency that should be 
changed to "exchanged in STUN"? Or is it actually limited to UDP?

19. Section 12.3.7, second paragraph:

     Processing of the Shared Secret response follows that of any other
    STUN response.  Note that clients MUST be prepared to be challenged
    for a long-term credential.

And if challenged for long-term credential, what should it do. At least 
some hint on what it should do. I think this is covered in earlier 
section, but I think some reference to the correct procedure would be 
appropriate here.

20. Section 13. There I think is missing an important vulnerability in 
the description. The usage of response code 13 and ALTERNATE-SERVER 
attribute do open up for some nasty DDoS attack, either on any host or 
especially an STUN server. The attack can be done in at least two ways. 
First is to gain access over one or more STUN servers and then start 
redirecting some or all the users to the target of the attack. This can 
also be done by having the attacker deploy its own STUN server 
performing the redirect and get the DNS for some domain to point at 
them. This attack doesn't have a massive amplification factor, but some. 
  For a non stun server being the target it will receive at least 7 
packets per binding discovery redirected. If in addition the STUN client 
redirects all its request for a particular domain to the new server the 
attack will become stronger by the number of streams the client attempts 
to bind. I think the more interesting attack is the one that targets 
some other domains STUN servers. With this redirect attack one can clear 
load a STUN server beyond the point it can cope with. That way one can 
deny service for the applications that rely on the STUN server. Also by 
doing redirect of a small percentage of users from several other servers 
will result in them getting some service and not becoming to obvious 
that they take part in an attack. They may also get some service from 
the overloaded node.

21. Section 13.3:
"   Thirdly, to prevent the DNS attack of Section 13.2.2, Section 8.2
    recommends that the client verify the credentials provided by the
    server with the name used in the DNS lookup."

What about using DNS-SEC?

22. Section 13.4:
What about identity leakage based on long term user names when using 
STUN. Maybe one of these issues that needs to pointed out.

23. Section 15:

I think the port registration should be moved from you Jonathan 
personally to this document.

NITS:
A. Page 9, last paragraph:
"... client wants to receive Real
    Time Transport Protocol (RTP [17]) traffic."

Please use "-" between Real and Time

B. Page 13, second paragraph: Missing space "... 2**967 - 1.AS with ..."

C. Page 25, last line: ERROR-CORE -> ERROR-CODE

D. Section 12.1:
"The previous version of this specification, RFC3489 [15], described
    only this binding discovery usage."

Didn't RFC 3489 describe a testing usage that is being deprecated? Maybe 
reformulate the sentence?

E. Section 13.3, sixth paragraph:

"The above three techniques ..." I can count to four things, but maybe 
not all can be counted as techniques. Still somewhat confusing.

Cheers

Magnus Westerlund

IETF Transport Area Director & TSVWG Chair
----------------------------------------------------------------------
Multimedia Technologies, Ericsson Research EAB/TVA/A
----------------------------------------------------------------------
Ericsson AB                | Phone +46 8 4048287
Torshamsgatan 23           | Fax   +46 8 7575550
S-164 80 Stockholm, Sweden | mailto: [email-masked]
----------------------------------------------------------------------

---

From: "Francois Audet" <[email-masked]>
Date: Sun, 26 Nov 2006 13:45:07 -0600

Jonathan,

I have finished my review of draft-ietf-behave-3489bis-05. I have found
no major issues.

My comments are mostly minor or editorial.

In the interest of time, since the comments are minor, I don't need you
to 
debate/discuss the points, unless there is a good reason. Feel free to
ignore the 
comments you don't agree with.

- 	Title page

	Add "Obsoletes: RFC 3489", and "Category: Standards Track".
	As a side note, I find the title "Simple Traversal Underneath
Network Address
	Translators (NAT)" kind of ridiculous. I know the intent is to
match the acronymn
	STUN, but I'd rather have a more descriptive title, and keep the
STUN name even
	if it doesn't match the title.

1.	Applicability Statement

	This section is very difficult to read as the first section of
this document. If
	legal, it would help the readability of the document if the
Introduction was placed
	before the Applicability Statement.

4.	Definitions

	Use "send STUN indications" for both the definition of STUN
Client and STUN Server
	for consistency.

	In the description of "Reflexive Transport Address", change
"binding allocated" to
	"mapped address allocated".

	Also, the definitions of "Reflexive Transport Address" and
"Mapped Transport Address"
	are not very clear on what is the difference between those 2
terms. I'm not sure
	I understand why we need two terms for those as they appear to
be the same thing.

	In Long Term credentials, replace "enrolles" with "enrolls".

5.	Overview of Operation

	p. 8, end of second paragraph under Figure 1: add "Further
indications may be
	defined in future specifications."

	p. 9, last paragraph: I'm not sure the phrase "and run by the
service provider
	offering the application service (such as a SIP service
provider)," is 
	useful or necessary. I would delete it.

	p. 10, first paragraph: STUN is really for the SDP, not for SIP.
You could
	add "with an SDP Offer or Answer [19]" right after "200 OK
response".

	p. 10, end of second paragraph: replace "SIP outbound" with
"Client initiated
	Connections in SIP".

6.	STUN Message Structure

	First sentence: remove "encoded using big endian (network
ordered) binary" since
	it is redundant with the 3rd sentence.

	Figure 3: Second bit from the right should say "M1" instead of
"M2".

	Paragraph under Figure 3: Instead of "0", "1", "2", "3", I would
use
	"0b00", "0b01", "0b10", "0b11" respectively. Also, instead of
"M11
	through M0" and "C1 through C0", I would use "M11-M0" and
"C1-C0".

	p. 13, first paragraph: change to "...as is done for example in
[13] for 
	RTP and [18] for SIP."

	p. 13, second paragraph: change "1.As" to "1. As".

	p. 13, last paragraph: there is no indication of what the
padding should be.
	All zeros? Does it matter? Please clarify.

8.1	Discovery

	p. 16, 4th paragraph: remove the following phrase as it is not
necessary
	"as long as ports are not needed to differentiate the usage".

8.3.1	Formulating the Request Message

	2nd paragraph: need to justify the "SHOULD add a
MESSAGE-INTEGRITY and USERNAME".
	Why is it a SHOULD instead of a MUST?

8.3.2	Processing Responses

	4th paragraph: I assume that 400 should not trigger a retry.
Please clarify.

	p. 21, last paragraph. When 600 is received, how long does the
client MUST NOT 
	retry? Forever? Probably not. Until reboot? A timer? I think we
need some sort of
	rule for predictability.

9.1.2	Constructing the Response

	p.27, 3rd paragraph: explain why "its inclusion is RECOMMENDED"
(as opposed to MUST).
	Perhaps a pointer to section 10 will be sufficient.

	I think this section needs to say that if a STUN server wants to
deny STUN 
	requests, it uses 600. This is useful if a server does not wish
to respond to 
	STUN (for example, for a SIP server to tell a client to stop
sending STUN requests).
	The use of 600 is implied in section 11.6, but it would be good
to spell it out in
	the procedural section.

10.	Demultiplexing of STUN and Application Traffic

	3rd paragraph: is there really cases where the 2 bits set to
zero suffice???
	If not, should we look at the magic cookie all the time (as a
minimal 
	check)? I'm a little worried that people will use the current
wording as
	justification for not looking at the magic cookie.

11.1	MAPPED-ADDRESS

	Last paragraph: clarify what is meant by "Clients MUST be
prepared for this case.".
	I think what it needs to say is that if the client is dual-stack
(i.e., support both
	IPv4 and IPv6), then it must be prepared to use IP addresses of
whatever version.
	However, (and this is the important point), if it is NOT dual
stack, it must be 
	prepared to process the response anyways. It probably needs to
interpret an IP 
	address of a version it doesn't support as a failed STUN request
(for the Binding
	usage), as it will not be able to use the address.

11.5, 11.10 and perhaps other places

	Make sure you are consistent with the use of the term XOR,
x-ored, or'ed, 
	exclusively or'ed, X-Address and so forth. I'd recommend using
XOR and XOR'ed
	everywhere.

11.6	ERROR-CODE

	p. 32, 2nd paragraph under the binary message description:
explain what the
	padding is (e.g., all zeroes).

11.10 XOR-MAPPED-ADDRESS

	Last-paragraph: same comment as for 11.1 applies.

12.2.1	Applicability

	First paragraph implies that STUN can only be used for TCP and
UDP. Alter the wording
	so that it is clear that other protocols might use STUN.

	Second paragraph: change "very-long lived" to "very long-lived".

12.2.3	Server Determination of Usage

	The second sentence is only true for SIP: other mechanism than a
URI could be used
	for other protocols. I'd suggest removing the sentence
altogether as it it
	belongs in sip-outbound instead.

12.3.9	Security Considerations for Short-Term Password

	p.44, second-to-last paragraph: change "memory or disk to store"
to "storage".

13.1.1	Attack I: DDoS Against a Target

	p.46, first line: remove "or H.323". We have outgrown the habit
of pointing to H.323,
	and if we did leave it in, then we'd have to add it to the
Reference section anyways.

Other possible attack for section 13:
	
	Is there a risk that the ALTERNATE-SERVER may be used for a DDos
attack? You cover
	the case where a legitimate STUN server is compromised (where
this could happen), but
	there is the case of a non-legitimate STUN server that redirects
improperly?
	This could happen if an evil STUN server was configured. (For
example, what if
	larry.gloo.net got annoyed at everybody using it, and decided to
redirect elsewhere?).

13.2.5	Approach V: Response Injection Plus DoS

	First line: replace "MitM" with "man-in-the-middle".

14.5	Issues with Existing NAPT Boxes

	Title: replace "NAPT" with "NAT" for consistency.

18.1	Normative References

	Replace [5] with rFC 4346 (i.e., TLS 1.1 instead of 1.0).

18.2	Informational References

	Update [18] to outbound-05.