Skip to content

Commit

Permalink
Translate several OpenSSL verify error codes to unknown_ca
Browse files Browse the repository at this point in the history
Summary:
This mirrors the TLS alerting behavior of OpenSSL for conditions that deal with
building a path to a trust anchor.

Reviewed By: NickR23

Differential Revision: D68844979

fbshipit-source-id: 113f3aa46ec7a03255a3fee2ec2a2adaffbae9e7
  • Loading branch information
Mingtao Yang authored and facebook-github-bot committed Feb 3, 2025
1 parent fd6b124 commit a9ffb4e
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
12 changes: 12 additions & 0 deletions fizz/protocol/DefaultCertificateVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ static AlertDescription toTLSAlert(int opensslVerifyErr) {
* "A certificate has expired or is not currently valid."
*/
return AlertDescription::certificate_expired;
case X509_V_ERR_CERT_CHAIN_TOO_LONG:
case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT:
case X509_V_ERR_PATH_LENGTH_EXCEEDED:
case X509_V_ERR_INVALID_CA:
case X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN:
case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT:
case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY:
/**
* All of these conditions indicate that building a path to a trusted
* anchor failed.
*/
return AlertDescription::unknown_ca;
default:
return AlertDescription::bad_certificate;
}
Expand Down
15 changes: 9 additions & 6 deletions fizz/protocol/test/DefaultCertificateVerifierTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ TEST_F(DefaultCertificateVerifierTest, TestVerifySelfSignedCert) {
auto selfsigned = createCert("self", false, nullptr);
expectThrowWithAlert(
[&] { verifier_->verify({getPeerCert(selfsigned)}); },
AlertDescription::bad_certificate);
AlertDescription::unknown_ca);
}

TEST_F(DefaultCertificateVerifierTest, TestVerifySelfSignedCertWithOverride) {
Expand All @@ -114,7 +114,7 @@ TEST_F(DefaultCertificateVerifierTest, TestVerifyWithIntermediateMissing) {
auto subleaf = createCert("subleaf", false, &subauth);
expectThrowWithAlert(
[&] { verifier_->verify({getPeerCert(subleaf)}); },
AlertDescription::bad_certificate);
AlertDescription::unknown_ca);
}

TEST_F(
Expand All @@ -124,18 +124,21 @@ TEST_F(
auto subleaf = createCert("subleaf", false, &subauth);
verifier_->setCustomVerifyCallback(
&DefaultCertificateVerifierTest::allowSelfSignedLeafCertCallback);
// The override is irrelevant to the error here. So exception is expected.
// The override asserts that self signed certs (chain length = 1) are
// accepted. But since this certificate is not self signed, this override
// should effectively be a no-op and normal certificate verification
// should take place.
expectThrowWithAlert(
[&] { verifier_->verify({getPeerCert(subleaf)}); },
AlertDescription::bad_certificate);
AlertDescription::unknown_ca);
}

TEST_F(DefaultCertificateVerifierTest, TestVerifyWithBadIntermediate) {
auto subauth = createCert("badsubauth", false, &rootCertAndKey_);
auto subleaf = createCert("badsubleaf", false, &subauth);
expectThrowWithAlert(
[&] { verifier_->verify({getPeerCert(subleaf)}); },
AlertDescription::bad_certificate);
AlertDescription::unknown_ca);
}

TEST_F(DefaultCertificateVerifierTest, TestVerifyWithBadRoot) {
Expand All @@ -144,7 +147,7 @@ TEST_F(DefaultCertificateVerifierTest, TestVerifyWithBadRoot) {
auto subleaf = createCert("leaf2", false, &subauth);
expectThrowWithAlert(
[&] { verifier_->verify({getPeerCert(subleaf), getPeerCert(subauth)}); },
AlertDescription::bad_certificate);
AlertDescription::unknown_ca);
}
TEST_F(DefaultCertificateVerifierTest, TestVerifyWithExpiredLeafTooOld) {
auto now = std::chrono::system_clock::now();
Expand Down
6 changes: 5 additions & 1 deletion fizz/test/HandshakeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,10 @@ TEST_F(HandshakeTest, CertRequestPermitNoCert) {
}

TEST_F(HandshakeTest, CertRequestBadCert) {
/**
* A server requires client authentication, and the client presents a
* self signed certificate to the server.
*/
serverContext_->setClientAuthMode(ClientAuthMode::Required);
auto badCert = createCert("foo", false, nullptr);
std::vector<folly::ssl::X509UniquePtr> certVec;
Expand All @@ -425,7 +429,7 @@ TEST_F(HandshakeTest, CertRequestBadCert) {
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");
expectServerError("alert: unknown_ca", "client certificate failure");
doHandshake();
}

Expand Down

0 comments on commit a9ffb4e

Please sign in to comment.