Hi, See responses inline Den 2018-02-02 kl. 12:12, skrev Christer Holmberg:
---E. Section 7.2.5.2.2. ICMP Error An ICE agent MAY support processing of ICMP errors for connectivity checks. If the agent supports processing of ICMP errors, and if a Binging request generates an ICMP error, the agent SHOULD set the state of the candidate pair to Failed. I am a bit worried by this blanket statement on ICMP errors. I think it should be clarified which ICMP message types that are relevant to consider as errors? I assume Type 3 (Destination Unreachable) but maybe not all responde codes as Codes 4, 11,12 may be addressable in other ways, and likely Type 11 (Time exceeded) with response code 0, response code 1 is not a clear indication of a non working path.This is from RFC 5245. I don¹t think the ICE WG should go through all different codes and combinations, and determine what should be considered an error, and what not. If you can provide something (table, guidance etc), we are happy to include it. Otherwise I¹d like to keep it as it is, and let implementations deal with it, as at least I am not aware that this would Have caused issues in ICE deployments.I think we there is a point to clarify that this applies to ICMP messages indicating a non-usable path. So maybe it could be rewritten to something like this: An ICE agent MAY support processing of ICMP messaging indicating a non-functioning path for connectivity checks. ICMP messages of type 3 (Destination Unreachable) are indicators of a currently non-functioning path. However, the issue can be temporary as it can depend on routing transients, this for example applies to codes 0, 1 and 5. Other messages that appear to indicate non-functioning path such as Type=11 (Time Exceeded) with code=0 (Time to Live exceeded in Transit) are not clear indicator as the IP packets potentially can reach the destination with a larger TTL value set at transmission. Therefore, implementation needs to analyse the different ICMP messages types and codes for which it considers the path as non-functioning. If the agent supports processing of ICMP errors, and if a Binging request generates an ICMP error, the agent SHOULD set the state of the candidate pair to Failed. What also is not addressed in this is the risk of denial of service attacks using spoofed ICMP messages to shutdown certain connectivity checks. The security considerations lack any discussion of this issue. If ICMP processing are retained, I think a recommendation about validation is needed to avoid at least off path attackers from doing these attacks easily. Unfortunately ICMP response will only include the IP/UDP header, thus no data from the STUN messages which would allow verification that the ICMP messages matches an actually sent check. It may be simplest to recommend against reacting to ICMP errors from both the perspective that it is a risk for denial of service attack, as well as that it represents a risk terminating connectivity checks for a transient issue. From my perspective I expect this to reduce the number of sent connectivity checks very littleSo, are you saying that an agent should simply ignore ICMP messages?Yes, I think that may be the best. There are a bit to many corner cases and significant attack surface that getting all the details right are significant work for a relatively small reduction in sent connection check messages.I am not an expert, but aren’t you going to get ICMP unreachable every time you try to contact a host candidate behind a NAT? Are you saying that the agent should ignore it, as the connectivity test will anyway timeout at some point?
To my understanding NATs and firewalls do not send ICMP unreachable because that would violates its role as a gateway rather than end-host, also it want to avoid giving away information about its internal side, and is a risk in creating a lot of load on the middlebox.
Also, note that RFC 5398 (STUN) says: "A STUN transaction over UDP is also considered failed if there has been a hard ICMP error [RFC1122].” I am a little worried that people would have to use ICE-specific STUN stacks if they are required to ignore ICMP errors. Couldn’t we simply use the existing text, and add a sentence about DoS attacks? OLD: An ICE agent MAY support processing of ICMP errors for connectivity checks. If the agent supports processing of ICMP errors, and if a Binging request generates an ICMP error, the agent SHOULD set the state of the candidate pair to Failed. NEW: An ICE agent MAY support processing of ICMP errors for connectivity checks. If the agent supports processing of ICMP errors, and if a Binging request generates an ICMP error, the agent SHOULD set the state of the candidate pair to Failed. Implementers need to be aware that ICMP errors can be used as a method for denial of service attacks when making a decision on how and if to process ICMP errors.
I think you need to at least make it clear that it is "hard ICMP errors". Which RFC 1122 defined as Type 3 with codes 2-4 for TCP. So, in that case one could just as well be explicit. I don't know if any of the later defined codes are defined as hard errors.
When it comes to the security consideration, I am actually quite worried by this. To spoof ICMP so that they arrive back at the client, you need to be able to send an ICMP packet back that matches the NAT's binding. That requires that you now the connectivity checks intended target address+port, and the NAT's source address+port. With that information you can generate an ICMP message that will arrive at the agent as long as the attacker can route a packet to the NATs address. And if you are in the local address domain to the agent, you can fake an ICMP error with only the information matching the peer agents candidate.
I think this issue do need security considerations text.
======= Minor/Editorial Issues:---9. Section 15: 4.57566E+18 (note that an implementation would represent this as a 64-bit integer so as not to lose precision). Why the floating point representation? Priorities are integer numbers and thus should be presented as such in this example.This is from RFC 5245, and unfortunately I don¹t know.Can you not just calculate the 64-bit integer and write it out?So, you want me to write 4575660000000000000?No, I thought the pair priority will not be 0 for the lower 32 bits and that there actually are overflow in this deduction. My understanding is that what should be written here is the calculation of: pair priority = 2^32*MIN(G,D) + 2*MAX(G,D) + (G>D?1:0) Where G and D are the priority values for $L_PRIV_1 and $R_PUB_1. $L_PRIV_1 is L's host candidate, and stated to have a priorty of 2130706431 $R_PUB_1 is R's host candidate and stated to have a priorty of 2130706431 So G = 2130706431 and D=2130706431 pair priority then becomes 2^32*2130706431 + 2*2130706431 + 0 = 9151314442783293438 Thus, I think the next two pair priorities in the example also needs to be fixed. The first pair priority for R is the same value as for L, as G and D are identical. But the next value will be different. To my understanding L will be controlling, i.e. L's candidate will be G G=1694498815 D= 2130706431 Pair priority = 2^32*1694498815+2*2130706431 + 0 = 7277816997797167102Please provide text for the section (or, at least the modified paragraphs) :)
Fine, I can put in the numbers in the right place. But please check my calculations:
OLD: Agents L and R both pair up the candidates. They both initially have two pairs. However, agent L will prune the pair containing its server reflexive candidate, resulting in just one. At agent L, this pair has a local candidate of $L_PRIV_1 and remote candidate of $R_PUB_1, and has a candidate pair priority of 4.57566E+18 (note that an implementation would represent this as a 64-bit integer so as not to lose precision). At agent R, there are two pairs. The highest priority has a local candidate of $R_PUB_1 and remote candidate of $L_PRIV_1 and has a priority of 4.57566E+18, and the second has a local candidate of $R_PUB_1 and remote candidate of $NAT_PUB_1 and priority 3.63891E+18. NEW: Agents L and R both pair up the candidates. They both initially have two pairs. However, agent L will prune the pair containing its server reflexive candidate, resulting in just one. At agent L, this pair has a local candidate of $L_PRIV_1 and remote candidate of $R_PUB_1, and has a candidate pair priority of 9151314442783293438. At agent R, there are two pairs. The highest priority has a local candidate of $R_PUB_1 and remote candidate of$L_PRIV_1 and has a priority of 9151314442783293438, and the second has a
local candidate of $R_PUB_1 and remote candidate of $NAT_PUB_1 and priority 7277816997797167102. Cheers Magnus Westerlund ---------------------------------------------------------------------- Media Technologies, Ericsson Research ---------------------------------------------------------------------- Ericsson AB | Phone +46 10 7148287 Torshamnsgatan 23 | Mobile +46 73 0949079 SE-164 80 Stockholm, Sweden | mailto: magnus.westerlund@xxxxxxxxxxxx ----------------------------------------------------------------------
<<attachment: smime.p7s>>