sped updates

api/uma_metrics.h

@@ -252,6 +252,8 @@ enum SdpMungingType {
   kDataChannelSctpInit = 100,
   kDataChannelMaxMessageSize = 101,
   kDataChannelSctpPort = 102,
+  // Overflow area.
+  kIceOptionsSped = 110,
   kMaxValue,
 };
 

experiments/field_trials.py

@@ -98,6 +98,9 @@ def bug_url(self) -> str:
     FieldTrial('WebRTC-DisableSslGroupIds',
                404763475,
                date(2025,9,1)),
+    FieldTrial('WebRTC-DtlsStunPiggybackControllerSped',
+               367395350,
+               date(2027, 1, 1)),
     FieldTrial('WebRTC-ElasticBitrateAllocation',
                350555527,
                date(2025, 3, 1)),
@@ -130,7 +133,7 @@ def bug_url(self) -> str:
                date(2024, 4, 1)),
     FieldTrial('WebRTC-IceHandshakeDtls',
                367395350,
-               date(2026, 1, 1)),
+               date(2027, 1, 1)),
     FieldTrial('WebRTC-IncomingTimestampOnMarkerBitOnly',
                42224805,
                date(2024, 4, 1)),

p2p/BUILD.gn

@@ -666,6 +666,9 @@ rtc_library("dtls_stun_piggyback_controller") {
     "dtls/dtls_stun_piggyback_callbacks.h",
     "dtls/dtls_stun_piggyback_controller.cc",
     "dtls/dtls_stun_piggyback_controller.h",
+    "dtls/dtls_stun_piggyback_controller_interface.h",
+    "dtls/dtls_stun_piggyback_controller_sped.cc",
+    "dtls/dtls_stun_piggyback_controller_sped.h",
   ]
   deps = [
     ":dtls_utils",

p2p/base/connection.cc

@@ -652,20 +652,9 @@ void Connection::MaybeHandleDtlsPiggybackingAttributes(
   if (dtls_piggyback_ack != nullptr) {
     piggyback_acks = dtls_piggyback_ack->GetUInt32Vector();
   }
-  // A response implicitly acknowledges the original embedded packet
-  // when the ack attribute is included.
-  if (dtls_piggyback_ack != nullptr && original_request != nullptr) {
-    const StunByteStringAttribute* request_dtls_piggyback =
-        original_request->msg()->GetByteString(STUN_ATTR_META_DTLS_IN_STUN);
-    if (request_dtls_piggyback) {
-      uint32_t sent_hash =
-          ComputeDtlsPacketHash(request_dtls_piggyback->array_view());
-      if (!piggyback_acks) {
-        piggyback_acks = {};
-      }
-      piggyback_acks->push_back(sent_hash);
-    }
-  }
+  // TODO: bugs.webrtc.org/367395350 - a binding response could
+  // implicitly acknowledge data sent in its associated binding
+  // request.
   dtls_stun_piggyback_callbacks_.recv_data(piggyback_data, piggyback_acks);
 }
 

p2p/base/connection_unittest.cc

@@ -92,7 +92,7 @@ class ConnectionTest : public ::testing::Test {
   void SendPingAndCaptureReply(Connection* lconn,
                                Connection* rconn,
                                int64_t ms,
-                               BufferT<uint8_t>* reply) {
+                               Buffer* reply) {
     TestPort* lport =
         lconn->PortForTest() == lport_.get() ? lport_.get() : rport_.get();
     TestPort* rport =
@@ -116,7 +116,7 @@ class ConnectionTest : public ::testing::Test {
   void SendPingAndReceiveResponse(Connection* lconn,
                                   Connection* rconn,
                                   int64_t ms) {
-    BufferT<uint8_t> reply;
+    Buffer reply;
     SendPingAndCaptureReply(lconn, rconn, ms, &reply);
 
     lconn->OnReadPacket(ReceivedIpPacket(reply, SocketAddress(), std::nullopt));
@@ -206,7 +206,7 @@ TEST_F(ConnectionTest, ConnectionForgetLearnedStateDiscardsPendingPings) {
   EXPECT_TRUE(lconn->writable());
   EXPECT_TRUE(lconn->receiving());
 
-  BufferT<uint8_t> reply;
+  Buffer reply;
   SendPingAndCaptureReply(lconn, rconn, 10, &reply);
 
   lconn->ForgetLearnedState();
@@ -405,5 +405,76 @@ TEST_F(ConnectionTest, TooBigDeltaIsNotSent) {
   EXPECT_FALSE(received_goog_delta_ack);
 }
 
+class DtlsStunPiggybackConnectionTest : public ConnectionTest {};
+
+TEST_F(DtlsStunPiggybackConnectionTest, Callbacks) {
+  std::optional<size_t> request_data_size;
+  std::optional<size_t> request_ack_size;
+  std::optional<size_t> response_data_size;
+  std::optional<size_t> response_ack_size;
+
+  Connection* lconn = CreateConnection(ICEROLE_CONTROLLING);
+  lconn->RegisterDtlsPiggyback(DtlsStunPiggybackCallbacks(
+      [&](auto type) {
+        std::optional<absl::string_view> data = "request";
+        std::optional<std::vector<uint32_t>> ack = {{0}};
+        return std::make_pair(data, ack);
+      },
+      [&](auto data, auto ack) {
+        if (data)
+          response_data_size = data->size();
+        if (ack)
+          response_ack_size = ack->size();
+      }));
+  Connection* rconn = CreateConnection(ICEROLE_CONTROLLED);
+  rconn->RegisterDtlsPiggyback(DtlsStunPiggybackCallbacks(
+      [&](auto type) { return std::make_pair(std::nullopt, std::nullopt); },
+      [&](auto data, auto ack) {
+        if (data)
+          request_data_size = data->size();
+        if (ack)
+          request_ack_size = ack->size();
+      }));
+  Buffer reply;
+  SendPingAndCaptureReply(lconn, rconn, env().clock().CurrentTime().ms(),
+                          &reply);
+  lconn->OnReadPacket(ReceivedIpPacket(reply, SocketAddress(), std::nullopt));
+
+  EXPECT_EQ(request_data_size, 7);
+  EXPECT_EQ(request_ack_size, 1);
+  EXPECT_EQ(response_data_size, std::nullopt);
+  EXPECT_EQ(response_ack_size, std::nullopt);
+}
+
+TEST_F(DtlsStunPiggybackConnectionTest, NoImplicitDtlsInStunAck) {
+  std::optional<size_t> ack_size;
+
+  Connection* lconn = CreateConnection(ICEROLE_CONTROLLING);
+  lconn->RegisterDtlsPiggyback(DtlsStunPiggybackCallbacks(
+      [&](auto type) {
+        std::optional<absl::string_view> data = "test";
+        std::optional<std::vector<uint32_t>> ack;
+        return std::make_pair(data, ack);
+      },
+      [&](auto data, auto ack) {
+        if (ack)
+          ack_size = ack->size();
+      }));
+  Connection* rconn = CreateConnection(ICEROLE_CONTROLLED);
+  rconn->RegisterDtlsPiggyback(DtlsStunPiggybackCallbacks(
+      [&](auto type) {
+        std::vector<uint32_t> empty;
+        std::optional<absl::string_view> data;
+        std::optional<std::vector<uint32_t>> ack = empty;
+        return std::make_pair(data, ack);
+      },
+      [&](auto data, auto ack) {}));
+  Buffer reply;
+  SendPingAndCaptureReply(lconn, rconn, env().clock().CurrentTime().ms(),
+                          &reply);
+  lconn->OnReadPacket(ReceivedIpPacket(reply, SocketAddress(), std::nullopt));
+  EXPECT_EQ(ack_size, 0);
+}
+
 }  // namespace
 }  // namespace webrtc

p2p/base/transport_description.h

@@ -97,6 +97,8 @@ struct IceParameters {
 constexpr auto* ICE_OPTION_TRICKLE = "trickle";
 constexpr auto* ICE_OPTION_RENOMINATION = "renomination";
 constexpr auto* ICE_OPTION_GOOG_SPED_V1 = "goog-sped-v1";
+// STUN Protocol for Embedding DTLS.
+constexpr auto* ICE_OPTION_SPED = "sped";
 
 std::optional<ConnectionRole> StringToConnectionRole(
     absl::string_view role_str);

p2p/base/transport_description_factory.cc

@@ -48,6 +48,9 @@ std::unique_ptr<TransportDescription> TransportDescriptionFactory::CreateOffer(
   if (options.enable_ice_renomination) {
     desc->AddOption(ICE_OPTION_RENOMINATION);
   }
+  if (options.dtls_handshake_in_stun) {
+    desc->AddOption(ICE_OPTION_SPED);
+  }
 
   if (SSLStreamAdapter::IsBoringSsl() &&
       field_trials_.IsEnabled("WebRTC-IceHandshakeDtls") &&
@@ -105,7 +108,9 @@ std::unique_ptr<TransportDescription> TransportDescriptionFactory::CreateAnswer(
        current_description->HasOption(ICE_OPTION_GOOG_SPED_V1))) {
     desc->AddOption(ICE_OPTION_GOOG_SPED_V1);
   }
-
+  if (options.dtls_handshake_in_stun) {
+    desc->AddOption(ICE_OPTION_SPED);
+  }
   // Special affordance for testing: Answer without DTLS params
   // if we are insecure without a certificate, or if we are
   // insecure with a non-DTLS offer.

p2p/base/transport_description_factory.h

@@ -28,6 +28,9 @@ struct TransportOptions {
   // If true, ICE renomination is supported and will be used if it is also
   // supported by the remote side.
   bool enable_ice_renomination = false;
+  // If true, SPED (STUN Protocol for Embedding DTLS) is supported and will
+  // be used if it is also supported by the remote side.
+  bool dtls_handshake_in_stun = false;
 };
 
 // Creates transport descriptions according to the supplied configuration.

p2p/base/transport_description_factory_unittest.cc

@@ -454,6 +454,28 @@ TEST_F(
   EXPECT_FALSE(new_answer->HasOption(ICE_OPTION_GOOG_SPED_V1));
 }
 
+TEST_F(TransportDescriptionFactoryTest, AddsDtlsInStunIceOption) {
+  webrtc::TransportOptions options;
+  options.dtls_handshake_in_stun = true;
+  std::unique_ptr<TransportDescription> offer =
+      f1_.CreateOffer(options, nullptr, &ice_credentials_);
+  ASSERT_THAT(offer, NotNull());
+  EXPECT_TRUE(offer->HasOption("sped"));
+  std::unique_ptr<TransportDescription> answer =
+      f2_.CreateAnswer(offer.get(), options, true, nullptr, &ice_credentials_);
+  EXPECT_TRUE(answer->HasOption("sped"));
+
+  options.dtls_handshake_in_stun = false;
+  std::unique_ptr<TransportDescription> offer2 =
+      f1_.CreateOffer(options, nullptr, &ice_credentials_);
+  ASSERT_THAT(offer2, NotNull());
+  EXPECT_FALSE(offer2->HasOption("sped"));
+  options.dtls_handshake_in_stun = true;
+  std::unique_ptr<TransportDescription> answer2 =
+      f2_.CreateAnswer(offer2.get(), options, true, nullptr, &ice_credentials_);
+  EXPECT_TRUE(answer2->HasOption("sped"));
+}
+
 // Test CreateOffer with IceCredentialsIterator.
 TEST_F(TransportDescriptionFactoryTest, CreateOfferIceCredentialsIterator) {
   std::vector<webrtc::IceParameters> credentials = {

p2p/dtls/dtls_ice_integration_fixture.h

@@ -559,6 +559,11 @@ class Base {
         ep.env, ep.ice_transport, crypto_options,
         ep.config.max_protocol_version);
 
+    // No SDP exchange in this fixture; stand in for JsepTransport's call.
+    if (ep.config.dtls_in_stun) {
+      ep.dtls->MaybeStartDtlsInStun();
+    }
+
     if (ice_lite_agent) {
       ep.dtls->SetFakeIceLite();
     }

p2p/dtls/dtls_stun_piggyback_controller.cc

@@ -140,6 +140,12 @@ DtlsStunPiggybackController::GetDataToPiggyback(
   RTC_DCHECK(!writing_packets_);
 
   if (pending_packets_.empty()) {
+    // In confirmed state include an empty data attribute. Can happen e.g.
+    // with PQC after receiving a partial flight.
+    // In unconfirmed and pending states do not include the attribute.
+    if (state_ == State::CONFIRMED) {
+      return "";
+    }
     return std::nullopt;
   }
 

p2p/dtls/dtls_stun_piggyback_controller.h

@@ -20,6 +20,7 @@
 #include "absl/strings/string_view.h"
 #include "api/sequence_checker.h"
 #include "api/transport/stun.h"
+#include "p2p/dtls/dtls_stun_piggyback_controller_interface.h"
 #include "p2p/dtls/dtls_utils.h"
 #include "rtc_base/network/received_packet.h"
 #include "rtc_base/system/no_unique_address.h"
@@ -29,7 +30,8 @@ namespace webrtc {
 
 // This class is not thread safe; all methods must be called on the same thread
 // as the constructor.
-class DtlsStunPiggybackController {
+class DtlsStunPiggybackController
+    : public DtlsStunPiggybackControllerInterface {
  public:
   // Never ack more than 4 packets.
   static constexpr unsigned kMaxAckSize = 4;
@@ -41,69 +43,54 @@ class DtlsStunPiggybackController {
       // NOLINTNEXTLINE(readability/casting) - not a cast; false positive!
       absl::AnyInvocable<void(bool) &&> piggyback_complete_callback);
 
-  ~DtlsStunPiggybackController();
-
-  enum class State {
-    // We don't know if peer support DTLS piggybacked in STUN.
-    // We will piggyback DTLS until we get a piggybacked response
-    // or a STUN response with piggyback support.
-    TENTATIVE = 0,
-    // The peer supports DTLS in STUN and we continue the handshake.
-    CONFIRMED = 1,
-    // We are waiting for the final ack. Semantic differs depending
-    // on DTLS role.
-    PENDING = 2,
-    // We successfully completed the DTLS handshake in STUN.
-    COMPLETE = 3,
-    // The peer does not support piggybacking DTLS in STUN.
-    OFF = 4,
-  };
-
-  State state() const {
+  ~DtlsStunPiggybackController() override;
+
+  State state() const override {
     RTC_DCHECK_RUN_ON(&sequence_checker_);
     return state_;
   }
 
   // Called by DtlsTransport when the handshake is complete "locally",
   // i.e. we can send encrypted packets to peer (but we don't strictly know
   // that peer can decode them).
-  void SetDtlsHandshakeComplete(bool is_dtls_client, bool is_dtls13);
+  void SetDtlsHandshakeComplete(bool is_dtls_client, bool is_dtls13) override;
 
   // Called by DtlsTransport when a packet has been received and passed
   // to layers above us. This means that dtls is writable for the peer,
   // and maybe we are complete.
-  void ApplicationPacketReceived(const ReceivedIpPacket& packet);
+  void ApplicationPacketReceived(const ReceivedIpPacket& packet) override;
 
   // Called by DtlsTransport when DTLS failed.
-  void SetDtlsFailed();
+  void SetDtlsFailed() override;
 
   // Intercepts DTLS packets which should go into the STUN packets during the
   // handshake.
-  void CapturePacket(std::span<const uint8_t> data);
-  void ClearCachedPacketForTesting();
+  void CapturePacket(std::span<const uint8_t> data) override;
+  void ClearCachedPacketForTesting() override;
 
   // Inform piggybackcontroller that a flight is complete.
-  void Flush();
+  void Flush() override;
 
   // Called by Connection, when sending a STUN BINDING { REQUEST / RESPONSE }
   // to obtain optional DTLS data or ACKs.
   std::optional<absl::string_view> GetDataToPiggyback(
-      StunMessageType stun_message_type);
+      StunMessageType stun_message_type) override;
   std::optional<const std::vector<uint32_t>> GetAckToPiggyback(
-      StunMessageType stun_message_type);
-  std::vector<std::span<const uint8_t>> GetPending();
+      StunMessageType stun_message_type) override;
+  std::vector<std::span<const uint8_t>> GetPending() override;
 
   // Called by Connection when receiving a STUN BINDING { REQUEST / RESPONSE }.
-  void ReportDataPiggybacked(std::optional<std::span<uint8_t>> data,
-                             std::optional<std::vector<uint32_t>> acks);
+  void ReportDataPiggybacked(
+      std::optional<std::span<uint8_t>> data,
+      std::optional<std::vector<uint32_t>> acks) override;
 
   // Called by
   // * DTLSTransport when receiving a DTLS packet (possibly after the packet
   //   was emitted by this class).
   // * This class when processing a DTLS packet.
-  void ReportDtlsPacket(std::span<const uint8_t> data);
+  void ReportDtlsPacket(std::span<const uint8_t> data) override;
 
-  int GetCountOfReceivedData() const { return data_recv_count_; }
+  int GetCountOfReceivedData() const override { return data_recv_count_; }
 
  private:
   State state_ RTC_GUARDED_BY(sequence_checker_) = State::TENTATIVE;