mobile: Enable setting the Platform Cert Validator thread priority (e…

…nvoyproxy#36104)

If the Envoy Mobile thread has a thread priority set, the Platform Cert
Validator's validation thread also has the thread priority set to the
same value.

---------

Signed-off-by: Ali Beyad <[email protected]>

mobile/library/cc/engine_builder.cc

@@ -554,6 +554,9 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> EngineBuilder::generate
   if (platform_certificates_validation_on_) {
     envoy_mobile::extensions::cert_validator::platform_bridge::PlatformBridgeCertValidator
         validator;
+    if (network_thread_priority_.has_value()) {
+      validator.mutable_thread_priority()->set_value(*network_thread_priority_);
+    }
     validation->mutable_custom_validator_config()->set_name(
         "envoy_mobile.cert_validator.platform_bridge_cert_validator");
     validation->mutable_custom_validator_config()->mutable_typed_config()->PackFrom(validator);

mobile/library/common/extensions/cert_validator/platform_bridge/BUILD

@@ -38,6 +38,7 @@ envoy_cc_library(
         "@envoy//envoy/thread:thread_interface",
         "@envoy//source/common/common:macros",
         "@envoy//source/common/common:thread_impl_lib_posix",
+        "@envoy//source/common/config:utility_lib",
         "@envoy//source/common/tls/cert_validator:cert_validator_lib",
     ],
 )

mobile/library/common/extensions/cert_validator/platform_bridge/platform_bridge.proto

@@ -2,5 +2,15 @@ syntax = "proto3";
 
 package envoy_mobile.extensions.cert_validator.platform_bridge;
 
+import "google/protobuf/wrappers.proto";
+
+// Configuration for the platform bridge cert validator.
 message PlatformBridgeCertValidator {
+  // The thread priority that will be set on the thread that is created to execute platform cert
+  // validation. The exact values and meaning of the thread priority is OS dependent. For example,
+  // on Android, the values range from -20 to 19. On iOS, supply a value between 1 to 100, which
+  // will be divided by 100 to provide a value to the OS in the range of 0 to 1.
+  //
+  // If this field is not set, the platform-specific default thread priorities will be used.
+  google.protobuf.Int32Value thread_priority = 1;
 }

mobile/library/common/extensions/cert_validator/platform_bridge/platform_bridge_cert_validator.cc

@@ -3,7 +3,11 @@
 #include <list>
 #include <memory>
 
+#include "source/common/config/utility.h"
+#include "source/common/protobuf/message_validator_impl.h"
+
 #include "library/common/bridge//utility.h"
+#include "library/common/extensions/cert_validator/platform_bridge/platform_bridge.pb.h"
 #include "library/common/system/system_helper.h"
 
 namespace Envoy {
@@ -22,6 +26,15 @@ PlatformBridgeCertValidator::PlatformBridgeCertValidator(
   ENVOY_BUG(config != nullptr && config->caCert().empty() &&
                 config->certificateRevocationList().empty(),
             "Invalid certificate validation context config.");
+  if (config != nullptr && config->customValidatorConfig().has_value()) {
+    envoy_mobile::extensions::cert_validator::platform_bridge::PlatformBridgeCertValidator cfg;
+    Envoy::Config::Utility::translateOpaqueConfig(
+        config->customValidatorConfig().value().typed_config(),
+        ProtobufMessage::getStrictValidationVisitor(), cfg);
+    if (cfg.has_thread_priority()) {
+      thread_priority_ = cfg.thread_priority().value();
+    }
+  }
 }
 
 PlatformBridgeCertValidator::PlatformBridgeCertValidator(
@@ -89,12 +102,14 @@ ValidationResults PlatformBridgeCertValidator::doVerifyCertChain(
   ValidationJob job;
   job.result_callback_ = std::move(callback);
   Event::Dispatcher& dispatcher = job.result_callback_->dispatcher();
+  Thread::Options thread_options;
+  thread_options.priority_ = thread_priority_;
   job.validation_thread_ = thread_factory_->createThread(
       [this, &dispatcher, certs = std::move(certs), host = std::string(host),
        subject_alt_names = std::move(subject_alt_names)]() -> void {
-        verifyCertChainByPlatform(&dispatcher, certs, host, subject_alt_names, this);
+        verifyCertChainByPlatform(&dispatcher, certs, host, subject_alt_names);
       },
-      /* options= */ absl::nullopt, /* crash_on_failure=*/false);
+      thread_options, /* crash_on_failure=*/false);
   if (job.validation_thread_ == nullptr) {
     return {ValidationResults::ValidationStatus::Failed,
             Envoy::Ssl::ClientValidationStatus::NotValidated, absl::nullopt,
@@ -108,7 +123,7 @@ ValidationResults PlatformBridgeCertValidator::doVerifyCertChain(
 
 void PlatformBridgeCertValidator::verifyCertChainByPlatform(
     Event::Dispatcher* dispatcher, std::vector<std::string> cert_chain, std::string hostname,
-    std::vector<std::string> subject_alt_names, PlatformBridgeCertValidator* parent) {
+    std::vector<std::string> subject_alt_names) {
   ASSERT(!cert_chain.empty());
   ENVOY_LOG(trace, "Start verifyCertChainByPlatform for host {}", hostname);
   // This is running in a stand alone thread other than the engine thread.
@@ -122,7 +137,7 @@ void PlatformBridgeCertValidator::verifyCertChainByPlatform(
   if (!success) {
     ENVOY_LOG(debug, result.error_details);
     postVerifyResultAndCleanUp(success, std::move(hostname), result.error_details, result.tls_alert,
-                               ValidationFailureType::FailVerifyError, dispatcher, parent);
+                               ValidationFailureType::FailVerifyError, dispatcher, this);
     return;
   }
 
@@ -135,12 +150,12 @@ void PlatformBridgeCertValidator::verifyCertChainByPlatform(
     error_details = "PlatformBridgeCertValidator_verifySubjectAltName failed: SNI mismatch.";
     ENVOY_LOG(debug, error_details);
     postVerifyResultAndCleanUp(success, std::move(hostname), error_details, SSL_AD_BAD_CERTIFICATE,
-                               ValidationFailureType::FailVerifySan, dispatcher, parent);
+                               ValidationFailureType::FailVerifySan, dispatcher, this);
     return;
   }
   postVerifyResultAndCleanUp(success, std::move(hostname), error_details,
                              SSL_AD_CERTIFICATE_UNKNOWN, ValidationFailureType::Success, dispatcher,
-                             parent);
+                             this);
 }
 
 void PlatformBridgeCertValidator::postVerifyResultAndCleanUp(bool success, std::string hostname,

mobile/library/common/extensions/cert_validator/platform_bridge/platform_bridge_cert_validator.h

@@ -57,12 +57,7 @@ class PlatformBridgeCertValidator : public CertValidator, Logger::Loggable<Logge
     return SSL_VERIFY_PEER;
   }
 
-private:
-  GTEST_FRIEND_CLASS(PlatformBridgeCertValidatorTest, ThreadCreationFailed);
-
-  PlatformBridgeCertValidator(const Envoy::Ssl::CertificateValidationContextConfig* config,
-                              SslStats& stats, Thread::PosixThreadFactoryPtr thread_factory);
-
+protected:
   enum class ValidationFailureType {
     Success,
     FailVerifyError,
@@ -73,18 +68,26 @@ class PlatformBridgeCertValidator : public CertValidator, Logger::Loggable<Logge
   // Once the validation is done, the result will be posted back to the current
   // thread to trigger callback and update verify stats.
   // Must be called on the validation thread.
-  static void verifyCertChainByPlatform(Event::Dispatcher* dispatcher,
-                                        std::vector<std::string> cert_chain, std::string hostname,
-                                        std::vector<std::string> subject_alt_names,
-                                        PlatformBridgeCertValidator* parent);
+  //
+  // `protected` for testing purposes.
+  virtual void verifyCertChainByPlatform(Event::Dispatcher* dispatcher,
+                                         std::vector<std::string> cert_chain, std::string hostname,
+                                         std::vector<std::string> subject_alt_names);
 
   // Must be called on the validation thread.
+  // `protected` for testing purposes.
   static void postVerifyResultAndCleanUp(bool success, std::string hostname,
                                          absl::string_view error_details, uint8_t tls_alert,
                                          ValidationFailureType failure_type,
                                          Event::Dispatcher* dispatcher,
                                          PlatformBridgeCertValidator* parent);
 
+private:
+  GTEST_FRIEND_CLASS(PlatformBridgeCertValidatorTest, ThreadCreationFailed);
+
+  PlatformBridgeCertValidator(const Envoy::Ssl::CertificateValidationContextConfig* config,
+                              SslStats& stats, Thread::PosixThreadFactoryPtr thread_factory);
+
   // Called when a pending verification completes. Must be invoked on the main thread.
   void onVerificationComplete(const Thread::ThreadId& thread_id, const std::string& hostname,
                               bool success, const std::string& error_details, uint8_t tls_alert,
@@ -100,6 +103,7 @@ class PlatformBridgeCertValidator : public CertValidator, Logger::Loggable<Logge
   absl::flat_hash_map<Thread::ThreadId, ValidationJob> validation_jobs_;
   std::shared_ptr<size_t> alive_indicator_{new size_t(1)};
   Thread::PosixThreadFactoryPtr thread_factory_;
+  absl::optional<int> thread_priority_;
 };
 
 } // namespace Tls

mobile/test/common/extensions/cert_validator/platform_bridge/BUILD

@@ -18,6 +18,7 @@ envoy_extension_cc_test(
     repository = "@envoy",
     deps = [
         "//library/common/extensions/cert_validator/platform_bridge:config",
+        "//library/common/extensions/cert_validator/platform_bridge:platform_bridge_cc_proto",
         "//test/common/mocks/common:common_mocks",
         "@envoy//source/common/tls/cert_validator:cert_validator_lib",
         "@envoy//test/common/tls:ssl_test_utils",

mobile/test/common/extensions/cert_validator/platform_bridge/platform_bridge_cert_validator_test.cc

@@ -24,6 +24,7 @@
 #include "gtest/gtest.h"
 #include "library/common/bridge/utility.h"
 #include "library/common/extensions/cert_validator/platform_bridge/config.h"
+#include "library/common/extensions/cert_validator/platform_bridge/platform_bridge.pb.h"
 #include "openssl/ssl.h"
 #include "openssl/x509v3.h"
 
@@ -60,6 +61,30 @@ class MockValidator {
               (const std::vector<std::string>& certs, absl::string_view hostname));
 };
 
+class PlatformBridgeCertValidatorCustomValidate : public PlatformBridgeCertValidator {
+public:
+  PlatformBridgeCertValidatorCustomValidate(
+      const Envoy::Ssl::CertificateValidationContextConfig* config, SslStats& stats,
+      Thread::PosixThreadFactory& thread_factory)
+      : PlatformBridgeCertValidator(config, stats), thread_factory_(thread_factory) {}
+
+  int recordedThreadPriority() const { return recorded_thread_priority_; }
+
+protected:
+  void verifyCertChainByPlatform(Event::Dispatcher* dispatcher,
+                                 std::vector<std::string> /* cert_chain */, std::string hostname,
+                                 std::vector<std::string> /* subject_alt_names */) override {
+    recorded_thread_priority_ = thread_factory_.currentThreadPriority();
+    postVerifyResultAndCleanUp(/* success = */ true, std::move(hostname), "",
+                               SSL_AD_CERTIFICATE_UNKNOWN, ValidationFailureType::Success,
+                               dispatcher, this);
+  }
+
+private:
+  Thread::PosixThreadFactory& thread_factory_;
+  int recorded_thread_priority_;
+};
+
 class PlatformBridgeCertValidatorTest
     : public testing::TestWithParam<CertificateValidationContext::TrustChainVerification> {
 protected:
@@ -84,6 +109,8 @@ class PlatformBridgeCertValidatorTest
     EXPECT_CALL(config_, caCert()).WillOnce(ReturnRef(empty_string_));
     EXPECT_CALL(config_, certificateRevocationList()).WillOnce(ReturnRef(empty_string_));
     EXPECT_CALL(config_, trustChainVerification()).WillOnce(Return(GetParam()));
+    EXPECT_CALL(config_, customValidatorConfig())
+        .WillRepeatedly(ReturnRef(platform_bridge_config_));
   }
 
   ~PlatformBridgeCertValidatorTest() {
@@ -136,6 +163,7 @@ class PlatformBridgeCertValidatorTest
   std::unique_ptr<MockValidator> mock_validator_;
   Thread::ThreadId main_thread_id_;
   std::unique_ptr<test::SystemHelperPeer::Handle> helper_handle_;
+  absl::optional<envoy::config::core::v3::TypedExtensionConfig> platform_bridge_config_;
 };
 
 INSTANTIATE_TEST_SUITE_P(TrustMode, PlatformBridgeCertValidatorTest,
@@ -152,6 +180,7 @@ TEST_P(PlatformBridgeCertValidatorTest, NonEmptyCaCert) {
   EXPECT_CALL(config_, caCert()).WillRepeatedly(ReturnRef(ca_cert));
   EXPECT_CALL(config_, certificateRevocationList()).WillRepeatedly(ReturnRef(empty_string_));
   EXPECT_CALL(config_, trustChainVerification()).WillRepeatedly(Return(GetParam()));
+  EXPECT_CALL(config_, customValidatorConfig()).WillRepeatedly(ReturnRef(platform_bridge_config_));
 
   EXPECT_ENVOY_BUG({ PlatformBridgeCertValidator validator(&config_, stats_); },
                    "Invalid certificate validation context config.");
@@ -162,6 +191,7 @@ TEST_P(PlatformBridgeCertValidatorTest, NonEmptyRevocationList) {
   EXPECT_CALL(config_, caCert()).WillRepeatedly(ReturnRef(empty_string_));
   EXPECT_CALL(config_, certificateRevocationList()).WillRepeatedly(ReturnRef(revocation_list));
   EXPECT_CALL(config_, trustChainVerification()).WillRepeatedly(Return(GetParam()));
+  EXPECT_CALL(config_, customValidatorConfig()).WillRepeatedly(ReturnRef(platform_bridge_config_));
 
   EXPECT_ENVOY_BUG({ PlatformBridgeCertValidator validator(&config_, stats_); },
                    "Invalid certificate validation context config.");
@@ -412,6 +442,42 @@ TEST_P(PlatformBridgeCertValidatorTest, ThreadCreationFailed) {
   EXPECT_EQ("Failed creating a thread for cert chain validation.", *results.error_details);
 }
 
+TEST_P(PlatformBridgeCertValidatorTest, ThreadPriority) {
+  const int expected_thread_priority = 15;
+  envoy_mobile::extensions::cert_validator::platform_bridge::PlatformBridgeCertValidator
+      platform_bridge_config;
+  platform_bridge_config.mutable_thread_priority()->set_value(expected_thread_priority);
+  envoy::config::core::v3::TypedExtensionConfig typed_config;
+  typed_config.set_name("PlatformBridgeCertValidator");
+  typed_config.mutable_typed_config()->PackFrom(platform_bridge_config);
+  platform_bridge_config_ = std::move(typed_config);
+
+  EXPECT_CALL(helper_handle_->mock_helper(), cleanupAfterCertificateValidation());
+
+  initializeConfig();
+  PlatformBridgeCertValidatorCustomValidate validator(&config_, stats_, *thread_factory_);
+
+  std::string hostname = "server1.example.com";
+  bssl::UniquePtr<STACK_OF(X509)> cert_chain = readCertChainFromFile(
+      TestEnvironment::substitute("{{ test_rundir }}/test/common/tls/test_data/san_dns2_cert.pem"));
+  EXPECT_CALL(*mock_validator_, cleanup());
+  auto& callback_ref = *callback_;
+  EXPECT_CALL(callback_ref, dispatcher()).WillRepeatedly(ReturnRef(*dispatcher_));
+
+  ValidationResults results =
+      validator.doVerifyCertChain(*cert_chain, std::move(callback_), transport_socket_options_,
+                                  *ssl_ctx_, validation_context_, is_server_, hostname);
+  EXPECT_EQ(ValidationResults::ValidationStatus::Pending, results.status);
+
+  EXPECT_CALL(callback_ref,
+              onCertValidationResult(true, Envoy::Ssl::ClientValidationStatus::Validated, "", 46))
+      .WillOnce(Invoke([this, &validator, expected_thread_priority]() {
+        EXPECT_EQ(validator.recordedThreadPriority(), expected_thread_priority);
+        dispatcher_->exit();
+      }));
+  EXPECT_FALSE(waitForDispatcherToExit());
+}
+
 } // namespace Tls
 } // namespace TransportSockets
 } // namespace Extensions