1760769 - WebRTC data channel message integrity violation

Status: NEW

(Core :: WebRTC: Networking, defect)

Description

[Danilo B.]

Attachment: data-channel-mangling-reproducer.html

Steps to reproduce:

In the unit test suite of a project I'm working on, we test the transmission of many WebRTC data channel messages of various sizes in parallel. Frequently (but not always) this test would fail.

Actual results:

I tracked down the reason for this, and found that sometimes messages are reassembled incorrectly. So, for example when sending a message with 131072 bytes followed by a message with 82 bytes, on the other side I would receive an array buffer with 87698 bytes and one with 43456 bytes. Both pairs sum up to 131154 bytes, but the "message boundary" is wrong.

I attached a small hacked-together reproducer. Run it a few times, and you'll get error logs in the debug console eventually. If you get a "unexpected size" entry in the log, filter the logs with that data channel tag (for example "pc1.dc4"). Here's an example from a failed run:

pc1.dc4 outgoing message (131072 bytes)
pc1.dc4 outgoing message (82 bytes)
pc1.dc4 incoming message (87698 bytes)
ERROR: pc1.dc4 unexpected size: 87698 is not one of [131072,82]
pc1.dc4 incoming message (43456 bytes)
ERROR: pc1.dc4 unexpected size: 43456 is not one of [131072,82]

So far I could only reproduce the issue when multiple data channels send in parallel, so I assume it must be a race condition. Maybe in the Firefox code, maybe directly in usrsctp.

Expected results:

This is clearly a violation of data channel message integrity.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::WebRTC: Audio/Video' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Danilo B.]

A similar (the same?) issue affects Chromium as well: https://bugs.chromium.org/p/chromium/issues/detail?id=1309034

Comment 3

[Lennart Grahl [:lgrahl]]

I believe it to be the same issue as the one for Chromium. It appears I added a safeguard in the reassembly routine in Chromium that's being hit.

  // Expect only continuation messages belonging to the same SID. The SCTP
  // stack is expected to ensure this as long as the User Message
  // Interleaving extension (RFC 8260) is not explicitly enabled, so this
  // merely acts as a safeguard.
  if ((partial_incoming_message_.size() != 0) &&
      (rcv.rcv_sid != partial_params_.sid)) {
    RTC_LOG(LS_ERROR) << "Received a new SID without EOR in the previous"
                      << " SCTP packet. Discarding the previous packet.";
    partial_incoming_message_.Clear();
  }

(Source)

This safeguard is not in Firefox because the code in Chromium is not ready to handle interleaved messages (it only has one reassembly buffer for all streams) while the code in Firefox should be, although I was never able to test that.

However, we can already ping Michael. IIRC usrsctp should not interleave messages without explicit activation. But it does look like MSG_EOR is set for those partial messages even though it shouldn't be. Any thoughts on this bug?

(I've edited the comment - sorry, it has been a while since I worked on that code. :) )

Comment 4

[Michael Froman [:mjf]]

I'm unable to repo on macOS, but I am able to repro easily on Win10 (v98 and v100) and Linux (v98).

Comment 5

[Michael Tüxen]

(In reply to Lennart Grahl [:lgrahl] from comment #3)

I believe it to be the same issue as the one for Chromium. It appears I added a safeguard in the reassembly routine in Chromium that's being hit.

  // Expect only continuation messages belonging to the same SID. The SCTP
  // stack is expected to ensure this as long as the User Message
  // Interleaving extension (RFC 8260) is not explicitly enabled, so this
  // merely acts as a safeguard.
  if ((partial_incoming_message_.size() != 0) &&
      (rcv.rcv_sid != partial_params_.sid)) {
    RTC_LOG(LS_ERROR) << "Received a new SID without EOR in the previous"
                      << " SCTP packet. Discarding the previous packet.";
    partial_incoming_message_.Clear();
  }

(Source)

This safeguard is not in Firefox because the code in Chromium is not ready to handle interleaved messages (it only has one reassembly buffer for all streams) while the code in Firefox should be, although I was never able to test that.

However, we can already ping Michael. IIRC usrsctp should not interleave messages without explicit activation. But it does look like MSG_EOR is set for those partial messages even though it shouldn't be. Any thoughts on this bug?

(I've edited the comment - sorry, it has been a while since I worked on that code. :) )

Hi Lennart,

this safeguard should never fire, except you are using an interleaving level of 2 or higher.
Do you have a way to reproduce this problem with usrsctp and possibly a simple test program? If not, I can try to reproduce it next week (this week is IETF and the first week of the semester).

Is this using reliable, order transfer or not?

Best regards
Michael

Comment 6

[Lennart Grahl [:lgrahl]]

(In reply to Michael Tüxen from comment #5)

this safeguard should never fire, except you are using an interleaving level of 2 or higher.
Do you have a way to reproduce this problem with usrsctp and possibly a simple test program? If not, I can try to reproduce it next week (this week is IETF and the first week of the semester).

Hi Michael!

Unfortunately I don't have a simple test program right now (and also a heavy workload :( ), so it would be great if you could give it a try. It looks like sending 128 KiB followed by 82 bytes on multiple streams concurrently in both directions seems to be a sweet spot to trigger it.

Is this using reliable, order transfer or not?

It should be the data channel default, so ordered and reliable.

Cheers
Lennart

Comment 7

[Michael Froman [:mjf]]

Setting this S3 for now.