diff --git a/p2p/base/stun_port.cc b/p2p/base/stun_port.cc index 373b21a9f8..44776d701b 100644 --- a/p2p/base/stun_port.cc +++ b/p2p/base/stun_port.cc @@ -46,7 +46,9 @@ class StunBindingRequest : public StunRequest { std::make_unique(STUN_BINDING_REQUEST)), port_(port), server_addr_(addr), - start_time_(start_time) {} + start_time_(start_time) { + SetAuthenticationRequired(false); + } const rtc::SocketAddress& server_addr() const { return server_addr_; } diff --git a/p2p/base/stun_request.cc b/p2p/base/stun_request.cc index 15c1e6a269..5bf7dfe69c 100644 --- a/p2p/base/stun_request.cc +++ b/p2p/base/stun_request.cc @@ -57,6 +57,10 @@ void StunRequestManager::Send(StunRequest* request) { void StunRequestManager::SendDelayed(StunRequest* request, int delay) { RTC_DCHECK_RUN_ON(thread_); RTC_DCHECK_EQ(this, request->manager()); + RTC_DCHECK(!request->AuthenticationRequired() || + request->msg()->integrity() != + StunMessage::IntegrityStatus::kNotSet) + << "Sending request w/o integrity!"; auto [iter, was_inserted] = requests_.emplace(request->id(), absl::WrapUnique(request)); RTC_DCHECK(was_inserted); @@ -104,15 +108,23 @@ bool StunRequestManager::CheckResponse(StunMessage* msg) { StunRequest* request = iter->second.get(); // Now that we know the request, we can see if the response is - // integrity-protected or not. - // For some tests, the message integrity is not set in the request. - // Complain, and then don't check. + // integrity-protected or not. Some requests explicitly disables + // integrity checks using SetAuthenticationRequired. + // TODO(chromium:1177125): Remove below! + // And we suspect that for some tests, the message integrity is not set in the + // request. Complain, and then don't check. bool skip_integrity_checking = (request->msg()->integrity() == StunMessage::IntegrityStatus::kNotSet); - if (skip_integrity_checking) { + if (!request->AuthenticationRequired()) { + // This is a STUN_BINDING to from stun_port.cc or + // the initial (unauthenticated) TURN_ALLOCATE_REQUEST. + } else if (skip_integrity_checking) { + // TODO(chromium:1177125): Remove below! // This indicates lazy test writing (not adding integrity attribute). // Complain, but only in debug mode (while developing). - RTC_DLOG(LS_ERROR) + RTC_LOG(LS_ERROR) + << "CheckResponse called on a passwordless request. Fix test!"; + RTC_DCHECK(false) << "CheckResponse called on a passwordless request. Fix test!"; } else { if (msg->integrity() == StunMessage::IntegrityStatus::kNotSet) { diff --git a/p2p/base/stun_request.h b/p2p/base/stun_request.h index 6e83be3830..f2fd7e5211 100644 --- a/p2p/base/stun_request.h +++ b/p2p/base/stun_request.h @@ -115,6 +115,12 @@ class StunRequest { // Time elapsed since last send (in ms) int Elapsed() const; + // Add method to explitly allow requests w/o password. + // - STUN_BINDINGs from StunPort to a stun server + // - The initial TURN_ALLOCATE_REQUEST + void SetAuthenticationRequired(bool val) { authentication_required_ = val; } + bool AuthenticationRequired() const { return authentication_required_; } + protected: friend class StunRequestManager; @@ -155,6 +161,7 @@ class StunRequest { bool timeout_ RTC_GUARDED_BY(network_thread()); webrtc::ScopedTaskSafety task_safety_{ webrtc::PendingTaskSafetyFlag::CreateDetachedInactive()}; + bool authentication_required_ = true; }; } // namespace cricket diff --git a/p2p/base/stun_request_unittest.cc b/p2p/base/stun_request_unittest.cc index 2f88ab1fd3..8ce89f8a00 100644 --- a/p2p/base/stun_request_unittest.cc +++ b/p2p/base/stun_request_unittest.cc @@ -79,7 +79,9 @@ class StunRequestThunker : public StunRequest { public: StunRequestThunker(StunRequestManager& manager, StunRequestTest* test) : StunRequest(manager, CreateStunMessage(STUN_BINDING_REQUEST)), - test_(test) {} + test_(test) { + SetAuthenticationRequired(false); + } std::unique_ptr CreateResponseMessage(StunMessageType type) { return CreateStunMessage(type, msg()); diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index 6cd6c451a3..042727ff67 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -1314,6 +1314,8 @@ TurnAllocateRequest::TurnAllocateRequest(TurnPort* port) message->AddAttribute(std::move(transport_attr)); if (!port_->hash().empty()) { port_->AddRequestAuthInfo(message); + } else { + SetAuthenticationRequired(false); } port_->MaybeAddTurnLoggingId(message); port_->TurnCustomizerMaybeModifyOutgoingStunMessage(message);