diff --git a/rtc_base/openssl_adapter.cc b/rtc_base/openssl_adapter.cc index 563fe0f9d9..c381f04899 100644 --- a/rtc_base/openssl_adapter.cc +++ b/rtc_base/openssl_adapter.cc @@ -289,8 +289,8 @@ int OpenSSLAdapter::BeginSSL() { RTC_LOG(LS_INFO) << "OpenSSLAdapter::BeginSSL: " << ssl_host_name_; RTC_DCHECK(state_ == SSL_CONNECTING); - int err = 0; - BIO* bio = nullptr; + // Cleanup action to deal with on error cleanup a bit cleaner. + EarlyExitCatcher early_exit_catcher(*this); // First set up the context. We should either have a factory, with its own // pre-existing context, or be running standalone, in which case we will @@ -301,26 +301,22 @@ int OpenSSLAdapter::BeginSSL() { } if (!ssl_ctx_) { - err = -1; - goto ssl_error; + return -1; } if (identity_ && !identity_->ConfigureIdentity(ssl_ctx_)) { - SSL_CTX_free(ssl_ctx_); - err = -1; - goto ssl_error; + return -1; } - bio = BIO_new_socket(socket_); + std::unique_ptr bio{BIO_new_socket(socket_), + ::BIO_free}; if (!bio) { - err = -1; - goto ssl_error; + return -1; } ssl_ = SSL_new(ssl_ctx_); if (!ssl_) { - err = -1; - goto ssl_error; + return -1; } SSL_set_app_data(ssl_, this); @@ -346,8 +342,7 @@ int OpenSSLAdapter::BeginSSL() { if (cached) { if (SSL_set_session(ssl_, cached) == 0) { RTC_LOG(LS_WARNING) << "Failed to apply SSL session from cache"; - err = -1; - goto ssl_error; + return -1; } RTC_LOG(LS_INFO) << "Attempting to resume SSL session to " @@ -377,24 +372,16 @@ int OpenSSLAdapter::BeginSSL() { // Now that the initial config is done, transfer ownership of |bio| to the // SSL object. If ContinueSSL() fails, the bio will be freed in Cleanup(). - SSL_set_bio(ssl_, bio, bio); - bio = nullptr; + SSL_set_bio(ssl_, bio.get(), bio.get()); + bio.release(); // Do the connect. - err = ContinueSSL(); + int err = ContinueSSL(); if (err != 0) { - goto ssl_error; + return err; } - - return err; - -ssl_error: - Cleanup(); - if (bio) { - BIO_free(bio); - } - - return err; + early_exit_catcher.disable(); + return 0; } int OpenSSLAdapter::ContinueSSL() { @@ -1060,4 +1047,17 @@ OpenSSLAdapter* OpenSSLAdapterFactory::CreateAdapter(AsyncSocket* socket) { ssl_cert_verifier_); } +OpenSSLAdapter::EarlyExitCatcher::EarlyExitCatcher(OpenSSLAdapter& adapter_ptr) + : adapter_ptr_(adapter_ptr) {} + +void OpenSSLAdapter::EarlyExitCatcher::disable() { + disabled_ = true; +} + +OpenSSLAdapter::EarlyExitCatcher::~EarlyExitCatcher() { + if (!disabled_) { + adapter_ptr_.Cleanup(); + } +} + } // namespace rtc diff --git a/rtc_base/openssl_adapter.h b/rtc_base/openssl_adapter.h index 76b003a7dd..9b2a36e00f 100644 --- a/rtc_base/openssl_adapter.h +++ b/rtc_base/openssl_adapter.h @@ -89,6 +89,16 @@ class OpenSSLAdapter final : public SSLAdapter, void OnCloseEvent(AsyncSocket* socket, int err) override; private: + class EarlyExitCatcher { + public: + EarlyExitCatcher(OpenSSLAdapter& adapter_ptr); + void disable(); + ~EarlyExitCatcher(); + + private: + bool disabled_ = false; + OpenSSLAdapter& adapter_ptr_; + }; enum SSLState { SSL_NONE, SSL_WAIT, @@ -202,6 +212,10 @@ class OpenSSLAdapterFactory : public SSLAdapterFactory { friend class OpenSSLAdapter; }; +// The EarlyExitCatcher is responsible for calling OpenSSLAdapter::Cleanup on +// destruction. By doing this we have scoped cleanup which can be disabled if +// there were no errors, aka early exits. + std::string TransformAlpnProtocols(const std::vector& protos); } // namespace rtc