Handle case where UDP packet contains multiple DTLS records.
Our DTLS implementation doesn't do this, but other implementations may; see https://tools.ietf.org/html/rfc6347#section-4.1.1.
BUG=chromium:537189
Review-Url: https://codereview.webrtc.org/2970883005
Cr-Commit-Position: refs/heads/master@{#18934}
diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc
index 4249ae7..08dfb6b 100644
--- a/webrtc/p2p/base/dtlstransportchannel.cc
+++ b/webrtc/p2p/base/dtlstransportchannel.cc
@@ -537,20 +537,25 @@
char buf[kMaxDtlsPacketLen];
size_t read;
int read_error;
- rtc::StreamResult ret = dtls_->Read(buf, sizeof(buf), &read, &read_error);
- if (ret == rtc::SR_SUCCESS) {
- SignalReadPacket(this, buf, read, rtc::CreatePacketTime(0), 0);
- } else if (ret == rtc::SR_EOS) {
- // Remote peer shut down the association with no error.
- LOG_J(LS_INFO, this) << "DTLS transport closed";
- set_writable(false);
- set_dtls_state(DTLS_TRANSPORT_CLOSED);
- } else if (ret == rtc::SR_ERROR) {
- // Remote peer shut down the association with an error.
- LOG_J(LS_INFO, this) << "DTLS transport error, code=" << read_error;
- set_writable(false);
- set_dtls_state(DTLS_TRANSPORT_FAILED);
- }
+ rtc::StreamResult ret;
+ // The underlying DTLS stream may have received multiple DTLS records in
+ // one packet, so read all of them.
+ do {
+ ret = dtls_->Read(buf, sizeof(buf), &read, &read_error);
+ if (ret == rtc::SR_SUCCESS) {
+ SignalReadPacket(this, buf, read, rtc::CreatePacketTime(0), 0);
+ } else if (ret == rtc::SR_EOS) {
+ // Remote peer shut down the association with no error.
+ LOG_J(LS_INFO, this) << "DTLS transport closed";
+ set_writable(false);
+ set_dtls_state(DTLS_TRANSPORT_CLOSED);
+ } else if (ret == rtc::SR_ERROR) {
+ // Remote peer shut down the association with an error.
+ LOG_J(LS_INFO, this) << "DTLS transport error, code=" << read_error;
+ set_writable(false);
+ set_dtls_state(DTLS_TRANSPORT_FAILED);
+ }
+ } while (ret == rtc::SR_SUCCESS);
}
if (sig & rtc::SE_CLOSE) {
RTC_DCHECK(sig == rtc::SE_CLOSE); // SE_CLOSE should be by itself.
diff --git a/webrtc/p2p/base/dtlstransportchannel_unittest.cc b/webrtc/p2p/base/dtlstransportchannel_unittest.cc
index 225d6f7..e5e7a6d 100644
--- a/webrtc/p2p/base/dtlstransportchannel_unittest.cc
+++ b/webrtc/p2p/base/dtlstransportchannel_unittest.cc
@@ -670,6 +670,22 @@
TestTransfer(1, 1000, 100, false);
}
+// Connect with DTLS, combine multiple DTLS records into one packet.
+// Our DTLS implementation doesn't do this, but other implementations may;
+// see https://tools.ietf.org/html/rfc6347#section-4.1.1.
+// This has caused interoperability problems with ORTCLib in the past.
+TEST_F(DtlsTransportChannelTest, TestTransferDtlsCombineRecords) {
+ PrepareDtls(true, true, rtc::KT_DEFAULT);
+ ASSERT_TRUE(Connect());
+ // Our DTLS implementation always sends one record per packet, so to simulate
+ // an endpoint that sends multiple records per packet, we configure the fake
+ // ICE transport to combine every two consecutive packets into a single
+ // packet.
+ cricket::FakeIceTransport* transport = client1_.GetFakeIceTransort(0);
+ transport->combine_outgoing_packets(true);
+ TestTransfer(0, 500, 100, false);
+}
+
// Connect with A doing DTLS and B not, and transfer some data.
TEST_F(DtlsTransportChannelTest, TestTransferDtlsRejected) {
PrepareDtls(true, false, rtc::KT_DEFAULT);
diff --git a/webrtc/p2p/base/fakeicetransport.h b/webrtc/p2p/base/fakeicetransport.h
index d9c8485..eff246e 100644
--- a/webrtc/p2p/base/fakeicetransport.h
+++ b/webrtc/p2p/base/fakeicetransport.h
@@ -161,6 +161,11 @@
// Fake PacketTransportInternal implementation.
bool writable() const override { return writable_; }
bool receiving() const override { return receiving_; }
+ // If combine is enabled, every two consecutive packets to be sent with
+ // "SendPacket" will be combined into one outgoing packet.
+ void combine_outgoing_packets(bool combine) {
+ combine_outgoing_packets_ = combine;
+ }
int SendPacket(const char* data,
size_t len,
const rtc::PacketOptions& options,
@@ -168,14 +173,18 @@
if (!dest_) {
return -1;
}
- rtc::CopyOnWriteBuffer packet(data, len);
- if (async_) {
- invoker_.AsyncInvokeDelayed<void>(
- RTC_FROM_HERE, rtc::Thread::Current(),
- rtc::Bind(&FakeIceTransport::SendPacketInternal, this, packet),
- async_delay_ms_);
- } else {
- SendPacketInternal(packet);
+
+ send_packet_.AppendData(data, len);
+ if (!combine_outgoing_packets_ || send_packet_.size() > len) {
+ rtc::CopyOnWriteBuffer packet(std::move(send_packet_));
+ if (async_) {
+ invoker_.AsyncInvokeDelayed<void>(
+ RTC_FROM_HERE, rtc::Thread::Current(),
+ rtc::Bind(&FakeIceTransport::SendPacketInternal, this, packet),
+ async_delay_ms_);
+ } else {
+ SendPacketInternal(packet);
+ }
}
rtc::SentPacket sent_packet(options.packet_id, rtc::TimeMillis());
SignalSentPacket(this, sent_packet);
@@ -233,6 +242,8 @@
bool had_connection_ = false;
bool writable_ = false;
bool receiving_ = false;
+ bool combine_outgoing_packets_ = false;
+ rtc::CopyOnWriteBuffer send_packet_;
};
} // namespace cricket