diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index 71deb693c1..201903587d 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -1297,8 +1297,8 @@ void TurnPort::SetCallbacksForTest(CallbacksForTest* callbacks) { callbacks_for_test_ = callbacks; } -bool TurnPort::SetEntryChannelId(const rtc::SocketAddress& address, - int channel_id) { +bool TurnPort::SetEntryChannelIdForTesting(const rtc::SocketAddress& address, + int channel_id) { TurnEntry* entry = FindEntry(address); if (!entry) { return false; diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index f8783d5daf..e8b0f8671d 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -192,7 +192,11 @@ class TurnPort : public Port { } // Finds the turn entry with `address` and sets its channel id. // Returns true if the entry is found. - bool SetEntryChannelId(const rtc::SocketAddress& address, int channel_id); + // This method must not be used in production, it is a test only + // utility that doesn't check the channel id is valid according to + // RFC5766. + bool SetEntryChannelIdForTesting(const rtc::SocketAddress& address, + int channel_id); void HandleConnectionDestroyed(Connection* conn) override; diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index 8f81ad37ca..0726128fce 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -1518,11 +1518,12 @@ TEST_F(TurnPortTest, TestChannelBindGetErrorResponse) { ASSERT_TRUE(conn2 != nullptr); conn1->Ping(0); EXPECT_TRUE_SIMULATED_WAIT(conn1->writable(), kSimulatedRtt * 2, fake_clock_); - // TODO(deadbeef): SetEntryChannelId should not be a public method. - // Instead we should set an option on the fake TURN server to force it to - // send a channel bind errors. - ASSERT_TRUE(turn_port_->SetEntryChannelId( - udp_port_->Candidates()[0].address(), /*channel_id=*/1)); + // TODO(bugs.webrtc.org/345518625): SetEntryChannelIdForTesting should not be + // a public method. Instead we should set an option on the fake TURN server to + // force it to send a channel bind errors. + int illegal_channel_id = kMaxTurnChannelNumber + 1u; + ASSERT_TRUE(turn_port_->SetEntryChannelIdForTesting( + udp_port_->Candidates()[0].address(), illegal_channel_id)); std::string data = "ABC"; conn1->Send(data.data(), data.length(), options); @@ -1534,6 +1535,8 @@ TEST_F(TurnPortTest, TestChannelBindGetErrorResponse) { conn2->RegisterReceivedPacketCallback( [&](Connection* connection, const rtc::ReceivedPacket& packet) { + // TODO(bugs.webrtc.org/345518625): Verify that the packet was + // received unchanneled, not channeled. udp_packets_.push_back( rtc::Buffer(packet.payload().data(), packet.payload().size())); }); diff --git a/p2p/base/turn_server.cc b/p2p/base/turn_server.cc index 98b9f3ef6d..4fd899ee2d 100644 --- a/p2p/base/turn_server.cc +++ b/p2p/base/turn_server.cc @@ -42,9 +42,6 @@ constexpr TimeDelta kDefaultAllocationTimeout = TimeDelta::Minutes(10); constexpr TimeDelta kPermissionTimeout = TimeDelta::Minutes(5); constexpr TimeDelta kChannelTimeout = TimeDelta::Minutes(10); -constexpr int kMinChannelNumber = 0x4000; -constexpr int kMaxChannelNumber = 0x7FFF; - constexpr size_t kNonceKeySize = 16; constexpr size_t kNonceSize = 48; @@ -721,7 +718,8 @@ void TurnServerAllocation::HandleChannelBindRequest(const TurnMessage* msg) { // Check that channel id is valid. int channel_id = channel_attr->value() >> 16; - if (channel_id < kMinChannelNumber || channel_id > kMaxChannelNumber) { + if (channel_id < kMinTurnChannelNumber || + channel_id > kMaxTurnChannelNumber) { SendBadRequestResponse(msg); return; } diff --git a/p2p/base/turn_server.h b/p2p/base/turn_server.h index be42338a3b..94188d6df5 100644 --- a/p2p/base/turn_server.h +++ b/p2p/base/turn_server.h @@ -38,6 +38,9 @@ class PacketSocketFactory; namespace cricket { +constexpr int kMinTurnChannelNumber = 0x4000; +constexpr int kMaxTurnChannelNumber = 0x7FFF; + class StunMessage; class TurnMessage; class TurnServer;