From 12574a315f8c1fe66d79f8b28b6e4679c103523b Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Thu, 19 Dec 2024 15:15:50 +0100 Subject: [PATCH] DTLS 1.3 - patch 4 This patchs adds a field trial for enabling DTLS1.3, WebRTC-ForceDtls13 - "Enabled" set max version to DTLS1.3 - "Only" set min & max version to DTLS1.3 Wireup a FieldTrialsView so that this does not use the global string. Also convert the WebRTC-DisableTlsSessionTicketKillswitch from global string to FieldTrialsView. BUG=webrtc:383141571 Change-Id: Ia775efc1dcbffd01bfddb6030490438cb8de89d7 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372261 Reviewed-by: Harald Alvestrand Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/main@{#43616} --- experiments/field_trials.py | 3 +++ p2p/base/ice_transport_internal.h | 4 +++ p2p/base/p2p_transport_channel.cc | 3 ++- p2p/base/p2p_transport_channel.h | 6 +++++ p2p/dtls/dtls_transport.cc | 3 ++- rtc_base/BUILD.gn | 2 +- rtc_base/openssl_adapter.cc | 1 - rtc_base/openssl_stream_adapter.cc | 43 +++++++++++++++++++++++++++--- rtc_base/openssl_stream_adapter.h | 9 ++++++- rtc_base/ssl_stream_adapter.cc | 7 ++--- rtc_base/ssl_stream_adapter.h | 4 ++- 11 files changed, 72 insertions(+), 13 deletions(-) diff --git a/experiments/field_trials.py b/experiments/field_trials.py index 3eea5853ec..e8df3cb3ea 100755 --- a/experiments/field_trials.py +++ b/experiments/field_trials.py @@ -83,6 +83,9 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([ FieldTrial('WebRTC-EncoderDataDumpDirectory', 296242528, date(2024, 4, 1)), + FieldTrial('WebRTC-ForceDtls13', + 383141571, + date(2024,9,1)), FieldTrial('WebRTC-FrameCadenceAdapter-UseVideoFrameTimestamp', 42226256, date(2024, 10, 1)), diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h index a0cdbbc78f..1d7bd89a43 100644 --- a/p2p/base/ice_transport_internal.h +++ b/p2p/base/ice_transport_internal.h @@ -395,6 +395,10 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal { dictionary_writer_synced_callback_list_.RemoveReceivers(tag); } + virtual const webrtc::FieldTrialsView* field_trials() const { + return nullptr; + } + protected: void SendGatheringStateEvent() { gathering_state_callback_list_.Send(this); diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 78532332c0..1e5b0fef3a 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -199,7 +199,8 @@ P2PTransportChannel::P2PTransportChannel( STRONG_AND_STABLE_WRITABLE_CONNECTION_PING_INTERVAL, true /* presume_writable_when_fully_relayed */, REGATHER_ON_FAILED_NETWORKS_INTERVAL, - RECEIVING_SWITCHING_DELAY) { + RECEIVING_SWITCHING_DELAY), + field_trials_(field_trials) { TRACE_EVENT0("webrtc", "P2PTransportChannel::P2PTransportChannel"); RTC_DCHECK(allocator_ != nullptr); // Validate IceConfig even for mostly built-in constant default values in case diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 71a9041d7b..0bc3b84616 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -248,6 +248,10 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal, return stun_dict_writer_; } + const webrtc::FieldTrialsView* field_trials() const override { + return field_trials_; + } + private: P2PTransportChannel( absl::string_view transport_name, @@ -503,6 +507,8 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal, // Parsed field trials. IceFieldTrials ice_field_trials_; + // Unparsed field trials. + const webrtc::FieldTrialsView* field_trials_; // A dictionary of attributes that will be reflected to peer. StunDictionaryWriter stun_dict_writer_; diff --git a/p2p/dtls/dtls_transport.cc b/p2p/dtls/dtls_transport.cc index b9e9d9abc1..26567fcb28 100644 --- a/p2p/dtls/dtls_transport.cc +++ b/p2p/dtls/dtls_transport.cc @@ -363,7 +363,8 @@ bool DtlsTransport::SetupDtls() { dtls_ = rtc::SSLStreamAdapter::Create( std::move(downward), - [this](rtc::SSLHandshakeError error) { OnDtlsHandshakeError(error); }); + [this](rtc::SSLHandshakeError error) { OnDtlsHandshakeError(error); }, + ice_transport_->field_trials()); if (!dtls_) { RTC_LOG(LS_ERROR) << ToString() << ": Failed to create DTLS adapter."; return false; diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index c060dfb92e..0cd28c73e9 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -1606,10 +1606,10 @@ rtc_library("ssl_adapter") { ":threading", ":timeutils", "../api:array_view", + "../api:field_trials_view", "../api:sequence_checker", "../api/task_queue:pending_task_safety_flag", "../api/units:time_delta", - "../system_wrappers:field_trial", "system:rtc_export", "task_utils:repeating_task", "//third_party/abseil-cpp/absl/functional:any_invocable", diff --git a/rtc_base/openssl_adapter.cc b/rtc_base/openssl_adapter.cc index ffafab2c1f..006222d790 100644 --- a/rtc_base/openssl_adapter.cc +++ b/rtc_base/openssl_adapter.cc @@ -63,7 +63,6 @@ #include "rtc_base/openssl_utility.h" #include "rtc_base/strings/str_join.h" #include "rtc_base/thread.h" -#include "system_wrappers/include/field_trial.h" ////////////////////////////////////////////////////////////////////// // SocketBIO diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc index cd08c54568..6951899bba 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc @@ -58,7 +58,6 @@ #include "rtc_base/string_encode.h" #include "rtc_base/thread.h" #include "rtc_base/time_utils.h" -#include "system_wrappers/include/field_trial.h" #if (OPENSSL_VERSION_NUMBER < 0x10100000L) #error "webrtc requires at least OpenSSL version 1.1.0, to support DTLS-SRTP" @@ -140,6 +139,28 @@ uint16_t GetMaxVersion(SSLMode ssl_mode, SSLProtocolVersion version) { } } +constexpr int kForceDtls13Off = 0; +#ifdef DTLS1_3_VERSION +constexpr int kForceDtls13Enabled = 1; +constexpr int kForceDtls13Only = 2; +#endif + +int GetForceDtls13(const webrtc::FieldTrialsView* field_trials) { + if (field_trials == nullptr) { + return kForceDtls13Off; + } +#ifdef DTLS1_3_VERSION + auto mode = field_trials->Lookup("WebRTC-ForceDtls13"); + RTC_LOG(LS_WARNING) << "WebRTC-ForceDtls13: " << mode; + if (mode == "Enabled") { + return kForceDtls13Enabled; + } else if (mode == "Only") { + return kForceDtls13Only; + } +#endif + return kForceDtls13Off; +} + } // namespace ////////////////////////////////////////////////////////////////////// @@ -264,7 +285,8 @@ static long stream_ctrl(BIO* b, int cmd, long num, void* ptr) { OpenSSLStreamAdapter::OpenSSLStreamAdapter( std::unique_ptr stream, - absl::AnyInvocable handshake_error) + absl::AnyInvocable handshake_error, + const webrtc::FieldTrialsView* field_trials) : stream_(std::move(stream)), handshake_error_(std::move(handshake_error)), owner_(rtc::Thread::Current()), @@ -276,8 +298,12 @@ OpenSSLStreamAdapter::OpenSSLStreamAdapter( ssl_ctx_(nullptr), ssl_mode_(SSL_MODE_DTLS), ssl_max_version_(SSL_PROTOCOL_DTLS_12), - disable_handshake_ticket_(!webrtc::field_trial::IsDisabled( - "WebRTC-DisableTlsSessionTicketKillswitch")) { + disable_handshake_ticket_( + (field_trials == nullptr) + ? true + : !field_trials->IsDisabled( + "WebRTC-DisableTlsSessionTicketKillswitch")), + force_dtls_13_(GetForceDtls13(field_trials)) { stream_->SetEventCallback( [this](int events, int err) { OnEvent(events, err); }); } @@ -988,6 +1014,15 @@ SSL_CTX* OpenSSLStreamAdapter::SetupSSLContext() { auto min_version = ssl_mode_ == SSL_MODE_DTLS ? DTLS1_2_VERSION : TLS1_2_VERSION; auto max_version = GetMaxVersion(ssl_mode_, ssl_max_version_); +#ifdef DTLS1_3_VERSION + if (force_dtls_13_ == kForceDtls13Enabled) { + max_version = DTLS1_3_VERSION; + } else if (force_dtls_13_ == kForceDtls13Only) { + min_version = DTLS1_3_VERSION; + max_version = DTLS1_3_VERSION; + } +#endif + SSL_CTX_set_min_proto_version(ctx, min_version); SSL_CTX_set_max_proto_version(ctx, max_version); diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h index 8b8e966816..92307ab288 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h @@ -31,6 +31,7 @@ #else #include "rtc_base/openssl_identity.h" #endif +#include "api/field_trials_view.h" #include "api/task_queue/pending_task_safety_flag.h" #include "rtc_base/ssl_identity.h" #include "rtc_base/ssl_stream_adapter.h" @@ -70,7 +71,8 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter { public: OpenSSLStreamAdapter( std::unique_ptr stream, - absl::AnyInvocable handshake_error); + absl::AnyInvocable handshake_error, + const webrtc::FieldTrialsView* field_trials = nullptr); ~OpenSSLStreamAdapter() override; void SetIdentity(std::unique_ptr identity) override; @@ -244,6 +246,11 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter { // Rollout killswitch for disabling session tickets. const bool disable_handshake_ticket_; + + // 0 == Disabled + // 1 == Max + // 2 == Enabled (both min and max) + const int force_dtls_13_ = 0; }; ///////////////////////////////////////////////////////////////////////////// diff --git a/rtc_base/ssl_stream_adapter.cc b/rtc_base/ssl_stream_adapter.cc index cb58ff7a1d..082807d68d 100644 --- a/rtc_base/ssl_stream_adapter.cc +++ b/rtc_base/ssl_stream_adapter.cc @@ -82,9 +82,10 @@ bool IsGcmCryptoSuite(int crypto_suite) { std::unique_ptr SSLStreamAdapter::Create( std::unique_ptr stream, - absl::AnyInvocable handshake_error) { - return std::make_unique(std::move(stream), - std::move(handshake_error)); + absl::AnyInvocable handshake_error, + const webrtc::FieldTrialsView* field_trials) { + return std::make_unique( + std::move(stream), std::move(handshake_error), field_trials); } bool SSLStreamAdapter::IsBoringSsl() { diff --git a/rtc_base/ssl_stream_adapter.h b/rtc_base/ssl_stream_adapter.h index cc3adcabe8..fe115d6eb7 100644 --- a/rtc_base/ssl_stream_adapter.h +++ b/rtc_base/ssl_stream_adapter.h @@ -22,6 +22,7 @@ #include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" #include "api/array_view.h" +#include "api/field_trials_view.h" #include "rtc_base/buffer.h" #include "rtc_base/ssl_certificate.h" #include "rtc_base/ssl_identity.h" @@ -123,7 +124,8 @@ class SSLStreamAdapter : public StreamInterface { // Caller is responsible for freeing the returned object. static std::unique_ptr Create( std::unique_ptr stream, - absl::AnyInvocable handshake_error = nullptr); + absl::AnyInvocable handshake_error = nullptr, + const webrtc::FieldTrialsView* field_trials = nullptr); SSLStreamAdapter() = default; ~SSLStreamAdapter() override = default;