Skip to content

Commit

Permalink
Refactor uses of setClientCertificate to setClientCertManager
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Abdulkadir Fiqi authored and facebook-github-bot committed Sep 16, 2024
1 parent c8b31f7 commit ddc37dc
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 35 deletions.
3 changes: 3 additions & 0 deletions third-party/fizz/src/fizz/client/CertManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
20 changes: 0 additions & 20 deletions third-party/fizz/src/fizz/client/FizzClientContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion third-party/fizz/src/fizz/client/test/ClientProtocolTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() =
Expand Down
4 changes: 3 additions & 1 deletion third-party/fizz/src/fizz/test/BogoShim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 7 additions & 3 deletions third-party/fizz/src/fizz/test/HandshakeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
}
Expand Down
4 changes: 3 additions & 1 deletion third-party/fizz/src/fizz/test/HandshakeTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 3 additions & 1 deletion third-party/fizz/src/fizz/tool/FizzClientCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<fizz::client::CertManager>();
certMgr->addCert(std::move(cert));
ctx->setClientCertManager(std::move(certMgr));
}
std::shared_ptr<fizz::DefaultCertificateVerifier> verifier;
if (!pemCaPath.empty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ void Client::setupFizzContext(std::shared_ptr<fizz::client::PskCache> 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<fizz::client::CertManager>();
certMgr->addCert(std::move(selfCert));
fizzContext_->setClientCertManager(std::move(certMgr));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<fizz::client::CertManager>();
certMgr->addCert(std::move(cert));
ctx->setClientCertManager(std::move(certMgr));
ctx->setSupportedAlpns(params.supportedAlpns);
ctx->setDefaultShares(
{fizz::NamedGroup::x25519, fizz::NamedGroup::secp256r1});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<fizz::client::CertManager>();
certMgr->addCert(std::move(cert));
ctx->setClientCertManager(std::move(certMgr));
}

std::vector<std::string> supportedAlpns = {proxygen::kH3FBCurrentDraft};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ std::shared_ptr<fizz::client::FizzClientContext> 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<fizz::client::CertManager>();
certMgr->addCert(std::move(selfCert));
ctx->setClientCertManager(std::move(certMgr));
}
return ctx;
}();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3462,12 +3462,13 @@ static std::shared_ptr<quic::QuicClientTransport> makeQuicClient(

std::string keyData;
folly::readFile(find_resource(folly::test::kTestKey).c_str(), keyData);

auto certMgr = std::make_shared<CertManager>();
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<quic::QuicClientTransport>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<fizz::client::CertManager>();
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));
Expand Down

0 comments on commit ddc37dc

Please sign in to comment.