Permalink
Browse files

Fix case where ssl cert does not match key

Summary: In some cases, SSLContextManager seg faults if a cert and key do not match.  This guards against that case when strictSSL = false, and throws a more useful error in the cases when SSL is required.

Reviewed By: xybu

Differential Revision: D6513964

fbshipit-source-id: 8e63a22b346fd3f2a30d558a3659ab6794c7a105
  • Loading branch information...
ngoyal authored and facebook-github-bot committed Dec 12, 2017
1 parent c850775 commit ab7a6f191c1150fb4aa95f325023b88ae4201850
Showing with 36 additions and 33 deletions.
  1. +36 −33 wangle/ssl/SSLContextManager.cpp
@@ -76,7 +76,9 @@ X509* getX509(SSL_CTX* ctx) {
SSL* ssl = SSL_new(ctx);
SSL_set_connect_state(ssl);
X509* x509 = SSL_get_certificate(ssl);
X509_up_ref(x509);
if (x509) {
X509_up_ref(x509);
}
SSL_free(ssl);
return x509;
}
@@ -270,21 +272,43 @@ void SSLContextManager::addSSLContextConfig(
std::make_shared<ServerSSLContext>(ctxConfig.sslVersion);
for (const auto& cert : ctxConfig.certificates) {
try {
loadCertificate(sslCtx.get(), ctxConfig, cert.certPath);
if (ctxConfig.isLocalPrivateKey ||
ctxConfig.keyOffloadParams.offloadType.empty()) {
// The private key lives in the same process
// This needs to be called before loadPrivateKey().
if (!cert.passwordPath.empty()) {
auto sslPassword = std::make_shared<PasswordInFile>(
cert.passwordPath);
sslCtx->passwordCollector(std::move(sslPassword));
}
sslCtx->loadCertKeyPairFromFiles(
cert.certPath.c_str(),
cert.keyPath.c_str(),
"PEM",
"PEM");
} else {
loadCertificate(sslCtx.get(), ctxConfig, cert.certPath);
enableAsyncCrypto(sslCtx, ctxConfig, cert.certPath);
}
} catch (const std::exception& ex) {
// The exception isn't very useful without the certificate path name,
// so throw a new exception that includes the path to the certificate.
string msg = folly::to<string>("error loading SSL certificate ",
cert.certPath, ": ",
folly::exceptionStr(ex));
LOG(ERROR) << msg;
throw std::runtime_error(msg);
// The exception isn't very useful without the certificate path name,
// so throw a new exception that includes the path to the certificate.
string msg = folly::to<string>("error loading SSL certificate ",
cert.certPath, ": ",
folly::exceptionStr(ex));
LOG(ERROR) << msg;
throw std::runtime_error(msg);
}
// Verify that the Common Name and (if present) Subject Alternative Names
// are the same for all the certs specified for the SSL context.
numCerts++;
X509* x509 = getX509(sslCtx->getSSLCtx());
if (!x509) {
throw std::runtime_error(
folly::to<std::string>(
"Certificate: ", cert.certPath, " is invalid"));
}
auto guard = folly::makeGuard([x509] { X509_free(x509); });
auto cn = SSLUtil::getCommonName(x509);
if (!cn) {
@@ -323,30 +347,6 @@ void SSLContextManager::addSSLContextConfig(
}
}
lastCertPath = cert.certPath;
if (ctxConfig.isLocalPrivateKey
|| ctxConfig.keyOffloadParams.offloadType.empty()) {
// The private key lives in the same process
// This needs to be called before loadPrivateKey().
if (!cert.passwordPath.empty()) {
auto sslPassword = std::make_shared<PasswordInFile>(cert.passwordPath);
sslCtx->passwordCollector(sslPassword);
}
try {
sslCtx->loadPrivateKey(cert.keyPath.c_str());
} catch (const std::exception& ex) {
// Throw an error that includes the key path, so the user can tell
// which key had a problem.
string msg = folly::to<string>("error loading private SSL key ",
cert.keyPath, ": ",
folly::exceptionStr(ex));
LOG(ERROR) << msg;
throw std::runtime_error(msg);
}
} else {
enableAsyncCrypto(sslCtx, ctxConfig, cert.certPath);
}
}
overrideConfiguration(sslCtx, ctxConfig);
@@ -596,6 +596,9 @@ SSLContextManager::insert(shared_ptr<ServerSSLContext> sslCtx,
bool defaultFallback,
SslContexts& contexts) {
X509* x509 = getX509(sslCtx->getSSLCtx());
if (!x509) {
throw std::runtime_error("SSLCtx is invalid");
}
auto guard = folly::makeGuard([x509] { X509_free(x509); });
auto cn = SSLUtil::getCommonName(x509);
if (!cn) {

0 comments on commit ab7a6f1

Please sign in to comment.