From 3c02842f2ef4876979f6603fcb789d28385f051a Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Thu, 22 Aug 2019 16:16:35 +0200 Subject: [PATCH] Add TURN_LOGGING_ID This patch adds a new (optional) attribute to TURN_ALLOCATE_REQUEST, TURN_LOGGING_ID (0xFF05). The attribute is put into the comprehension-optional range so that a TURN server should ignore it if it doesn't know if. https://tools.ietf.org/html/rfc5389#section-18.2 The intended usage of this attribute is to correlate client and backend logs. Bug: webrtc:10897 Change-Id: I51fdbe15f9025e817cd91ee8e2c3355133212daa Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149829 Reviewed-by: Qingsi Wang Reviewed-by: Steve Anton Reviewed-by: Niels Moller Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/master@{#28966} --- api/peer_connection_interface.h | 6 ++++ p2p/base/port_allocator.h | 1 + p2p/base/turn_port.cc | 16 +++++++++++ p2p/base/turn_port.h | 13 +++++++++ p2p/base/turn_port_unittest.cc | 49 +++++++++++++++++++++++++++++++++ p2p/client/turn_port_factory.cc | 2 ++ pc/peer_connection.cc | 15 +++++++++- 7 files changed, 101 insertions(+), 1 deletion(-) diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 4ade0b3e2a..2d5e314c9c 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -650,6 +650,12 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { // passed. bool offer_extmap_allow_mixed = false; + // TURN logging identifier. + // This identifier is added to a TURN allocation + // and it intended to be used to be able to match client side + // logs with TURN server logs. It will not be added if it's an empty string. + std::string turn_logging_id; + // // Don't forget to update operator== if adding something. // diff --git a/p2p/base/port_allocator.h b/p2p/base/port_allocator.h index c0b0e605dc..f29877c5b9 100644 --- a/p2p/base/port_allocator.h +++ b/p2p/base/port_allocator.h @@ -182,6 +182,7 @@ struct RTC_EXPORT RelayServerConfig { std::vector tls_alpn_protocols; std::vector tls_elliptic_curves; rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr; + std::string turn_logging_id; }; class RTC_EXPORT PortAllocatorSession : public sigslot::has_slots<> { diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index f9104978b3..7845b6a362 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -33,7 +33,11 @@ namespace cricket { // TODO(juberti): Move to stun.h when relay messages have been renamed. static const int TURN_ALLOCATE_REQUEST = STUN_ALLOCATE_REQUEST; +// Attributes in comprehension-optional range, +// ignored by TURN server that doesn't know about them. +// https://tools.ietf.org/html/rfc5389#section-18.2 static const int STUN_ATTR_MULTI_MAPPING = 0xff04; +const int STUN_ATTR_TURN_LOGGING_ID = 0xff05; // TODO(juberti): Extract to turnmessage.h static const int TURN_DEFAULT_PORT = 3478; @@ -314,6 +318,10 @@ void TurnPort::SetTlsCertPolicy(TlsCertPolicy tls_cert_policy) { tls_cert_policy_ = tls_cert_policy; } +void TurnPort::SetTurnLoggingId(const std::string& turn_logging_id) { + turn_logging_id_ = turn_logging_id; +} + std::vector TurnPort::GetTlsAlpnProtocols() const { return tls_alpn_protocols_; } @@ -1313,6 +1321,13 @@ bool TurnPort::TurnCustomizerAllowChannelData(const void* data, return turn_customizer_->AllowChannelData(this, data, size, payload); } +void TurnPort::MaybeAddTurnLoggingId(StunMessage* msg) { + if (!turn_logging_id_.empty()) { + msg->AddAttribute(absl::make_unique( + STUN_ATTR_TURN_LOGGING_ID, turn_logging_id_)); + } +} + TurnAllocateRequest::TurnAllocateRequest(TurnPort* port) : StunRequest(new TurnMessage()), port_(port) {} @@ -1326,6 +1341,7 @@ void TurnAllocateRequest::Prepare(StunMessage* request) { if (!port_->hash().empty()) { port_->AddRequestAuthInfo(request); } + port_->MaybeAddTurnLoggingId(request); port_->TurnCustomizerMaybeModifyOutgoingStunMessage(request); } diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index e6dab6e3fc..8247dbc777 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -33,6 +33,7 @@ class TurnCustomizer; namespace cricket { +extern const int STUN_ATTR_TURN_LOGGING_ID; extern const char TURN_PORT_TYPE[]; class TurnAllocateRequest; class TurnEntry; @@ -148,6 +149,8 @@ class TurnPort : public Port { virtual TlsCertPolicy GetTlsCertPolicy() const; virtual void SetTlsCertPolicy(TlsCertPolicy tls_cert_policy); + void SetTurnLoggingId(const std::string& turn_logging_id); + virtual std::vector GetTlsAlpnProtocols() const; virtual std::vector GetTlsEllipticCurves() const; @@ -347,6 +350,8 @@ class TurnPort : public Port { // Reconstruct the URL of the server which the candidate is gathered from. std::string ReconstructedServerUrl(bool use_hostname); + void MaybeAddTurnLoggingId(StunMessage* message); + void TurnCustomizerMaybeModifyOutgoingStunMessage(StunMessage* message); bool TurnCustomizerAllowChannelData(const void* data, size_t size, @@ -388,6 +393,14 @@ class TurnPort : public Port { // must outlive the TurnPort's lifetime. webrtc::TurnCustomizer* turn_customizer_ = nullptr; + // Optional TurnLoggingId. + // An identifier set by application that is added to TURN_ALLOCATE_REQUEST + // and can be used to match client/backend logs. + // TODO(jonaso): This should really be initialized in constructor, + // but that is currently so terrible. Fix once constructor is changed + // to be more easy to work with. + std::string turn_logging_id_; + friend class TurnEntry; friend class TurnAllocateRequest; friend class TurnRefreshRequest; diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index b51a1266b1..73dadb6718 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -16,6 +16,7 @@ #include #include +#include "absl/memory/memory.h" #include "absl/types/optional.h" #include "api/units/time_delta.h" #include "p2p/base/basic_packet_socket_factory.h" @@ -810,6 +811,54 @@ TEST_F(TurnPortTest, TestTurnAllocate) { EXPECT_NE(0, turn_port_->Candidates()[0].address().port()); } +class TurnLoggingIdValidator : public StunMessageObserver { + public: + explicit TurnLoggingIdValidator(const char* expect_val) + : expect_val_(expect_val) {} + ~TurnLoggingIdValidator() {} + void ReceivedMessage(const TurnMessage* msg) override { + if (msg->type() == cricket::STUN_ALLOCATE_REQUEST) { + const StunByteStringAttribute* attr = + msg->GetByteString(cricket::STUN_ATTR_TURN_LOGGING_ID); + if (expect_val_) { + ASSERT_NE(nullptr, attr); + ASSERT_EQ(expect_val_, attr->GetString()); + } else { + EXPECT_EQ(nullptr, attr); + } + } + } + void ReceivedChannelData(const char* data, size_t size) override {} + + private: + const char* expect_val_; +}; + +TEST_F(TurnPortTest, TestTurnAllocateWithLoggingId) { + CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr); + turn_port_->SetTurnLoggingId("KESO"); + turn_server_.server()->SetStunMessageObserver( + absl::make_unique("KESO")); + turn_port_->PrepareAddress(); + EXPECT_TRUE_SIMULATED_WAIT(turn_ready_, kSimulatedRtt * 2, fake_clock_); + ASSERT_EQ(1U, turn_port_->Candidates().size()); + EXPECT_EQ(kTurnUdpExtAddr.ipaddr(), + turn_port_->Candidates()[0].address().ipaddr()); + EXPECT_NE(0, turn_port_->Candidates()[0].address().port()); +} + +TEST_F(TurnPortTest, TestTurnAllocateWithoutLoggingId) { + CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr); + turn_server_.server()->SetStunMessageObserver( + absl::make_unique(nullptr)); + turn_port_->PrepareAddress(); + EXPECT_TRUE_SIMULATED_WAIT(turn_ready_, kSimulatedRtt * 2, fake_clock_); + ASSERT_EQ(1U, turn_port_->Candidates().size()); + EXPECT_EQ(kTurnUdpExtAddr.ipaddr(), + turn_port_->Candidates()[0].address().ipaddr()); + EXPECT_NE(0, turn_port_->Candidates()[0].address().port()); +} + // Test bad credentials. TEST_F(TurnPortTest, TestTurnBadCredentials) { CreateTurnPort(kTurnUsername, "bad", kTurnUdpProtoAddr); diff --git a/p2p/client/turn_port_factory.cc b/p2p/client/turn_port_factory.cc index 934c019bb7..de4b9e6a09 100644 --- a/p2p/client/turn_port_factory.cc +++ b/p2p/client/turn_port_factory.cc @@ -29,6 +29,7 @@ std::unique_ptr TurnPortFactory::Create( args.config->credentials, args.config->priority, args.origin, args.turn_customizer); port->SetTlsCertPolicy(args.config->tls_cert_policy); + port->SetTurnLoggingId(args.config->turn_logging_id); return std::move(port); } @@ -42,6 +43,7 @@ std::unique_ptr TurnPortFactory::Create(const CreateRelayPortArgs& args, args.config->tls_alpn_protocols, args.config->tls_elliptic_curves, args.turn_customizer, args.config->tls_cert_verifier); port->SetTlsCertPolicy(args.config->tls_cert_policy); + port->SetTurnLoggingId(args.config->turn_logging_id); return std::move(port); } diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 0f7970c536..927155928a 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -809,6 +809,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( absl::optional use_datagram_transport_for_data_channels; absl::optional crypto_options; bool offer_extmap_allow_mixed; + std::string turn_logging_id; }; static_assert(sizeof(stuff_being_tested_for_equality) == sizeof(*this), "Did you add something to RTCConfiguration and forget to " @@ -871,7 +872,8 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( use_datagram_transport_for_data_channels == o.use_datagram_transport_for_data_channels && crypto_options == o.crypto_options && - offer_extmap_allow_mixed == o.offer_extmap_allow_mixed; + offer_extmap_allow_mixed == o.offer_extmap_allow_mixed && + turn_logging_id == o.turn_logging_id; } bool PeerConnectionInterface::RTCConfiguration::operator!=( @@ -1023,6 +1025,11 @@ bool PeerConnection::Initialize( return false; } + // Add the turn logging id to all turn servers + for (cricket::RelayServerConfig& turn_server : turn_servers) { + turn_server.turn_logging_id = configuration.turn_logging_id; + } + // The port allocator lives on the network thread and should be initialized // there. const auto pa_result = @@ -3625,6 +3632,7 @@ RTCError PeerConnection::SetConfiguration( modified_config.use_datagram_transport = configuration.use_datagram_transport; modified_config.use_datagram_transport_for_data_channels = configuration.use_datagram_transport_for_data_channels; + modified_config.turn_logging_id = configuration.turn_logging_id; if (configuration != modified_config) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, "Modifying the configuration in an unsupported way."); @@ -3651,6 +3659,11 @@ RTCError PeerConnection::SetConfiguration( if (parse_error != RTCErrorType::NONE) { return RTCError(parse_error); } + // Add the turn logging id to all turn servers + for (cricket::RelayServerConfig& turn_server : turn_servers) { + turn_server.turn_logging_id = configuration.turn_logging_id; + } + // Note if STUN or TURN servers were supplied. if (!stun_servers.empty()) { NoteUsageEvent(UsageEvent::STUN_SERVER_ADDED);