Commit ea46f07e authored by Mingtao Yang's avatar Mingtao Yang Committed by Facebook GitHub Bot

Remove bad CertificateIdentityVerifier test.

Summary:
The certificate identity verifier is meant to verify the end entity certificate
in a certificate chain. It assumes that OpenSSL has already validated that
the end entity has a valid signature by an issuer.

The test that I am removing is asserting buggy behavior in OpenSSL prior to
version OpenSSL 1.1.1i. The test scenario sets up a client *without a certificate
store* connecting to a server. When the client attempts to validate this
certificate chain, it will receive two errors:

1. Unable to find issuer locally. Because the test server's certificate is signed
   by an issuer that is not present in the client's (empty) certificate store,
   while trying to determine the issuer during chain building, OpenSSL fails as
   expected. Because the test scenario *intentionally* overrides this error by
   returning `true` in `handshakeVer`, OpenSSL continues its certificate
   validation procedure.

2. Unable to verify leaf signature. In order to validate the signature for a
   certificate, one needs to know the public key of the issuer. However, the
   client does not know about the issuer. Control flow would normally not reach
   here, but the client chose to continue the validation process without knowing
   the issuer in step 1. The test scenario forcefully returns "true" here, telling
   OpenSSL to continue.

Prior to OpenSSL 1.1.1i (specifically commit [2e06150e](https://github.com/openssl/openssl/commit/2e06150e3928daa06d5ff70c32bffad8088ebe58)),
OpenSSL would stop the verification procedure here. It would never signal the
completion of the verification procedure (by sending `preverify_ok=1`).

The test was asserting that this was expected. This meant that the test was
asserting behavior on an incorrect implementation. With OpenSSL 1.1.1i, if
all previous `handshakeVer` callbacks say that the certificate is fine, then
OpenSSL will perform the final call with `preverify_ok=1`, which will cause the
CertificateIdentityVerifier to be invoked.

Differential Revision: D25775428

fbshipit-source-id: 8a5f388ce2eb9a8d5667fd4e840b6b223e618d43
parent 0ab97552
...@@ -1461,83 +1461,6 @@ TEST( ...@@ -1461,83 +1461,6 @@ TEST(
serverSock->close(); serverSock->close();
} }
MATCHER_P(
hasError,
error,
"X509_STORE_CTX matcher to check for verification error code") {
return X509_STORE_CTX_get_error(arg) == error;
}
/**
* Verify that the client CertificateIdentityVerifier is not invoked if
* OpenSSL preverify fails but HandshakeCB::handshakeVer succeeds.
*/
TEST(
AsyncSSLSocketTest,
SSLCertificateIdentityVerifierHandshakeCBOverrideOpenSSLResult) {
EventBase eventBase;
auto clientCtx = std::make_shared<folly::SSLContext>();
auto serverCtx = std::make_shared<folly::SSLContext>();
getctx(clientCtx, serverCtx);
// the client socket will default to USE_CTX, so set VERIFY here
clientCtx->setVerificationOption(SSLContext::SSLVerifyPeerEnum::VERIFY);
// DO NOT load root certificate, will cause X509 chain validation error
NetworkSocket fds[2];
getfds(fds);
AsyncSocket::UniquePtr rawClient(new AsyncSocket(&eventBase, fds[0]));
AsyncSocket::UniquePtr rawServer(new AsyncSocket(&eventBase, fds[1]));
// should not be invoked
std::shared_ptr<StrictMock<MockCertificateIdentityVerifier>> verifier =
std::make_shared<StrictMock<MockCertificateIdentityVerifier>>();
AsyncSSLSocket::Options clientOpts;
clientOpts.verifier = verifier;
AsyncSSLSocket::Options serverOpts;
serverOpts.isServer = true;
AsyncSSLSocket::UniquePtr clientSock(new AsyncSSLSocket(
clientCtx, std::move(rawClient), std::move(clientOpts)));
AsyncSSLSocket::UniquePtr serverSock(new AsyncSSLSocket(
serverCtx, std::move(rawServer), std::move(serverOpts)));
serverSock->sslAccept(nullptr, std::chrono::milliseconds::zero());
StrictMock<MockHandshakeCB> clientHandshakeCB;
// first OpenSSL failure
EXPECT_CALL(
clientHandshakeCB,
handshakeVerImpl(
clientSock.get(),
false,
hasError(X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY)))
// overwrite OpenSSL error
.WillOnce(Return(true));
// second OpenSSL failure
EXPECT_CALL(
clientHandshakeCB,
handshakeVerImpl(
clientSock.get(),
false,
hasError(X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE)))
// overwrite OpenSSL error
.WillOnce(Return(true));
// success callback to confirm handshake succeeded
EXPECT_CALL(clientHandshakeCB, handshakeSucImpl(clientSock.get()))
.WillOnce(Return());
clientSock->sslConn(&clientHandshakeCB);
eventBase.loop();
clientSock->close();
serverSock->close();
}
/** /**
* Verify that the client CertificateIdentityVerifier is invoked on a server * Verify that the client CertificateIdentityVerifier is invoked on a server
* socket when peer verification is requested. * socket when peer verification is requested.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment