Discard unauthenticated TURN responses to unauthenticated requests

In StunRequestManager::CheckResponse, if a TURN request was sent
without authentication (initial ALLOCATE), we now explicitly discard
any success response. We also restrict allowed error responses to
401 Unauthorized and 300 Try Alternate.

This prevents an authentication bypass (b/504572664) where an attacker
could hijack a TURN session by forging a success response to the initial
unauthenticated request.

Add regression test for unauthenticated TURN ALLOCATE success.

Bug: chromium:504572664
Change-Id: Ie5619e10b6b6dccac3221661632e9f1d6312f760
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/466100
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#47529}

p2p/base/stun_request.cc

diff --git a/p2p/base/stun_request.cc b/p2p/base/stun_request.cc
index 3afabe1..1a18d2b 100644
--- a/p2p/base/stun_request.cc
+++ b/p2p/base/stun_request.cc
@@ -32,6 +32,7 @@
 #include "rtc_base/checks.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/string_encode.h"
+#include "rtc_base/string_utils.h"
 
 namespace webrtc {
 
@@ -122,8 +123,28 @@
   bool skip_integrity_checking =
       (request->msg()->integrity() == StunMessage::IntegrityStatus::kNotSet);
   if (!request->AuthenticationRequired()) {
-    // This is a STUN_BINDING to from stun_port.cc or
-    // the initial (unauthenticated) TURN_ALLOCATE_REQUEST.
+    if (request->type() != STUN_BINDING_REQUEST) {
+      if (msg->type() == GetStunSuccessResponseType(request->type())) {
+        RTC_LOG(LS_WARNING)
+            << "Discarding unauthenticated success response (0x"
+            << ToHex(msg->type()) << ") to TURN request of type 0x"
+            << ToHex(request->type())
+            << ", id=" << hex_encode(msg->transaction_id());
+        return false;
+      }
+      if (msg->type() == GetStunErrorResponseType(request->type())) {
+        int error_code = msg->GetErrorCodeValue();
+        if (error_code != STUN_ERROR_UNAUTHORIZED &&
+            error_code != STUN_ERROR_TRY_ALTERNATE) {
+          RTC_LOG(LS_WARNING)
+              << "Discarding unauthenticated error response with code "
+              << error_code << " to TURN request of type 0x"
+              << ToHex(request->type())
+              << ", id=" << hex_encode(msg->transaction_id());
+          return false;
+        }
+      }
+    }
   } else if (skip_integrity_checking) {
     // TODO(chromium:1177125): Remove below!
     // This indicates lazy test writing (not adding integrity attribute).

p2p/base/turn_port_unittest.cc

diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc
index 8a53aae..b23f33a 100644
--- a/p2p/base/turn_port_unittest.cc
+++ b/p2p/base/turn_port_unittest.cc
@@ -1948,6 +1948,65 @@
   unsigned int* attr_counter_ = nullptr;
 };
 
+// Test that an unauthenticated TURN ALLOCATE success response is NOT accepted
+// if integrity is expected but not present.
+// This is a regression test for b/504572664.
+TEST_F(TurnPortTest, TestUnauthenticatedAllocateSuccessRejected) {
+  SocketAddress fake_server_addr("99.99.99.99", 3478);
+  CreateTurnPort(kTurnUsername, kTurnPassword,
+                 ProtocolAddress(fake_server_addr, PROTO_UDP));
+
+  std::unique_ptr<AsyncPacketSocket> server_socket =
+      socket_factory()->CreateUdpSocket(env_, fake_server_addr, 0, 0);
+
+  std::string transaction_id;
+  server_socket->RegisterReceivedPacketCallback(
+      [&](AsyncPacketSocket* /* socket */, const ReceivedIpPacket& packet) {
+        ByteBufferReader reader(packet.payload());
+        TurnMessage msg;
+        if (msg.Read(&reader) && msg.type() == STUN_ALLOCATE_REQUEST) {
+          transaction_id = msg.transaction_id();
+        }
+      });
+
+  turn_port_->PrepareAddress();
+
+  // Wait for the request to reach the server.
+  ASSERT_TRUE(
+      WaitUntil([&] { return !transaction_id.empty(); },
+                {.timeout = kSimulatedRtt, .clock = &time_controller_}));
+
+  TurnMessage forged_success(STUN_ALLOCATE_RESPONSE, transaction_id);
+
+  // Add required attributes for ALLOCATE success.
+  SocketAddress relayed_addr("198.51.100.99", 6666);
+  SocketAddress mapped_addr("203.0.113.77", 5555);
+
+  forged_success.AddAttribute(std::make_unique<StunXorAddressAttribute>(
+      STUN_ATTR_XOR_RELAYED_ADDRESS, relayed_addr));
+  forged_success.AddAttribute(std::make_unique<StunXorAddressAttribute>(
+      STUN_ATTR_XOR_MAPPED_ADDRESS, mapped_addr));
+  forged_success.AddAttribute(
+      std::make_unique<StunUInt32Attribute>(STUN_ATTR_LIFETIME, 300));
+
+  ByteBufferWriter buf;
+  forged_success.Write(&buf);
+
+  // Send the forged response to the TurnPort.
+  SocketAddress local_addr = turn_port_->socket()->GetLocalAddress();
+  AsyncSocketPacketOptions local_options;
+  server_socket->SendTo(buf.Data(), buf.Length(), local_addr, local_options);
+
+  // Wait a bit for the packet to be processed.
+  time_controller_.AdvanceTime(kSimulatedRtt);
+
+  // If vulnerable, turn_ready_ would be true because it accepted the forged
+  // success. The correct behavior is to reject it and eventually fail or
+  // receive the 401.
+  EXPECT_FALSE(turn_ready_)
+      << "Vulnerability present: Unauthenticated ALLOCATE success accepted!";
+}
+
 // Do a TURN allocation, establish a TLS connection, and send some data.
 // Add customizer and check that it get called.
 TEST_F(TurnPortTest, TestTurnCustomizerCount) {