From 573ca2beb74eef2646a3e2173331ff6bc3a9a990 Mon Sep 17 00:00:00 2001 From: Tommi Date: Tue, 29 Aug 2023 11:05:16 +0200 Subject: [PATCH] Add thread guards for username/pwd Also change return type for username_fragment() to be const& and not create a copy. Bug: none Change-Id: I8591af3da54fc8a9784e13cb000c4e02c0cd2f40 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317980 Commit-Queue: Tomas Gunnarsson Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#40651} --- p2p/base/port.cc | 9 ++++++++- p2p/base/port.h | 10 +++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/p2p/base/port.cc b/p2p/base/port.cc index 3a1032a396..ca0a6c5b03 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc @@ -169,6 +169,7 @@ Port::Port(TaskQueueBase* thread, } void Port::Construct() { + RTC_DCHECK_RUN_ON(thread_); // TODO(pthatcher): Remove this old behavior once we're sure no one // relies on it. If the username_fragment and password are empty, // we should just create one. @@ -430,6 +431,7 @@ bool Port::GetStunMessage(const char* data, const rtc::SocketAddress& addr, std::unique_ptr* out_msg, std::string* out_username) { + RTC_DCHECK_RUN_ON(thread_); // NOTE: This could clearly be optimized to avoid allocating any memory. // However, at the data rates we'll be looking at on the client side, // this probably isn't worth worrying about. @@ -658,6 +660,7 @@ bool Port::ParseStunUsername(const StunMessage* stun_msg, bool Port::MaybeIceRoleConflict(const rtc::SocketAddress& addr, IceMessage* stun_msg, absl::string_view remote_ufrag) { + RTC_DCHECK_RUN_ON(thread_); // Validate ICE_CONTROLLING or ICE_CONTROLLED attributes. bool ret = true; IceRole remote_ice_role = ICEROLE_UNKNOWN; @@ -717,6 +720,7 @@ bool Port::MaybeIceRoleConflict(const rtc::SocketAddress& addr, } std::string Port::CreateStunUsername(absl::string_view remote_username) const { + RTC_DCHECK_RUN_ON(thread_); return std::string(remote_username) + ":" + username_fragment(); } @@ -737,6 +741,7 @@ void Port::SendBindingErrorResponse(StunMessage* message, const rtc::SocketAddress& addr, int error_code, absl::string_view reason) { + RTC_DCHECK_RUN_ON(thread_); RTC_DCHECK(message->type() == STUN_BINDING_REQUEST || message->type() == GOOG_PING_REQUEST); @@ -786,6 +791,7 @@ void Port::SendUnknownAttributesErrorResponse( StunMessage* message, const rtc::SocketAddress& addr, const std::vector& unknown_types) { + RTC_DCHECK_RUN_ON(thread_); RTC_DCHECK(message->type() == STUN_BINDING_REQUEST); // Fill in the response message. @@ -959,7 +965,8 @@ void Port::Destroy() { delete this; } -const std::string Port::username_fragment() const { +const std::string& Port::username_fragment() const { + RTC_DCHECK_RUN_ON(thread_); return ice_username_fragment_; } diff --git a/p2p/base/port.h b/p2p/base/port.h index 31091eb273..2678df8e52 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -255,7 +255,7 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { uint32_t generation() const { return generation_; } void set_generation(uint32_t generation) { generation_ = generation; } - const std::string username_fragment() const; + const std::string& username_fragment() const; const std::string& password() const { return password_; } // May be called when this port was initially created by a pooled @@ -496,12 +496,8 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { // sent through), the other side must send us a STUN binding request that is // authenticated with this username_fragment and password. // PortAllocatorSession will provide these username_fragment and password. - // - // Note: we should always use username_fragment() instead of using - // `ice_username_fragment_` directly. For the details see the comment on - // username_fragment(). - std::string ice_username_fragment_; - std::string password_; + std::string ice_username_fragment_ RTC_GUARDED_BY(thread_); + std::string password_ RTC_GUARDED_BY(thread_); std::vector candidates_ RTC_GUARDED_BY(thread_); AddressMap connections_; int timeout_delay_;