openssl: Use unique_ptr to remove unnecessary goto statements from openssl_adapter

Bug: None
Change-Id: I0fc09d8825a3d138f2b134f50634217b80677fff
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/224164
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34395}

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, decltype(&::BIO_free)> 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

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<std::string>& protos);
 
 }  // namespace rtc