From ddc37dc7f6e31704cf44998fb8f505d8af1e53e1 Mon Sep 17 00:00:00 2001 From: Abdulkadir Fiqi Date: Mon, 16 Sep 2024 07:23:41 -0700 Subject: [PATCH] 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 --- .../fizz/src/fizz/client/CertManager.cpp | 3 +++ .../fizz/src/fizz/client/FizzClientContext.h | 20 ------------------- .../fizz/client/test/ClientProtocolTest.cpp | 4 +++- third-party/fizz/src/fizz/test/BogoShim.cpp | 4 +++- .../fizz/src/fizz/test/HandshakeTest.cpp | 10 +++++++--- .../fizz/src/fizz/test/HandshakeTest.h | 4 +++- .../fizz/src/fizz/tool/FizzClientCommand.cpp | 4 +++- .../lib/network/FizzContextProvider.cpp | 4 +++- .../httpclient/samples/httperf2/Client.cpp | 4 +++- .../httpserver/samples/hq/FizzContext.cpp | 4 +++- .../lib/transport/H3DatagramAsyncSocket.cpp | 4 +++- .../stresstest/client/ClientFactory.cpp | 4 +++- .../lib/cpp2/test/server/ThriftServerTest.cpp | 5 +++-- .../integration/cpp2/zero_copy_client.cpp | 4 +++- 14 files changed, 43 insertions(+), 35 deletions(-) diff --git a/third-party/fizz/src/fizz/client/CertManager.cpp b/third-party/fizz/src/fizz/client/CertManager.cpp index b29a993076f80..287cd942c1032 100644 --- a/third-party/fizz/src/fizz/client/CertManager.cpp +++ b/third-party/fizz/src/fizz/client/CertManager.cpp @@ -42,6 +42,9 @@ void CertManager::addCertAndOverride(std::shared_ptr cert) { void CertManager::addCert( std::shared_ptr 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/third-party/fizz/src/fizz/client/FizzClientContext.h b/third-party/fizz/src/fizz/client/FizzClientContext.h index 4f4bc2fb47a33..9a81329fcf5d7 100644 --- a/third-party/fizz/src/fizz/client/FizzClientContext.h +++ b/third-party/fizz/src/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 cert) { - // Blow away any existing certs on the context. - if (cert != nullptr) { - auto certMgr = std::make_shared(); - 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/third-party/fizz/src/fizz/client/test/ClientProtocolTest.cpp b/third-party/fizz/src/fizz/client/test/ClientProtocolTest.cpp index 75446937b709a..ce9c4f1518db6 100644 --- a/third-party/fizz/src/fizz/client/test/ClientProtocolTest.cpp +++ b/third-party/fizz/src/fizz/client/test/ClientProtocolTest.cpp @@ -214,7 +214,9 @@ class ClientProtocolTest : public ProtocolTest { void setupExpectingCertificateRequest() { setMockRecord(); setMockContextAndScheduler(); - context_->setClientCertificate(mockClientCert_); + auto certMgr = std::make_shared(); + certMgr->addCert(mockClientCert_); + context_->setClientCertManager(std::move(certMgr)); state_.context() = context_; state_.state() = StateEnum::ExpectingCertificate; state_.handshakeTime() = diff --git a/third-party/fizz/src/fizz/test/BogoShim.cpp b/third-party/fizz/src/fizz/test/BogoShim.cpp index a2376090a3834..f4b7f76613f5a 100644 --- a/third-party/fizz/src/fizz/test/BogoShim.cpp +++ b/third-party/fizz/src/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(); + certMgr->addCert(readSelfCert()); + clientContext->setClientCertManager(std::move(certMgr)); } EventBase evb; diff --git a/third-party/fizz/src/fizz/test/HandshakeTest.cpp b/third-party/fizz/src/fizz/test/HandshakeTest.cpp index c39dacfb0e348..14b95acafad6b 100644 --- a/third-party/fizz/src/fizz/test/HandshakeTest.cpp +++ b/third-party/fizz/src/fizz/test/HandshakeTest.cpp @@ -397,7 +397,8 @@ TEST_F(HandshakeTest, CertRequestPskPreservesIdentity) { TEST_F(HandshakeTest, CertRequestNoCert) { serverContext_->setClientAuthMode(ClientAuthMode::Required); - clientContext_->setClientCertificate(nullptr); + auto certMgr = std::make_shared(); + clientContext_->setClientCertManager(std::move(certMgr)); expectServerError( "alert: certificate_required", "certificate requested but none received"); doHandshake(); @@ -405,7 +406,8 @@ TEST_F(HandshakeTest, CertRequestNoCert) { TEST_F(HandshakeTest, CertRequestPermitNoCert) { serverContext_->setClientAuthMode(ClientAuthMode::Optional); - clientContext_->setClientCertificate(nullptr); + auto certMgr = std::make_shared(); + clientContext_->setClientCertManager(std::move(certMgr)); expectSuccess(); doHandshake(); verifyParameters(); @@ -417,9 +419,11 @@ TEST_F(HandshakeTest, CertRequestBadCert) { auto badCert = createCert("foo", false, nullptr); std::vector certVec; certVec.emplace_back(std::move(badCert.cert)); - clientContext_->setClientCertificate( + auto certMgr = std::make_shared(); + certMgr->addCert( std::make_shared>( std::move(badCert.key), std::move(certVec))); + clientContext_->setClientCertManager(std::move(certMgr)); expectServerError("alert: bad_certificate", "client certificate failure"); doHandshake(); } diff --git a/third-party/fizz/src/fizz/test/HandshakeTest.h b/third-party/fizz/src/fizz/test/HandshakeTest.h index 19fc7c06d9e4a..0cd212c6ed0b7 100644 --- a/third-party/fizz/src/fizz/test/HandshakeTest.h +++ b/third-party/fizz/src/fizz/test/HandshakeTest.h @@ -107,7 +107,9 @@ class HandshakeTest : public Test { auto clientSelfCert = std::make_shared>( std::move(clientKey), std::move(certVec)); - clientContext_->setClientCertificate(std::move(clientSelfCert)); + auto certMgr = std::make_shared(); + certMgr->addCert(std::move(clientSelfCert)); + clientContext_->setClientCertManager(std::move(certMgr)); auto ticketCipher = std::make_shared( serverContext_->getFactoryPtr(), std::move(certManager)); diff --git a/third-party/fizz/src/fizz/tool/FizzClientCommand.cpp b/third-party/fizz/src/fizz/tool/FizzClientCommand.cpp index 5eba31be45a10..3017500f2c5c9 100644 --- a/third-party/fizz/src/fizz/tool/FizzClientCommand.cpp +++ b/third-party/fizz/src/fizz/tool/FizzClientCommand.cpp @@ -772,7 +772,9 @@ int fizzClientCommand(const std::vector& args) { } else { cert = openssl::CertUtils::makeSelfCert(certData, keyData); } - clientContext->setClientCertificate(std::move(cert)); + auto certMgr = std::make_shared(); + certMgr->addCert(std::move(cert)); + clientContext->setClientCertManager(std::move(certMgr)); } std::shared_ptr extensions; diff --git a/third-party/mcrouter/src/mcrouter/lib/network/FizzContextProvider.cpp b/third-party/mcrouter/src/mcrouter/lib/network/FizzContextProvider.cpp index 51e64dd76201d..3a2064c3794f6 100644 --- a/third-party/mcrouter/src/mcrouter/lib/network/FizzContextProvider.cpp +++ b/third-party/mcrouter/src/mcrouter/lib/network/FizzContextProvider.cpp @@ -45,7 +45,9 @@ FizzContextAndVerifier createClientFizzContextAndVerifier( if (!certData.empty() && !keyData.empty()) { auto cert = fizz::openssl::CertUtils::makeSelfCert( std::move(certData), std::move(keyData)); - ctx->setClientCertificate(std::move(cert)); + auto certMgr = std::make_shared(); + certMgr->addCert(std::move(cert)); + ctx->setClientCertManager(std::move(certMgr)); } std::shared_ptr verifier; if (!pemCaPath.empty()) { diff --git a/third-party/proxygen/src/proxygen/httpclient/samples/httperf2/Client.cpp b/third-party/proxygen/src/proxygen/httpclient/samples/httperf2/Client.cpp index 693d4554e5a80..fc44aff95562b 100644 --- a/third-party/proxygen/src/proxygen/httpclient/samples/httperf2/Client.cpp +++ b/third-party/proxygen/src/proxygen/httpclient/samples/httperf2/Client.cpp @@ -154,7 +154,9 @@ void Client::setupFizzContext(std::shared_ptr pskCache, folly::readFile(key.c_str(), keyData); } auto selfCert = fizz::openssl::CertUtils::makeSelfCert(certData, keyData); - fizzContext_->setClientCertificate(std::move(selfCert)); + auto certMgr = std::make_shared(); + certMgr->addCert(std::move(selfCert)); + fizzContext_->setClientCertManager(std::move(certMgr)); } } diff --git a/third-party/proxygen/src/proxygen/httpserver/samples/hq/FizzContext.cpp b/third-party/proxygen/src/proxygen/httpserver/samples/hq/FizzContext.cpp index 1ba23084bfb6d..40ab0a310e905 100644 --- a/third-party/proxygen/src/proxygen/httpserver/samples/hq/FizzContext.cpp +++ b/third-party/proxygen/src/proxygen/httpserver/samples/hq/FizzContext.cpp @@ -201,7 +201,9 @@ FizzClientContextPtr createFizzClientContext(const HQBaseParams& params, folly::readFile(params.keyFilePath.c_str(), keyData); } auto cert = fizz::openssl::CertUtils::makeSelfCert(certData, keyData); - ctx->setClientCertificate(std::move(cert)); + auto certMgr = std::make_shared(); + certMgr->addCert(std::move(cert)); + ctx->setClientCertManager(std::move(certMgr)); ctx->setSupportedAlpns(params.supportedAlpns); ctx->setDefaultShares( {fizz::NamedGroup::x25519, fizz::NamedGroup::secp256r1}); diff --git a/third-party/proxygen/src/proxygen/lib/transport/H3DatagramAsyncSocket.cpp b/third-party/proxygen/src/proxygen/lib/transport/H3DatagramAsyncSocket.cpp index 423f12cbf92b5..39b8bbf4e640d 100644 --- a/third-party/proxygen/src/proxygen/lib/transport/H3DatagramAsyncSocket.cpp +++ b/third-party/proxygen/src/proxygen/lib/transport/H3DatagramAsyncSocket.cpp @@ -310,7 +310,9 @@ H3DatagramAsyncSocket::createFizzClientContext() { std::string keyData; folly::readFile(options_.certAndKey_->second.c_str(), keyData); auto cert = fizz::openssl::CertUtils::makeSelfCert(certData, keyData); - ctx->setClientCertificate(std::move(cert)); + auto certMgr = std::make_shared(); + certMgr->addCert(std::move(cert)); + ctx->setClientCertManager(std::move(certMgr)); } std::vector supportedAlpns = {proxygen::kH3FBCurrentDraft}; diff --git a/third-party/thrift/src/thrift/conformance/stresstest/client/ClientFactory.cpp b/third-party/thrift/src/thrift/conformance/stresstest/client/ClientFactory.cpp index 70075a693c872..de7d7d2297024 100644 --- a/third-party/thrift/src/thrift/conformance/stresstest/client/ClientFactory.cpp +++ b/third-party/thrift/src/thrift/conformance/stresstest/client/ClientFactory.cpp @@ -94,7 +94,9 @@ std::shared_ptr getFizzContext( folly::readFile(cfg.certPath.c_str(), cert); folly::readFile(cfg.keyPath.c_str(), key); auto selfCert = fizz::openssl::CertUtils::makeSelfCert(cert, key); - ctx->setClientCertificate(std::move(selfCert)); + auto certMgr = std::make_shared(); + certMgr->addCert(std::move(selfCert)); + ctx->setClientCertManager(std::move(certMgr)); } return ctx; }(); diff --git a/third-party/thrift/src/thrift/lib/cpp2/test/server/ThriftServerTest.cpp b/third-party/thrift/src/thrift/lib/cpp2/test/server/ThriftServerTest.cpp index 087e015c8dbd3..4e67ff4c77e90 100644 --- a/third-party/thrift/src/thrift/lib/cpp2/test/server/ThriftServerTest.cpp +++ b/third-party/thrift/src/thrift/lib/cpp2/test/server/ThriftServerTest.cpp @@ -3462,12 +3462,13 @@ static std::shared_ptr makeQuicClient( std::string keyData; folly::readFile(find_resource(folly::test::kTestKey).c_str(), keyData); - + auto certMgr = std::make_shared(); if (!certData.empty() && !keyData.empty()) { auto cert = fizz::openssl::CertUtils::makeSelfCert( std::move(certData), std::move(keyData)); - ctx->setClientCertificate(std::move(cert)); + certMgr->addCert(std::move(cert)); } + ctx->setClientCertManager(std::move(certMgr)); } auto quicClient = std::make_shared( diff --git a/third-party/thrift/src/thrift/test/integration/cpp2/zero_copy_client.cpp b/third-party/thrift/src/thrift/test/integration/cpp2/zero_copy_client.cpp index 260d470d21ffb..c153b49fd74a8 100644 --- a/third-party/thrift/src/thrift/test/integration/cpp2/zero_copy_client.cpp +++ b/third-party/thrift/src/thrift/test/integration/cpp2/zero_copy_client.cpp @@ -77,8 +77,10 @@ class Client { const char* keypath = FLAGS_key.size() > 0 ? FLAGS_key.c_str() : FLAGS_cert.c_str(); CHECK(folly::readFile(keypath, key)); - context->setClientCertificate(fizz::openssl::CertUtils::makeSelfCert( + auto certMgr = std::make_shared(); + certMgr->addCert(fizz::openssl::CertUtils::makeSelfCert( std::move(cert), std::move(key))); + context->setClientCertManager(std::move(certMgr)); } auto* fizzClient = new fizz::client::AsyncFizzClient(&evb_, std::move(context));