diff options
| -rw-r--r-- | AUTHORS | 1 | ||||
| -rw-r--r-- | media/BUILD.gn | 9 | ||||
| -rw-r--r-- | media/sctp/sctp_transport.cc | 216 | ||||
| -rw-r--r-- | media/sctp/sctp_transport.h | 22 | ||||
| -rw-r--r-- | media/sctp/sctp_transport_unittest.cc | 68 |
5 files changed, 209 insertions, 107 deletions
@@ -90,6 +90,7 @@ CZ Theng <cz.theng@gmail.com> Miguel Paris <mparisdiaz@gmail.com> Raman Budny <budnyjj@gmail.com> Stephan Hartmann <stha09@googlemail.com> +Lennart Grahl <lennart.grahl@gmail.com> &yet LLC <*@andyet.com> 8x8 Inc. <*@sip-communicator.org> diff --git a/media/BUILD.gn b/media/BUILD.gn index b6c78fdb39..f1ead11e30 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -429,11 +429,6 @@ rtc_library("rtc_data") { } if (rtc_enable_sctp && rtc_build_usrsctp) { - include_dirs = [ - # TODO(jiayl): move this into the public_configs of - # //third_party/usrsctp/BUILD.gn. - "//third_party/usrsctp/usrsctplib", - ] deps += [ "//third_party/usrsctp" ] } } @@ -659,5 +654,9 @@ if (rtc_include_tests) { if (is_ios) { deps += [ ":rtc_media_unittests_bundle_data" ] } + + if (rtc_enable_sctp && rtc_build_usrsctp) { + deps += [ "//third_party/usrsctp" ] + } } } diff --git a/media/sctp/sctp_transport.cc b/media/sctp/sctp_transport.cc index 35824b7f25..c399dfe98d 100644 --- a/media/sctp/sctp_transport.cc +++ b/media/sctp/sctp_transport.cc @@ -16,10 +16,16 @@ enum PreservedErrno { SCTP_EINPROGRESS = EINPROGRESS, SCTP_EWOULDBLOCK = EWOULDBLOCK }; + +// Successful return value from usrsctp callbacks. Is not actually used by +// usrsctp, but all example programs for usrsctp use 1 as their return value. +constexpr int kSctpSuccessReturn = 1; + } // namespace #include <stdarg.h> #include <stdio.h> +#include <usrsctp.h> #include <memory> #include <unordered_map> @@ -43,7 +49,6 @@ enum PreservedErrno { #include "rtc_base/thread_annotations.h" #include "rtc_base/thread_checker.h" #include "rtc_base/trace_event.h" -#include "usrsctplib/usrsctp.h" namespace { @@ -62,7 +67,7 @@ ABSL_CONST_INIT rtc::GlobalLock g_usrsctp_lock_; // http://www.iana.org/assignments/sctp-parameters/sctp-parameters.xml // The value is not used by SCTP itself. It indicates the protocol running // on top of SCTP. -enum PayloadProtocolIdentifier { +enum { PPID_NONE = 0, // No protocol is specified. // Matches the PPIDs in mozilla source and // https://datatracker.ietf.org/doc/draft-ietf-rtcweb-data-protocol Sec. 9 @@ -143,7 +148,7 @@ void DebugSctpPrintf(const char* format, ...) { } // Get the PPID to use for the terminating fragment of this type. -PayloadProtocolIdentifier GetPpid(cricket::DataMessageType type) { +uint32_t GetPpid(cricket::DataMessageType type) { switch (type) { default: case cricket::DMT_NONE: @@ -157,8 +162,7 @@ PayloadProtocolIdentifier GetPpid(cricket::DataMessageType type) { } } -bool GetDataMediaType(PayloadProtocolIdentifier ppid, - cricket::DataMessageType* dest) { +bool GetDataMediaType(uint32_t ppid, cricket::DataMessageType* dest) { RTC_DCHECK(dest != NULL); switch (ppid) { case PPID_BINARY_PARTIAL: @@ -382,77 +386,10 @@ class SctpTransport::UsrSctpWrapper { int flags, void* ulp_info) { SctpTransport* transport = static_cast<SctpTransport*>(ulp_info); - // Post data to the transport's receiver thread (copying it). - // TODO(ldixon): Unclear if copy is needed as this method is responsible for - // memory cleanup. But this does simplify code. - const PayloadProtocolIdentifier ppid = - static_cast<PayloadProtocolIdentifier>( - rtc::NetworkToHost32(rcv.rcv_ppid)); - DataMessageType type = DMT_NONE; - if (!GetDataMediaType(ppid, &type) && !(flags & MSG_NOTIFICATION)) { - // It's neither a notification nor a recognized data packet. Drop it. - RTC_LOG(LS_ERROR) << "Received an unknown PPID " << ppid - << " on an SCTP packet. Dropping."; - free(data); - } else { - ReceiveDataParams params; - - params.sid = rcv.rcv_sid; - params.seq_num = rcv.rcv_ssn; - params.timestamp = rcv.rcv_tsn; - params.type = type; - - // Expect only continuation messages belonging to the same sid, the sctp - // stack should ensure this. - if ((transport->partial_incoming_message_.size() != 0) && - (rcv.rcv_sid != transport->partial_params_.sid)) { - // A message with a new sid, but haven't seen the EOR for the - // previous message. Deliver the previous partial message to avoid - // merging messages from different sid's. - transport->invoker_.AsyncInvoke<void>( - RTC_FROM_HERE, transport->network_thread_, - rtc::Bind(&SctpTransport::OnInboundPacketFromSctpToTransport, - transport, transport->partial_incoming_message_, - transport->partial_params_, transport->partial_flags_)); - - transport->partial_incoming_message_.Clear(); - } - - transport->partial_incoming_message_.AppendData( - reinterpret_cast<uint8_t*>(data), length); - transport->partial_params_ = params; - transport->partial_flags_ = flags; - - free(data); - - // Merge partial messages until they exceed the maximum send buffer size. - // This enables messages from a single send to be delivered in a single - // callback. Larger messages (originating from other implementations) will - // still be delivered in chunks. - if (!(flags & MSG_EOR) && - (transport->partial_incoming_message_.size() < kSctpSendBufferSize)) { - return 1; - } - - if (!(flags & MSG_EOR)) { - // TODO(bugs.webrtc.org/7774): We currently chunk messages if they are - // >= kSctpSendBufferSize. The better thing to do here is buffer up to - // the size negotiated in the SDP, and if a larger message is received - // close the channel and report the error. See discussion in the bug. - RTC_LOG(LS_WARNING) << "Chunking SCTP message without the EOR bit set."; - } - - // The ownership of the packet transfers to |invoker_|. Using - // CopyOnWriteBuffer is the most convenient way to do this. - transport->invoker_.AsyncInvoke<void>( - RTC_FROM_HERE, transport->network_thread_, - rtc::Bind(&SctpTransport::OnInboundPacketFromSctpToTransport, - transport, transport->partial_incoming_message_, params, - flags)); - - transport->partial_incoming_message_.Clear(); - } - return 1; + int result = + transport->OnDataOrNotificationFromSctp(data, length, rcv, flags); + free(data); + return result; } static SctpTransport* GetTransportFromSocket(struct socket* sock) { @@ -1132,31 +1069,120 @@ void SctpTransport::OnPacketFromSctpToNetwork( rtc::PacketOptions(), PF_NORMAL); } -void SctpTransport::OnInboundPacketFromSctpToTransport( - const rtc::CopyOnWriteBuffer& buffer, - ReceiveDataParams params, +int SctpTransport::InjectDataOrNotificationFromSctpForTesting( + void* data, + size_t length, + struct sctp_rcvinfo rcv, int flags) { - RTC_DCHECK_RUN_ON(network_thread_); - RTC_LOG(LS_VERBOSE) << debug_name_ - << "->OnInboundPacketFromSctpToTransport(...): " - "Received SCTP data:" - " sid=" - << params.sid - << " notification: " << (flags & MSG_NOTIFICATION) - << " length=" << buffer.size(); - // Sending a packet with data == NULL (no data) is SCTPs "close the - // connection" message. This sets sock_ = NULL; - if (!buffer.size() || !buffer.data()) { + return OnDataOrNotificationFromSctp(data, length, rcv, flags); +} + +int SctpTransport::OnDataOrNotificationFromSctp(void* data, + size_t length, + struct sctp_rcvinfo rcv, + int flags) { + // If data is NULL, the SCTP association has been closed. + if (!data) { RTC_LOG(LS_INFO) << debug_name_ - << "->OnInboundPacketFromSctpToTransport(...): " + << "->OnSctpInboundPacket(...): " "No data, closing."; - return; + return kSctpSuccessReturn; } + + // Handle notifications early. + // Note: Notifications are never split into chunks, so they can and should + // be handled early and entirely separate from the reassembly + // process. if (flags & MSG_NOTIFICATION) { - OnNotificationFromSctp(buffer); - } else { - OnDataFromSctpToTransport(params, buffer); + RTC_LOG(LS_VERBOSE) << debug_name_ + << "->OnSctpInboundPacket(...): SCTP notification" + << " length=" << length; + + // Copy and dispatch asynchronously + rtc::CopyOnWriteBuffer notification(reinterpret_cast<uint8_t*>(data), + length); + invoker_.AsyncInvoke<void>( + RTC_FROM_HERE, network_thread_, + rtc::Bind(&SctpTransport::OnNotificationFromSctp, this, notification)); + return kSctpSuccessReturn; } + + // Log data chunk + const uint32_t ppid = rtc::NetworkToHost32(rcv.rcv_ppid); + RTC_LOG(LS_VERBOSE) << debug_name_ + << "->OnSctpInboundPacket(...): SCTP data chunk" + << " length=" << length << ", sid=" << rcv.rcv_sid + << ", ppid=" << ppid << ", ssn=" << rcv.rcv_ssn + << ", cum-tsn=" << rcv.rcv_cumtsn + << ", eor=" << ((flags & MSG_EOR) ? "y" : "n"); + + // Validate payload protocol identifier + DataMessageType type = DMT_NONE; + if (!GetDataMediaType(ppid, &type)) { + // Unexpected PPID, dropping + RTC_LOG(LS_ERROR) << "Received an unknown PPID " << ppid + << " on an SCTP packet. Dropping."; + return kSctpSuccessReturn; + } + + // 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(); + } + + // Copy metadata of interest + ReceiveDataParams params; + params.type = type; + params.sid = rcv.rcv_sid; + // Note that the SSN is identical for each chunk of the same message. + // Furthermore, it is increased per stream and not on the whole + // association. + params.seq_num = rcv.rcv_ssn; + // There is no timestamp field in the SCTP API + params.timestamp = 0; + + // Append the chunk's data to the message buffer + partial_incoming_message_.AppendData(reinterpret_cast<uint8_t*>(data), + length); + partial_params_ = params; + partial_flags_ = flags; + + // If the message is not yet complete... + if (!(flags & MSG_EOR)) { + if (partial_incoming_message_.size() < kSctpSendBufferSize) { + // We still have space in the buffer. Continue buffering chunks until + // the message is complete before handing it out. + return kSctpSuccessReturn; + } else { + // The sender is exceeding the maximum message size that we announced. + // Spit out a warning but still hand out the partial message. Note that + // this behaviour is undesirable, see the discussion in issue 7774. + // + // TODO(lgrahl): Once sufficient time has passed and all supported + // browser versions obey the announced maximum message size, we should + // abort the SCTP association instead to prevent message integrity + // violation. + RTC_LOG(LS_ERROR) << "Handing out partial SCTP message."; + } + } + + // Dispatch the complete message. + // The ownership of the packet transfers to |invoker_|. Using + // CopyOnWriteBuffer is the most convenient way to do this. + invoker_.AsyncInvoke<void>( + RTC_FROM_HERE, network_thread_, + rtc::Bind(&SctpTransport::OnDataFromSctpToTransport, this, params, + partial_incoming_message_)); + + // Reset the message buffer + partial_incoming_message_.Clear(); + return kSctpSuccessReturn; } void SctpTransport::OnDataFromSctpToTransport( diff --git a/media/sctp/sctp_transport.h b/media/sctp/sctp_transport.h index 758503b509..38029ffeb3 100644 --- a/media/sctp/sctp_transport.h +++ b/media/sctp/sctp_transport.h @@ -34,6 +34,7 @@ // Defined by "usrsctplib/usrsctp.h" struct sockaddr_conn; struct sctp_assoc_change; +struct sctp_rcvinfo; struct sctp_stream_reset_event; struct sctp_sendv_spa; @@ -58,8 +59,8 @@ struct SctpInboundPacket; // 8. usrsctp_conninput(wrapped_data) // [network thread returns; sctp thread then calls the following] // 9. OnSctpInboundData(data) +// 10. SctpTransport::OnDataFromSctpToTransport(data) // [sctp thread returns having async invoked on the network thread] -// 10. SctpTransport::OnInboundPacketFromSctpToTransport(inboundpacket) // 11. SctpTransport::OnDataFromSctpToTransport(data) // 12. SctpTransport::SignalDataReceived(data) // [from the same thread, methods registered/connected to @@ -94,6 +95,10 @@ class SctpTransport : public SctpTransportInternal, void set_debug_name_for_testing(const char* debug_name) override { debug_name_ = debug_name; } + int InjectDataOrNotificationFromSctpForTesting(void* data, + size_t length, + struct sctp_rcvinfo rcv, + int flags); // Exposed to allow Post call from c-callbacks. // TODO(deadbeef): Remove this or at least make it return a const pointer. @@ -173,14 +178,17 @@ class SctpTransport : public SctpTransportInternal, // Called using |invoker_| to send packet on the network. void OnPacketFromSctpToNetwork(const rtc::CopyOnWriteBuffer& buffer); - // Called using |invoker_| to decide what to do with the packet. - // The |flags| parameter is used by SCTP to distinguish notification packets - // from other types of packets. - void OnInboundPacketFromSctpToTransport(const rtc::CopyOnWriteBuffer& buffer, - ReceiveDataParams params, - int flags); + + // Called on the SCTP thread. + // Flags are standard socket API flags (RFC 6458). + int OnDataOrNotificationFromSctp(void* data, + size_t length, + struct sctp_rcvinfo rcv, + int flags); + // Called using |invoker_| to decide what to do with the data. void OnDataFromSctpToTransport(const ReceiveDataParams& params, const rtc::CopyOnWriteBuffer& buffer); + // Called using |invoker_| to decide what to do with the notification. void OnNotificationFromSctp(const rtc::CopyOnWriteBuffer& buffer); void OnNotificationAssocChange(const sctp_assoc_change& change); diff --git a/media/sctp/sctp_transport_unittest.cc b/media/sctp/sctp_transport_unittest.cc index da6c6290fd..540d2bd5e4 100644 --- a/media/sctp/sctp_transport_unittest.cc +++ b/media/sctp/sctp_transport_unittest.cc @@ -12,6 +12,7 @@ #include <stdio.h> #include <string.h> +#include <usrsctp.h> #include <memory> #include <string> @@ -238,6 +239,73 @@ class SctpTransportTest : public ::testing::Test, public sigslot::has_slots<> { void OnChan2ReadyToSend() { ++transport2_ready_to_send_count_; } }; +TEST_F(SctpTransportTest, MessageInterleavedWithNotification) { + FakeDtlsTransport fake_dtls1("fake dtls 1", 0); + FakeDtlsTransport fake_dtls2("fake dtls 2", 0); + SctpFakeDataReceiver recv1; + SctpFakeDataReceiver recv2; + std::unique_ptr<SctpTransport> transport1( + CreateTransport(&fake_dtls1, &recv1)); + std::unique_ptr<SctpTransport> transport2( + CreateTransport(&fake_dtls2, &recv2)); + + // Add a stream. + transport1->OpenStream(1); + transport2->OpenStream(1); + + // Start SCTP transports. + transport1->Start(kSctpDefaultPort, kSctpDefaultPort, kSctpSendBufferSize); + transport2->Start(kSctpDefaultPort, kSctpDefaultPort, kSctpSendBufferSize); + + // Connect the two fake DTLS transports. + fake_dtls1.SetDestination(&fake_dtls2, false); + + // Ensure the SCTP association has been established + // Note: I'd rather watch for an assoc established state here but couldn't + // find any exposed... + SendDataResult result; + ASSERT_TRUE(SendData(transport2.get(), 1, "meow", &result)); + EXPECT_TRUE_WAIT(ReceivedData(&recv1, 1, "meow"), kDefaultTimeout); + + // Detach the DTLS transport to ensure only we will inject packets from here + // on. + transport1->SetDtlsTransport(nullptr); + + // Prepare chunk buffer and metadata + auto chunk = rtc::CopyOnWriteBuffer(32); + struct sctp_rcvinfo meta = {0}; + meta.rcv_sid = 1; + meta.rcv_ssn = 1337; + meta.rcv_ppid = rtc::HostToNetwork32(51); // text (complete) + + // Inject chunk 1/2. + meta.rcv_tsn = 42; + meta.rcv_cumtsn = 42; + chunk.SetData("meow?", 5); + EXPECT_EQ(1, transport1->InjectDataOrNotificationFromSctpForTesting( + chunk.data(), chunk.size(), meta, 0)); + + // Inject a notification in between chunks. + union sctp_notification notification; + memset(¬ification, 0, sizeof(notification)); + // Type chosen since it's not handled apart from being logged + notification.sn_header.sn_type = SCTP_PEER_ADDR_CHANGE; + notification.sn_header.sn_flags = 0; + notification.sn_header.sn_length = sizeof(notification); + EXPECT_EQ(1, transport1->InjectDataOrNotificationFromSctpForTesting( + ¬ification, sizeof(notification), {0}, MSG_NOTIFICATION)); + + // Inject chunk 2/2 + meta.rcv_tsn = 42; + meta.rcv_cumtsn = 43; + chunk.SetData(" rawr!", 6); + EXPECT_EQ(1, transport1->InjectDataOrNotificationFromSctpForTesting( + chunk.data(), chunk.size(), meta, MSG_EOR)); + + // Expect the message to contain both chunks. + EXPECT_TRUE_WAIT(ReceivedData(&recv1, 1, "meow? rawr!"), kDefaultTimeout); +} + // Test that data can be sent end-to-end when an SCTP transport starts with one // transport (which is unwritable), and then switches to another transport. A // common scenario due to how BUNDLE works. |