From 59574ca6d36f4769ecc734a5a52cdbb23f04d971 Mon Sep 17 00:00:00 2001 From: Tommi Date: Tue, 5 Sep 2023 09:21:57 +0200 Subject: [PATCH] Add absl::AnyInvocable to SSLStreamAdapter::Create Remove internal use of SignalSSLHandshakeError and prepare removal of sigslot dependency from SSLStreamAdapter. Bug: webrtc:11943 Change-Id: I9768e2e31529945620bdd8d0d285042bb2388b7b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/318881 Reviewed-by: Harald Alvestrand Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#40695} --- p2p/base/dtls_transport.cc | 6 +++--- rtc_base/BUILD.gn | 1 + rtc_base/openssl_stream_adapter.cc | 12 ++++++++++-- rtc_base/openssl_stream_adapter.h | 6 +++++- rtc_base/ssl_stream_adapter.cc | 6 ++++-- rtc_base/ssl_stream_adapter.h | 5 ++++- 6 files changed, 27 insertions(+), 9 deletions(-) diff --git a/p2p/base/dtls_transport.cc b/p2p/base/dtls_transport.cc index af16efad78..3a61fd4029 100644 --- a/p2p/base/dtls_transport.cc +++ b/p2p/base/dtls_transport.cc @@ -357,7 +357,9 @@ bool DtlsTransport::SetupDtls() { auto downward = std::make_unique(ice_transport_); StreamInterfaceChannel* downward_ptr = downward.get(); - dtls_ = rtc::SSLStreamAdapter::Create(std::move(downward)); + dtls_ = rtc::SSLStreamAdapter::Create( + std::move(downward), + [this](rtc::SSLHandshakeError error) { OnDtlsHandshakeError(error); }); if (!dtls_) { RTC_LOG(LS_ERROR) << ToString() << ": Failed to create DTLS adapter."; return false; @@ -370,8 +372,6 @@ bool DtlsTransport::SetupDtls() { dtls_->SetMaxProtocolVersion(ssl_max_version_); dtls_->SetServerRole(*dtls_role_); dtls_->SignalEvent.connect(this, &DtlsTransport::OnDtlsEvent); - dtls_->SignalSSLHandshakeError.connect(this, - &DtlsTransport::OnDtlsHandshakeError); if (remote_fingerprint_value_.size() && !dtls_->SetPeerCertificateDigest( remote_fingerprint_algorithm_, diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 30722a5f54..80ab5a6fe9 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -1548,6 +1548,7 @@ rtc_library("ssl") { absl_deps = [ "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/base:core_headers", + "//third_party/abseil-cpp/absl/functional:any_invocable", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc index bf400ff4ec..5064f0dfd4 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc @@ -294,8 +294,10 @@ bool ShouldAllowLegacyTLSProtocols() { } OpenSSLStreamAdapter::OpenSSLStreamAdapter( - std::unique_ptr stream) + std::unique_ptr stream, + absl::AnyInvocable handshake_error) : stream_(std::move(stream)), + handshake_error_(std::move(handshake_error)), owner_(rtc::Thread::Current()), state_(SSL_NONE), role_(SSL_CLIENT), @@ -938,7 +940,13 @@ int OpenSSLStreamAdapter::ContinueSSL() { } RTC_DLOG(LS_VERBOSE) << " -- error " << code << ", " << err_code << ", " << ERR_GET_REASON(err_code); - SignalSSLHandshakeError(ssl_handshake_err); + if (handshake_error_) { + handshake_error_(ssl_handshake_err); + } else { + // TODO(bugs.webrtc.org/11943): SignalSSLHandshakeError usage has been + // deprecated. Remove once external usage has been cleaned up. + SignalSSLHandshakeError(ssl_handshake_err); + } return (ssl_error != 0) ? ssl_error : -1; } diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h index aee8d36aad..779bc01516 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h @@ -19,6 +19,7 @@ #include #include +#include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "rtc_base/buffer.h" @@ -72,7 +73,9 @@ RTC_EXPORT void SetAllowLegacyTLSProtocols(const absl::optional& allow); class OpenSSLStreamAdapter final : public SSLStreamAdapter { public: - explicit OpenSSLStreamAdapter(std::unique_ptr stream); + OpenSSLStreamAdapter( + std::unique_ptr stream, + absl::AnyInvocable handshake_error); ~OpenSSLStreamAdapter() override; void SetIdentity(std::unique_ptr identity) override; @@ -202,6 +205,7 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter { } const std::unique_ptr stream_; + absl::AnyInvocable handshake_error_; rtc::Thread* const owner_; webrtc::ScopedTaskSafety task_safety_; diff --git a/rtc_base/ssl_stream_adapter.cc b/rtc_base/ssl_stream_adapter.cc index 4b60d6d7b1..931d0bf0b6 100644 --- a/rtc_base/ssl_stream_adapter.cc +++ b/rtc_base/ssl_stream_adapter.cc @@ -91,8 +91,10 @@ bool IsGcmCryptoSuiteName(absl::string_view crypto_suite) { } std::unique_ptr SSLStreamAdapter::Create( - std::unique_ptr stream) { - return std::make_unique(std::move(stream)); + std::unique_ptr stream, + absl::AnyInvocable handshake_error) { + return std::make_unique(std::move(stream), + std::move(handshake_error)); } bool SSLStreamAdapter::GetSslCipherSuite(int* cipher_suite) { diff --git a/rtc_base/ssl_stream_adapter.h b/rtc_base/ssl_stream_adapter.h index e68870c747..6494bcfa14 100644 --- a/rtc_base/ssl_stream_adapter.h +++ b/rtc_base/ssl_stream_adapter.h @@ -18,6 +18,7 @@ #include #include +#include "absl/functional/any_invocable.h" #include "absl/memory/memory.h" #include "absl/strings/string_view.h" #include "rtc_base/ssl_certificate.h" @@ -118,7 +119,8 @@ class SSLStreamAdapter : public StreamInterface, public sigslot::has_slots<> { // (using the selected implementation for the platform). // Caller is responsible for freeing the returned object. static std::unique_ptr Create( - std::unique_ptr stream); + std::unique_ptr stream, + absl::AnyInvocable handshake_error = nullptr); SSLStreamAdapter() = default; ~SSLStreamAdapter() override = default; @@ -261,6 +263,7 @@ class SSLStreamAdapter : public StreamInterface, public sigslot::has_slots<> { // authentication. bool GetClientAuthEnabled() const { return client_auth_enabled_; } + // TODO(bugs.webrtc.org/11943): Remove after updating downstream code. sigslot::signal1 SignalSSLHandshakeError; private: