Fix hostname validations for unit testings

Summary:
- This diff disables the hostname validation when the inner verifier is a InsecureCertificateVerifier. There's no need to perform hostname validation when the certificate verifier is being disabled.
- Lots of hostname validation failure in unit testings, since they use loopback or other IPs. This change also skips hostname validation for these unit tests, since they all use the InsecureCertificateVerifier already.

Reviewed By: mingtaoy

Differential Revision: D116676469

fbshipit-source-id: af86c2f6243d5abd32b2ffa14f8cb48c5bc4b4f3

Author: Huilin Chen

proxygen/httpserver/samples/hq/InsecureVerifierDangerousDoNotUseInProduction.h

@@ -16,8 +16,12 @@ namespace proxygen {
 // used in production. Using it in production would mean that this will
 // leave everyone insecure.
 class InsecureVerifierDangerousDoNotUseInProduction
-    : public fizz::CertificateVerifier {
+    : public fizz::InsecureCertificateVerifier {
  public:
+  InsecureVerifierDangerousDoNotUseInProduction()
+      : fizz::InsecureCertificateVerifier(fizz::VerificationContext::Client) {
+  }
+
   ~InsecureVerifierDangerousDoNotUseInProduction() override = default;
 
   fizz::Status verify(std::shared_ptr<const fizz::Cert>& ret,

proxygen/lib/http/coro/client/ProxygenCertVerifier.cpp

@@ -162,11 +162,15 @@ std::optional<folly::IPAddress> ExpectedIdentity::getIp() const {
   return std::nullopt;
 }
 
-std::shared_ptr<fizz::CertificateVerifier> makeVerifier(
+std::shared_ptr<const fizz::CertificateVerifier> makeVerifier(
     std::shared_ptr<const fizz::CertificateVerifier> verifier,
     ExpectedIdentity expectedIdentity,
     ValidationPolicy policy,
     CertVerifyLogFn logFn) {
+  if (std::dynamic_pointer_cast<const fizz::InsecureCertificateVerifier>(
+          verifier)) {
+    return verifier;
+  }
   return std::make_shared<ProxygenCertVerifier>(std::move(verifier),
                                                 std::move(expectedIdentity),
                                                 policy,

proxygen/lib/http/coro/client/ProxygenCertVerifier.h

@@ -67,11 +67,13 @@ struct ExpectedIdentity {
  * Creates a fizz::CertificateVerifier that validates the peer certificate chain
  * with `verifier` and, on top of that, verifies that the leaf certificate
  * matches `expectedIdentity`, handling a mismatch according to `policy`.
+ * An InsecureCertificateVerifier is returned directly without identity
+ * verification.
  *
  * `verifier` must not be null. `logFn`, if set, is invoked with
  * CertVerifyResult and error message if applicable.
  */
-std::shared_ptr<fizz::CertificateVerifier> makeVerifier(
+std::shared_ptr<const fizz::CertificateVerifier> makeVerifier(
     std::shared_ptr<const fizz::CertificateVerifier> verifier,
     ExpectedIdentity expectedIdentity,
     ValidationPolicy policy,

proxygen/lib/http/coro/client/test/ProxygenCertVerifierTest.cpp

@@ -34,7 +34,7 @@ class ProxygenCertVerifierTest : public testing::Test {
   }
 
  protected:
-  std::shared_ptr<fizz::CertificateVerifier> makeVerifier(
+  std::shared_ptr<const fizz::CertificateVerifier> makeVerifier(
       ExpectedIdentity expectedIdentity, ValidationPolicy policy) {
     folly::ssl::X509StoreUniquePtr store(X509_STORE_new());
     EXPECT_EQ(X509_STORE_add_cert(store.get(), rootCertAndKey_.cert.get()), 1);
@@ -109,6 +109,18 @@ TEST_F(ProxygenCertVerifierTest, UnderlyingVerifierFailure) {
             Status::Fail);
 }
 
+TEST_F(ProxygenCertVerifierTest, InsecureVerifierReturnedDirectly) {
+  auto insecureVerifier = std::make_shared<InsecureCertificateVerifier>(
+      VerificationContext::Client);
+
+  auto verifier = coro::makeVerifier(insecureVerifier,
+                                     ExpectedIdentity::expectDNS("example.com"),
+                                     ValidationPolicy::Enforcing,
+                                     nullptr);
+
+  EXPECT_EQ(verifier, insecureVerifier);
+}
+
 TEST_F(ProxygenCertVerifierTest, NullInnerVerifierFailsAtConstruction) {
   EXPECT_DEATH(coro::makeVerifier(nullptr,
                                   ExpectedIdentity::expectDNS("example.com"),

proxygen/lib/http/coro/test/TestUtils.h

@@ -15,8 +15,12 @@ namespace proxygen::coro::test {
 // used in production. Using it in production would mean that this will
 // leave everyone insecure.
 class InsecureVerifierDangerousDoNotUseInProduction
-    : public fizz::CertificateVerifier {
+    : public fizz::InsecureCertificateVerifier {
  public:
+  InsecureVerifierDangerousDoNotUseInProduction()
+      : fizz::InsecureCertificateVerifier(fizz::VerificationContext::Client) {
+  }
+
   ~InsecureVerifierDangerousDoNotUseInProduction() override = default;
 
   fizz::Status verify(std::shared_ptr<const fizz::Cert>& ret,