Closed Bug 1752896 Opened 3 years ago Closed 3 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
99 Branch
Tracking Status
firefox99 --- fixed

People

(Reporter: bwc, Assigned: bwc)

References

Details

Attachments

(1 file)

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.

Pushed by bcampen@mozilla.com: 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

Backed out for causing Gtest failures.

Flags: needinfo?(docfaraday)
Flags: needinfo?(docfaraday)
Pushed by bcampen@mozilla.com: 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
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 99 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: