Refactor uses of setClientCertificate to setClientCertManager

Summary: use the new setClientCertManager method instead of the deprecated setClientCertificate method and remove it from FizzClientContext.h Reviewed By: zxjtan Differential Revision: D62404702 fbshipit-source-id: 936c19c31499558043e50027875143edebb4539c
facebookincubator · Sep 16, 2024 · 15356f4 · 15356f4
1 parent 419b4ad
commit 15356f4
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 27 deletions.
diff --git a/fizz/client/CertManager.cpp b/fizz/client/CertManager.cpp
@@ -42,6 +42,9 @@ void CertManager::addCertAndOverride(std::shared_ptr<SelfCert> cert) {
 void CertManager::addCert(
     std::shared_ptr<SelfCert> cert,
     bool overrideExistingEntry) {
+  if (cert == nullptr) {
+    return;
+  }
   auto sigSchemes = cert->getSigSchemes();
   for (auto sigScheme : sigSchemes) {
     if (certs_.find(sigScheme) == certs_.end() || overrideExistingEntry) {

diff --git a/fizz/client/FizzClientContext.h b/fizz/client/FizzClientContext.h
@@ -123,26 +123,6 @@ class FizzClientContext {
     return echOuterExtensionTypes_;
   }
 
-  /**
-   * This is a legacy api, prefer setClientCertManager.
-   * Sets the certificate to use if the server requests client authentication.
-   * This api is meant to be used when you expect
-   * to only be presenting one possible cert. This will overwrite any
-   * pre-existing configuration.
-   */
-  [[deprecated("Use FizzClientContext::setClientCertManager")]]
-  void setClientCertificate(std::shared_ptr<SelfCert> cert) {
-    // Blow away any existing certs on the context.
-    if (cert != nullptr) {
-      auto certMgr = std::make_shared<CertManager>();
-      clientCert_ = cert;
-      certMgr->addCertAndOverride(std::move(cert));
-      certManager_ = std::move(certMgr);
-    } else {
-      certManager_ = nullptr;
-    }
-  }
-
   /*
    * Sets the certificate manager to select a cert if the server requests client
    * auth

diff --git a/fizz/client/test/ClientProtocolTest.cpp b/fizz/client/test/ClientProtocolTest.cpp
@@ -214,7 +214,9 @@ class ClientProtocolTest : public ProtocolTest<ClientTypes, Actions> {
   void setupExpectingCertificateRequest() {
     setMockRecord();
     setMockContextAndScheduler();
-    context_->setClientCertificate(mockClientCert_);
+    auto certMgr = std::make_shared<fizz::client::CertManager>();
+    certMgr->addCert(mockClientCert_);
+    context_->setClientCertManager(std::move(certMgr));
     state_.context() = context_;
     state_.state() = StateEnum::ExpectingCertificate;
     state_.handshakeTime() =

diff --git a/fizz/test/BogoShim.cpp b/fizz/test/BogoShim.cpp
@@ -351,7 +351,9 @@ int clientTest() {
   clientContext->setCompatibilityMode(true);
 
   if (!FLAGS_cert_file.empty()) {
-    clientContext->setClientCertificate(readSelfCert());
+    auto certMgr = std::make_shared<fizz::client::CertManager>();
+    certMgr->addCert(readSelfCert());
+    clientContext->setClientCertManager(std::move(certMgr));
   }
 
   EventBase evb;

diff --git a/fizz/test/HandshakeTest.cpp b/fizz/test/HandshakeTest.cpp
@@ -397,15 +397,17 @@ TEST_F(HandshakeTest, CertRequestPskPreservesIdentity) {
 
 TEST_F(HandshakeTest, CertRequestNoCert) {
   serverContext_->setClientAuthMode(ClientAuthMode::Required);
-  clientContext_->setClientCertificate(nullptr);
+  auto certMgr = std::make_shared<fizz::client::CertManager>();
+  clientContext_->setClientCertManager(std::move(certMgr));
   expectServerError(
       "alert: certificate_required", "certificate requested but none received");
   doHandshake();
 }
 
 TEST_F(HandshakeTest, CertRequestPermitNoCert) {
   serverContext_->setClientAuthMode(ClientAuthMode::Optional);
-  clientContext_->setClientCertificate(nullptr);
+  auto certMgr = std::make_shared<fizz::client::CertManager>();
+  clientContext_->setClientCertManager(std::move(certMgr));
   expectSuccess();
   doHandshake();
   verifyParameters();
@@ -417,9 +419,11 @@ TEST_F(HandshakeTest, CertRequestBadCert) {
   auto badCert = createCert("foo", false, nullptr);
   std::vector<folly::ssl::X509UniquePtr> certVec;
   certVec.emplace_back(std::move(badCert.cert));
-  clientContext_->setClientCertificate(
+  auto certMgr = std::make_shared<fizz::client::CertManager>();
+  certMgr->addCert(
       std::make_shared<openssl::OpenSSLSelfCertImpl<openssl::KeyType::P256>>(
           std::move(badCert.key), std::move(certVec)));
+  clientContext_->setClientCertManager(std::move(certMgr));
   expectServerError("alert: bad_certificate", "client certificate failure");
   doHandshake();
 }

diff --git a/fizz/test/HandshakeTest.h b/fizz/test/HandshakeTest.h
@@ -107,7 +107,9 @@ class HandshakeTest : public Test {
     auto clientSelfCert =
         std::make_shared<openssl::OpenSSLSelfCertImpl<openssl::KeyType::RSA>>(
             std::move(clientKey), std::move(certVec));
-    clientContext_->setClientCertificate(std::move(clientSelfCert));
+    auto certMgr = std::make_shared<fizz::client::CertManager>();
+    certMgr->addCert(std::move(clientSelfCert));
+    clientContext_->setClientCertManager(std::move(certMgr));
 
     auto ticketCipher = std::make_shared<AES128TicketCipher>(
         serverContext_->getFactoryPtr(), std::move(certManager));

diff --git a/fizz/tool/FizzClientCommand.cpp b/fizz/tool/FizzClientCommand.cpp
@@ -772,7 +772,9 @@ int fizzClientCommand(const std::vector<std::string>& args) {
     } else {
       cert = openssl::CertUtils::makeSelfCert(certData, keyData);
     }
-    clientContext->setClientCertificate(std::move(cert));
+    auto certMgr = std::make_shared<fizz::client::CertManager>();
+    certMgr->addCert(std::move(cert));
+    clientContext->setClientCertManager(std::move(certMgr));
   }
 
   std::shared_ptr<ClientExtensions> extensions;