commit c508216d6aed145b40adfe4327a8194b7b49481f
author Philipp Hancke <phancke@meta.com> Tue Jul 01 20:58:20 2025
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jul 03 15:25:10 2025
tree 16941faccaa415e36e436c4a191f1dbcb2debd98
parent 45835727fd4c471b8fe606ca3eed21ecdfd4bfd0

dtls-in-stun: add "implicit ack" for binding responses

Binding responses now implicitly ack the data that was sent in the original request.
This allows a receiver to omit the hash in the response (not implemented yet)

Note:
* duplicates are ignored
* the implicit ack is only passed when there is an ack attribute

Bug: webrtc:367395350
Change-Id: I80bcf1c0031bbe92a9e715f27222a458977726de
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/394141
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#45086}

p2p/BUILD.gn

diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn
index 4324972..01fe103 100644
--- a/p2p/BUILD.gn
+++ b/p2p/BUILD.gn
@@ -182,6 +182,7 @@
     ":candidate_pair_interface",
     ":connection_info",
     ":dtls_stun_piggyback_controller",
+    ":dtls_utils",
     ":ice_credentials_iterator",
     ":p2p_constants",
     ":p2p_transport_channel_ice_field_trials",

p2p/base/connection.cc

diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc
index a4207d4..11fabca 100644
--- a/p2p/base/connection.cc
+++ b/p2p/base/connection.cc
@@ -40,6 +40,7 @@
 #include "p2p/base/stun_request.h"
 #include "p2p/base/transport_description.h"
 #include "p2p/dtls/dtls_stun_piggyback_callbacks.h"
+#include "p2p/dtls/dtls_utils.h"
 #include "rtc_base/async_packet_socket.h"
 #include "rtc_base/byte_buffer.h"
 #include "rtc_base/checks.h"
@@ -629,7 +630,9 @@
   }
 }
 
-void Connection::MaybeHandleDtlsPiggybackingAttributes(const StunMessage* msg) {
+void Connection::MaybeHandleDtlsPiggybackingAttributes(
+    const StunMessage* msg,
+    const StunRequest* original_request) {
   if (dtls_stun_piggyback_callbacks_.empty()) {
     return;
   }
@@ -645,6 +648,20 @@
   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);
+    }
+  }
   dtls_stun_piggyback_callbacks_.recv_data(piggyback_data, piggyback_acks);
 }
 
@@ -690,7 +707,7 @@
 
   // This is a validated stun request from remote peer.
   if (msg->type() == STUN_BINDING_REQUEST) {
-    MaybeHandleDtlsPiggybackingAttributes(msg);
+    MaybeHandleDtlsPiggybackingAttributes(msg, /*original_request=*/nullptr);
     SendStunBindingResponse(msg);
   } else {
     RTC_DCHECK(msg->type() == GOOG_PING_REQUEST);
@@ -1558,7 +1575,7 @@
   const bool sent_dtls_piggyback_ack =
       request->msg()->GetByteString(STUN_ATTR_META_DTLS_IN_STUN_ACK) != nullptr;
   if (sent_dtls_piggyback || sent_dtls_piggyback_ack) {
-    MaybeHandleDtlsPiggybackingAttributes(response);
+    MaybeHandleDtlsPiggybackingAttributes(response, request);
   }
 }
 

p2p/base/connection.h

diff --git a/p2p/base/connection.h b/p2p/base/connection.h
index 4c08384..b62ed99 100644
--- a/p2p/base/connection.h
+++ b/p2p/base/connection.h
@@ -519,7 +519,9 @@
       received_packet_callback_;
 
   void MaybeAddDtlsPiggybackingAttributes(StunMessage* msg);
-  void MaybeHandleDtlsPiggybackingAttributes(const StunMessage* msg);
+  void MaybeHandleDtlsPiggybackingAttributes(
+      const StunMessage* msg,
+      const StunRequest* original_request);
   DtlsStunPiggybackCallbacks dtls_stun_piggyback_callbacks_;
 };
 

p2p/dtls/dtls_stun_piggyback_controller_unittest.cc

diff --git a/p2p/dtls/dtls_stun_piggyback_controller_unittest.cc b/p2p/dtls/dtls_stun_piggyback_controller_unittest.cc
index 4060e15..ebbaf0b 100644
--- a/p2p/dtls/dtls_stun_piggyback_controller_unittest.cc
+++ b/p2p/dtls/dtls_stun_piggyback_controller_unittest.cc
@@ -450,4 +450,13 @@
             std::string(dtls_flight2.begin(), dtls_flight2.end()));
 }
 
+TEST_F(DtlsStunPiggybackControllerTest, DuplicateAck) {
+  server_.CapturePacket(dtls_flight1);
+  server_.Flush();
+  server_.ReportDataPiggybacked(
+      std::nullopt,
+      std::vector<uint32_t>({ComputeDtlsPacketHash(dtls_flight1),
+                             ComputeDtlsPacketHash(dtls_flight1)}));
+}
+
 }  // namespace webrtc