Change tests to not add attributes after STUN signatures.

This is the behavior changes that are needed to check for correct
behavior downstream.

Bug: chromium:504567957
Change-Id: I9e9d4a54d0408146ca3a2175b0ee534f8bf212ad
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/465981
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Auto-Submit: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#47517}

api/transport/BUILD.gn

diff --git a/api/transport/BUILD.gn b/api/transport/BUILD.gn
index a9c55b0..c83d96a 100644
--- a/api/transport/BUILD.gn
+++ b/api/transport/BUILD.gn
@@ -115,6 +115,7 @@
     "../../rtc_base:logging",
     "../../rtc_base:net_helpers",
     "../../rtc_base:socket_address",
+    "../../rtc_base:stringutils",
     "../../system_wrappers:metrics",
     "//third_party/abseil-cpp/absl/strings:string_view",
   ]

p2p/base/port_unittest.cc

diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc
index 16ecc52..04d159f 100644
--- a/p2p/base/port_unittest.cc
+++ b/p2p/base/port_unittest.cc
@@ -2635,12 +2635,12 @@
 
   // Build ordinary message with valid ufrag/pass.
   in_msg = CreateStunMessageWithUsername(STUN_BINDING_REQUEST, "rfrag:lfrag");
-  in_msg->AddMessageIntegrity("rpass");
   // Add a couple attributes with ID in comprehension-required range.
   in_msg->AddAttribute(StunAttribute::CreateUInt32(0x7777));
   in_msg->AddAttribute(StunAttribute::CreateUInt32(0x4567));
   // ... And one outside the range.
   in_msg->AddAttribute(StunAttribute::CreateUInt32(0xdead));
+  in_msg->AddMessageIntegrity("rpass");
   in_msg->AddFingerprint();
   WriteStunMessage(*in_msg, buf.get());
   ASSERT_TRUE(GetStunMessageFromBufferWriter(port.get(), buf.get(), addr,
@@ -2694,7 +2694,9 @@
       IsRtcOk());
   auto modified_response = rport->last_stun_msg()->Clone();
   modified_response->AddAttribute(StunAttribute::CreateUInt32(0x7777));
+  modified_response->RemoveAttribute(STUN_ATTR_MESSAGE_INTEGRITY);
   modified_response->RemoveAttribute(STUN_ATTR_FINGERPRINT);
+  modified_response->AddMessageIntegrity("rpass");
   modified_response->AddFingerprint();
   ByteBufferWriter buf;
   WriteStunMessage(*modified_response, &buf);

p2p/base/turn_port.cc

diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc
index 1a6f3a7..a0df6b5 100644
--- a/p2p/base/turn_port.cc
+++ b/p2p/base/turn_port.cc
@@ -1377,12 +1377,12 @@
       StunAttribute::CreateUInt32(STUN_ATTR_REQUESTED_TRANSPORT);
   transport_attr->SetValue(IPPROTO_UDP << 24);
   message->AddAttribute(std::move(transport_attr));
+  port_->MaybeAddTurnLoggingId(message);
   if (!port_->hash().empty()) {
     port_->AddRequestAuthInfo(message);
   } else {
     SetAuthenticationRequired(false);
   }
-  port_->MaybeAddTurnLoggingId(message);
   port_->TurnCustomizerMaybeModifyOutgoingStunMessage(message);
 }
 

p2p/base/turn_port_unittest.cc

diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc
index 6823fbd..8a53aae 100644
--- a/p2p/base/turn_port_unittest.cc
+++ b/p2p/base/turn_port_unittest.cc
@@ -2011,7 +2011,9 @@
 
 // Do a TURN allocation, establish a TLS connection, and send some data.
 // Add customizer and check that it can add attribute to messages.
-TEST_F(TurnPortTest, TestTurnCustomizerAddAttribute) {
+// This destroys the integrity checks on the message.
+// TODO: crbug.com/504567957 - enable once integrity issues have been sorted.
+TEST_F(TurnPortTest, DISABLED_TestTurnCustomizerAddAttribute) {
   unsigned int observer_message_counter = 0;
   unsigned int observer_channel_data_counter = 0;
   unsigned int observer_attr_counter = 0;

p2p/test/test_turn_customizer.h

diff --git a/p2p/test/test_turn_customizer.h b/p2p/test/test_turn_customizer.h
index ae693c1..69cfcb6 100644
--- a/p2p/test/test_turn_customizer.h
+++ b/p2p/test/test_turn_customizer.h
@@ -37,6 +37,9 @@
 
     ASSERT_NE(0, message->type());
     if (add_counter_) {
+      // TODO: crbug.com/504567957 - redo authentication so that
+      // attribute will not be ignored, or move caller to do
+      // modification before authentication.
       message->AddAttribute(std::make_unique<StunUInt32Attribute>(
           STUN_ATTR_COUNTER, modify_cnt_));
     }

p2p/test/turn_server.cc

diff --git a/p2p/test/turn_server.cc b/p2p/test/turn_server.cc
index 5b59ebd..40060b1 100644
--- a/p2p/test/turn_server.cc
+++ b/p2p/test/turn_server.cc
@@ -488,11 +488,6 @@
                           EcnMarking ecn) {
   RTC_DCHECK_RUN_ON(thread_);
   ByteBufferWriter buf;
-  // Add a SOFTWARE attribute if one is set.
-  if (!software_.empty()) {
-    msg->AddAttribute(std::make_unique<StunByteStringAttribute>(
-        STUN_ATTR_SOFTWARE, software_));
-  }
   msg->Write(&buf);
   Send(conn, buf, ecn);
 }