commit b32650e219caae46c651dc64d9eb2607e2fa9738
author Harald Alvestrand <hta@webrtc.org> Fri Sep 10 08:03:22 2021
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Sep 10 13:40:31 2021
tree 74360198238f7c8d09aecd13372383179bca1e5d
parent 5eb5bb50070176caed9e90fb3ba6ffa200a97520

Improve SSLVerifyCallback variable naming

The names used were confusing.

Bug: chromium:1247577
Change-Id: I007f8b9b6b9c2188cbfc2dcfb2499acf3c14a9ce
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231683
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34967}

rtc_base/openssl_adapter.cc

diff --git a/rtc_base/openssl_adapter.cc b/rtc_base/openssl_adapter.cc
index d80b69d..93e5780 100644
--- a/rtc_base/openssl_adapter.cc
+++ b/rtc_base/openssl_adapter.cc
@@ -848,27 +848,31 @@
   return ssl_verify_ok;
 }
 #else  // WEBRTC_USE_CRYPTO_BUFFER_CALLBACK
-int OpenSSLAdapter::SSLVerifyCallback(int ok, X509_STORE_CTX* store) {
+int OpenSSLAdapter::SSLVerifyCallback(int status, X509_STORE_CTX* store) {
   // Get our stream pointer from the store
   SSL* ssl = reinterpret_cast<SSL*>(
       X509_STORE_CTX_get_ex_data(store, SSL_get_ex_data_X509_STORE_CTX_idx()));
 
   OpenSSLAdapter* stream =
       reinterpret_cast<OpenSSLAdapter*>(SSL_get_app_data(ssl));
-  ok = stream->SSLVerifyInternal(ok, ssl, store);
+  // Update status with the custom verifier.
+  // Status is unchanged if verification fails.
+  status = stream->SSLVerifyInternal(status, ssl, store);
 
   // Should only be used for debugging and development.
-  if (!ok && stream->ignore_bad_cert_) {
+  if (!status && stream->ignore_bad_cert_) {
     RTC_DLOG(LS_WARNING) << "Ignoring cert error while verifying cert chain";
     return 1;
   }
 
-  return ok;
+  return status;
 }
 
-int OpenSSLAdapter::SSLVerifyInternal(int ok, SSL* ssl, X509_STORE_CTX* store) {
+int OpenSSLAdapter::SSLVerifyInternal(int status_on_failure,
+                                      SSL* ssl,
+                                      X509_STORE_CTX* store) {
 #if !defined(NDEBUG)
-  if (!ok) {
+  if (!status_on_failure) {
     char data[256];
     X509* cert = X509_STORE_CTX_get_current_cert(store);
     int depth = X509_STORE_CTX_get_error_depth(store);
@@ -884,7 +888,7 @@
   }
 #endif
   if (ssl_cert_verifier_ == nullptr) {
-    return ok;
+    return status_on_failure;
   }
 
   RTC_LOG(LS_INFO) << "Invoking SSL Verify Callback.";
@@ -894,14 +898,14 @@
   int length = i2d_X509(X509_STORE_CTX_get_current_cert(store), &data);
   if (length < 0) {
     RTC_LOG(LS_ERROR) << "Failed to encode X509.";
-    return ok;
+    return status_on_failure;
   }
   bssl::UniquePtr<uint8_t> owned_data(data);
   bssl::UniquePtr<CRYPTO_BUFFER> crypto_buffer(
       CRYPTO_BUFFER_new(data, length, openssl::GetBufferPool()));
   if (!crypto_buffer) {
     RTC_LOG(LS_ERROR) << "Failed to allocate CRYPTO_BUFFER.";
-    return ok;
+    return status_on_failure;
   }
   const BoringSSLCertificate cert(std::move(crypto_buffer));
 #else
@@ -909,7 +913,7 @@
 #endif
   if (!ssl_cert_verifier_->Verify(cert)) {
     RTC_LOG(LS_INFO) << "Failed to verify certificate using custom callback";
-    return ok;
+    return status_on_failure;
   }
 
   custom_cert_verifier_status_ = true;

rtc_base/openssl_adapter.h

diff --git a/rtc_base/openssl_adapter.h b/rtc_base/openssl_adapter.h
index 4649de0..266ed35 100644
--- a/rtc_base/openssl_adapter.h
+++ b/rtc_base/openssl_adapter.h
@@ -131,7 +131,9 @@
   enum ssl_verify_result_t SSLVerifyInternal(SSL* ssl, uint8_t* out_alert);
 #else
   static int SSLVerifyCallback(int ok, X509_STORE_CTX* store);
-  int SSLVerifyInternal(int ok, SSL* ssl, X509_STORE_CTX* store);
+  // Call a custom verifier, if installed.
+  // Returns 1 on success, `status_on_error` on error or verification failure.
+  int SSLVerifyInternal(int status_on_error, SSL* ssl, X509_STORE_CTX* store);
 #endif
   friend class OpenSSLStreamAdapter;  // for custom_verify_callback_;