1752896 - NAT simulator allows incoming packets through even when they arrive on a port that is mapped to some other destination

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P2)

Description

[Byron Campen [:bwc]]

This code needs to be checking that the port mapping we've selected matches the port that the packet arrived on:

https://searchfox.org/mozilla-central/rev/78963fe42f8d5f582f84da84a5e78377b6c1fc32/dom/media/webrtc/transport/test_nr_socket.cpp#443

For example, let's say we have the following situation:

The TURN server is running on 3478
ICE agent A has host candidate host_a, and a srflx candidate srflx_a (that the NAT simulator opened when it sent an ALLOCATE request to TURN port 3478).
ICE agent B has a relay candidate relay_b (this address is on the TURN server).

• B sends a STUN check to srflx_a via the TURN server.
• As usual, the TURN server uses relay_b to send this relayed STUN request. So, we have a STUN packet going from relay_b -> srflx_a.

The NAT simulator should be dropping this packet, because srflx_a is the address it mapped to TURN port 3478, not relay_b. Right now, we're letting this packet through.

If we drop this packet, connectivity is established as follows:

• A sends a STUN check to relay_b
• The NAT simulator opens a new port (let's call this srflx_a_2), and then sends the packet from srflx_a_2 to relay_b.
• The TURN server receives the STUN check, and rejects it, because there is no permission yet for srflx_a_2.
• A receives the error response, and as a side effect learns about srflx_a_2.
• A trickles srflx_a_2
• B creates a TURN permission (on relay_b) for srflx_a_2, and sends a new STUN check to srflx_a_2 via the TURN server.
• The TURN server relays this packet, sending it from relay_b to srflx_a_2.
• A receives the packet on srflx_a_2, and this time the port mapping is a complete match.

Blocks bug 1253706 because allowing this packet through results in an assertion when a response is sent, because a brand new port mapping is opened for the response, and the STUNUDPPacketFilter does not like sending responses to stuff that it has not seen the request for. New tests in bug 1253706 triggered this (fairly new, added in bug 1748852) assertion.

Comment 1

[Byron Campen [:bwc]]

Attachment: Bug 1752896: In NAT simulator, check that incoming packets are arriving at the right port in addition to checking that they are arriving _from_ the right port. r?mjf

Also, some logging that was helpful in diagnosing the problem.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc684f283be4
In NAT simulator, check that incoming packets are arriving at the right port in addition to checking that they are arriving _from_ the right port. r=mjf

Comment 3

[Cristian Tuns]

Backed out for causing Gtest failures.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | WebRtcIcePacketFilterTest.TestRecvStunPacketWithoutAPendingId | Expected equality of these values:

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/359574aa5cd3
In NAT simulator, check that incoming packets are arriving at the right port in addition to checking that they are arriving _from_ the right port. r=mjf

Comment 5

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/359574aa5cd3