Skip to content

Commit

Permalink
Support using a cert manager to select certs
Browse files Browse the repository at this point in the history
Summary: Use the client cert manager to select a cert from the context.

Reviewed By: mingtaoy

Differential Revision: D56315918

fbshipit-source-id: 88d13ebaf2d8f2bfaa1da01c9d92edd3d92f31d7
  • Loading branch information
Ajanthan Asogamoorthy authored and facebook-github-bot committed May 8, 2024
1 parent 380a477 commit 263bada
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 57 deletions.
1 change: 1 addition & 0 deletions fizz/client/BUCK
Expand Up @@ -12,6 +12,7 @@ cpp_library(
"//fizz/backend:openssl",
],
exported_deps = [
":cert_manager",
":ech_policy",
":psk_cache",
"//fizz/compression:cert_decompression_manager",
Expand Down
60 changes: 29 additions & 31 deletions fizz/client/ClientProtocol.cpp
Expand Up @@ -1814,36 +1814,27 @@ Actions EventHandler<
}
}

static std::tuple<
folly::Optional<SignatureScheme>,
std::shared_ptr<const SelfCert>>
getClientCert(const State& state, const std::vector<SignatureScheme>& schemes) {
folly::Optional<SignatureScheme> selectedScheme;
auto clientCert = state.context()->getClientCertificate();
const auto& supportedSchemes = state.context()->getSupportedSigSchemes();

if (clientCert) {
const auto certSchemes = clientCert->getSigSchemes();
for (const auto& scheme : supportedSchemes) {
if (std::find(certSchemes.begin(), certSchemes.end(), scheme) !=
certSchemes.end() &&
std::find(schemes.begin(), schemes.end(), scheme) != schemes.end()) {
selectedScheme = scheme;
break;
}
}

if (!selectedScheme) {
VLOG(1) << "client cert/context doesn't support any signature algorithms "
<< "specified by the server";
}
static folly::Optional<
std::pair<std::shared_ptr<const SelfCert>, SignatureScheme>>
getClientCert(
const State& state,
const std::vector<SignatureScheme>& schemes,
const std::vector<Extension>& peerExtensions) {
auto certManager = state.context()->getCertManager();
if (certManager == nullptr) {
return folly::none;
}

if (!selectedScheme) {
clientCert = nullptr;
const auto& supportedSchemes = state.context()->getSupportedSigSchemes();
auto result = certManager->getCert(
state.sni(), supportedSchemes, schemes, peerExtensions);
if (!result) {
VLOG(1) << "client cert/context doesn't support any signature algorithms "
<< "specified by the server";
return folly::none;
}

return std::make_tuple(std::move(selectedScheme), std::move(clientCert));
std::shared_ptr<const SelfCert> cert = result->cert;
return std::make_pair(cert, result->scheme);
}

Actions EventHandler<
Expand Down Expand Up @@ -1873,12 +1864,19 @@ Actions EventHandler<
AlertDescription::illegal_parameter);
}

auto certAndScheme = getClientCert(
state,
sigAlgsExtension->supported_signature_algorithms,
certRequest.extensions);

folly::Optional<SignatureScheme> scheme;
std::shared_ptr<const SelfCert> cert;
std::tie(scheme, cert) =
getClientCert(state, sigAlgsExtension->supported_signature_algorithms);
ClientAuthType authType =
scheme ? ClientAuthType::Sent : ClientAuthType::RequestedNoMatch;
ClientAuthType authType = ClientAuthType::RequestedNoMatch;

if (certAndScheme.hasValue()) {
std::tie(cert, scheme) = *certAndScheme;
authType = ClientAuthType::Sent;
}

MutateState mutateState([scheme = std::move(scheme),
cert = std::move(cert),
Expand Down
54 changes: 48 additions & 6 deletions fizz/client/FizzClientContext.h
Expand Up @@ -8,6 +8,7 @@

#pragma once

#include <fizz/client/CertManager.h>
#include <fizz/client/ECHPolicy.h>
#include <fizz/client/PskCache.h>
#include <fizz/compression/CertDecompressionManager.h>
Expand Down Expand Up @@ -110,14 +111,53 @@ class FizzClientContext {
}

/**
* Sets the certificate to use if the server requests client authentication
* 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.
*/
void setClientCertificate(std::shared_ptr<const SelfCert> cert) {
clientCert_ = std::move(cert);
[[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->addCert(std::move(cert));
certManager_ = std::move(certMgr);
} else {
certManager_ = nullptr;
}
}

/*
* Sets the certificate manager to select a cert if the server requests client
* auth
*/
void setClientCertManager(std::shared_ptr<CertManager> manager) {
certManager_ = std::move(manager);
}

const auto& getClientCertificate() const {
return clientCert_;
std::shared_ptr<CertManager> getCertManager() const {
return certManager_;
}

/*
* Retrieves the default client cert used for the ctx. This is a legacy api,
* do not use.
*/
[[deprecated(
"FizzClientContext will no longer allow access to the client certificate. The application is responsible for keeping track of the client certificate installed")]]
std::shared_ptr<SelfCert> getClientCertificate() const {
if (clientCert_) {
return clientCert_;
}
if (certManager_) {
auto result = certManager_->getCert(
folly::none, supportedSigSchemes_, supportedSigSchemes_, {});
return result ? result->cert : nullptr;
}
return nullptr;
}

void setECHPolicy(std::shared_ptr<ECHPolicy> echPolicy) {
Expand Down Expand Up @@ -330,7 +370,9 @@ class FizzClientContext {

std::shared_ptr<ECHPolicy> echPolicy_;
std::shared_ptr<PskCache> pskCache_;
std::shared_ptr<const SelfCert> clientCert_;
// Legacy to support non cert mgr api.
std::shared_ptr<SelfCert> clientCert_{nullptr};
std::shared_ptr<CertManager> certManager_{nullptr};
std::shared_ptr<CertDecompressionManager> certDecompressionManager_;
std::shared_ptr<Clock> clock_;

Expand Down
38 changes: 21 additions & 17 deletions fizz/client/test/ClientProtocolTest.cpp
Expand Up @@ -4166,6 +4166,12 @@ TEST_F(ClientProtocolTest, TestCertificateRequestDuplicated) {
}

TEST_F(ClientProtocolTest, TestCertificateRequestAlgosMismatch) {
// Note we must call this before setting up the cert req request, since our
// cert manager will store certs based on sig schemes
EXPECT_CALL(*mockClientCert_, getSigSchemes())
.WillOnce(Return(
std::vector<SignatureScheme>(1, SignatureScheme::rsa_pss_sha256)));

setupExpectingCertificateRequest();
auto certificateRequest = TestMessages::certificateRequest();

Expand All @@ -4176,10 +4182,6 @@ TEST_F(ClientProtocolTest, TestCertificateRequestAlgosMismatch) {
certificateRequest.extensions.emplace_back(
encodeExtension(std::move(sigAlgs)));

EXPECT_CALL(*mockClientCert_, getSigSchemes())
.WillOnce(Return(
std::vector<SignatureScheme>(1, SignatureScheme::rsa_pss_sha256)));

auto actions = detail::processEvent(state_, std::move(certificateRequest));
expectActions<MutateState>(actions);
processStateMutations(actions);
Expand All @@ -4189,6 +4191,12 @@ TEST_F(ClientProtocolTest, TestCertificateRequestAlgosMismatch) {
}

TEST_F(ClientProtocolTest, TestCertificateRequestContextAlgosUnsupported) {
// Note we must call this before setting up the cert req request, since our
// cert manager will store certs based on sig schemes
EXPECT_CALL(*mockClientCert_, getSigSchemes())
.WillOnce(Return(
std::vector<SignatureScheme>(1, SignatureScheme::rsa_pss_sha256)));

setupExpectingCertificateRequest();
context_->setSupportedSigSchemes({SignatureScheme::rsa_pss_sha512});
auto certificateRequest = TestMessages::certificateRequest();
Expand All @@ -4200,10 +4208,6 @@ TEST_F(ClientProtocolTest, TestCertificateRequestContextAlgosUnsupported) {
certificateRequest.extensions.emplace_back(
encodeExtension(std::move(sigAlgs)));

EXPECT_CALL(*mockClientCert_, getSigSchemes())
.WillOnce(Return(
std::vector<SignatureScheme>(1, SignatureScheme::rsa_pss_sha256)));

auto actions = detail::processEvent(state_, std::move(certificateRequest));
expectActions<MutateState>(actions);
processStateMutations(actions);
Expand All @@ -4213,6 +4217,12 @@ TEST_F(ClientProtocolTest, TestCertificateRequestContextAlgosUnsupported) {
}

TEST_F(ClientProtocolTest, TestCertificateRequestPrefersContextOrder) {
EXPECT_CALL(*mockClientCert_, getSigSchemes())
.WillOnce(Return(std::vector<SignatureScheme>(
{SignatureScheme::ed25519,
SignatureScheme::ecdsa_secp521r1_sha512,
SignatureScheme::rsa_pss_sha512})));

setupExpectingCertificateRequest();
context_->setSupportedSigSchemes(
{SignatureScheme::rsa_pss_sha512,
Expand All @@ -4228,12 +4238,6 @@ TEST_F(ClientProtocolTest, TestCertificateRequestPrefersContextOrder) {
certificateRequest.extensions.emplace_back(
encodeExtension(std::move(requestAlgos)));

EXPECT_CALL(*mockClientCert_, getSigSchemes())
.WillOnce(Return(std::vector<SignatureScheme>(
{SignatureScheme::ed25519,
SignatureScheme::ecdsa_secp521r1_sha512,
SignatureScheme::rsa_pss_sha512})));

auto actions = detail::processEvent(state_, std::move(certificateRequest));
expectActions<MutateState>(actions);
processStateMutations(actions);
Expand All @@ -4243,13 +4247,13 @@ TEST_F(ClientProtocolTest, TestCertificateRequestPrefersContextOrder) {
}

TEST_F(ClientProtocolTest, TestCertificateRequestMatch) {
setupExpectingCertificateRequest();
auto certificateRequest = TestMessages::certificateRequest();

EXPECT_CALL(*mockClientCert_, getSigSchemes())
.WillOnce(Return(
std::vector<SignatureScheme>(1, SignatureScheme::rsa_pss_sha256)));

setupExpectingCertificateRequest();
auto certificateRequest = TestMessages::certificateRequest();

auto actions = detail::processEvent(state_, std::move(certificateRequest));
expectActions<MutateState>(actions);
processStateMutations(actions);
Expand Down
2 changes: 1 addition & 1 deletion fizz/experimental/ktls/test/AsyncFizzBaseKTLSTest.cpp
Expand Up @@ -231,7 +231,7 @@ makeTestServerContext() {
true);

auto factory = std::make_shared<fizz::test::MockFactory>();
auto certManager = std::make_shared<CertManager>();
auto certManager = std::make_shared<server::CertManager>();
auto ticketCipher = std::make_shared<
Aead128GCMTicketCipher<TicketCodec<CertificateStorage::X509>>>(
std::move(factory), std::move(certManager));
Expand Down
2 changes: 1 addition & 1 deletion fizz/test/BogoShim.cpp
Expand Up @@ -305,7 +305,7 @@ std::unique_ptr<SelfCert> readSelfCert() {
}

int serverTest() {
auto certManager = std::make_shared<CertManager>();
auto certManager = std::make_shared<server::CertManager>();
certManager->addCert(readSelfCert(), true);

auto serverContext = std::make_shared<FizzServerContext>();
Expand Down
2 changes: 1 addition & 1 deletion fizz/test/HandshakeTest.h
Expand Up @@ -65,7 +65,7 @@ class HandshakeTest : public Test {
auto pskCache = std::make_shared<BasicPskCache>();
clientContext_->setPskCache(std::move(pskCache));

auto certManager = std::make_shared<CertManager>();
auto certManager = std::make_shared<server::CertManager>();
std::vector<std::shared_ptr<CertificateCompressor>> compressors = {
std::make_shared<ZlibCertificateCompressor>(9)};
std::vector<ssl::X509UniquePtr> rsaCerts;
Expand Down

0 comments on commit 263bada

Please sign in to comment.