patches/wangle-ssl-segfault-if-misconfigured.patch

   1 commit ab7a6f191c1150fb4aa95f325023b88ae4201850
   2 Author: Neel Goyal <ngoyal@fb.com>
   3 Date:   Mon Dec 11 18:48:49 2017 -0800
   4
   5     Fix case where ssl cert does not match key
   6
   7     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.
   8
   9     Reviewed By: xybu
  10
  11     Differential Revision: D6513964
  12
  13 diff --git a/third-party/wangle/src/wangle/ssl/SSLContextManager.cpp b/third-party/wangle/src/wangle/ssl/SSLContextManager.cpp
  14 index c0b7e0d..b4e97c8 100644
  15 --- a/third-party/wangle/src/wangle/ssl/SSLContextManager.cpp
  16 +++ b/third-party/wangle/src/wangle/ssl/SSLContextManager.cpp
  17 @@ -76,7 +76,9 @@ X509* getX509(SSL_CTX* ctx) {
  18    SSL* ssl = SSL_new(ctx);
  19    SSL_set_connect_state(ssl);
  20    X509* x509 = SSL_get_certificate(ssl);
  21 -  X509_up_ref(x509);
  22 +  if (x509) {
  23 +    X509_up_ref(x509);
  24 +  }
  25    SSL_free(ssl);
  26    return x509;
  27  }
  28 @@ -270,21 +272,43 @@ void SSLContextManager::addSSLContextConfig(
  29        std::make_shared<ServerSSLContext>(ctxConfig.sslVersion);
  30    for (const auto& cert : ctxConfig.certificates) {
  31      try {
  32 -      loadCertificate(sslCtx.get(), ctxConfig, cert.certPath);
  33 +      if (ctxConfig.isLocalPrivateKey ||
  34 +          ctxConfig.keyOffloadParams.offloadType.empty()) {
  35 +        // The private key lives in the same process
  36 +        // This needs to be called before loadPrivateKey().
  37 +        if (!cert.passwordPath.empty()) {
  38 +          auto sslPassword = std::make_shared<PasswordInFile>(
  39 +              cert.passwordPath);
  40 +          sslCtx->passwordCollector(std::move(sslPassword));
  41 +        }
  42 +        sslCtx->loadCertKeyPairFromFiles(
  43 +          cert.certPath.c_str(),
  44 +          cert.keyPath.c_str(),
  45 +          "PEM",
  46 +          "PEM");
  47 +      } else {
  48 +        loadCertificate(sslCtx.get(), ctxConfig, cert.certPath);
  49 +        enableAsyncCrypto(sslCtx, ctxConfig, cert.certPath);
  50 +      }
  51      } catch (const std::exception& ex) {
  52 -      // The exception isn't very useful without the certificate path name,
  53 -      // so throw a new exception that includes the path to the certificate.
  54 -      string msg = folly::to<string>("error loading SSL certificate ",
  55 -                                     cert.certPath, ": ",
  56 -                                     folly::exceptionStr(ex));
  57 -      LOG(ERROR) << msg;
  58 -      throw std::runtime_error(msg);
  59 +        // The exception isn't very useful without the certificate path name,
  60 +        // so throw a new exception that includes the path to the certificate.
  61 +        string msg = folly::to<string>("error loading SSL certificate ",
  62 +                                       cert.certPath, ": ",
  63 +                                       folly::exceptionStr(ex));
  64 +        LOG(ERROR) << msg;
  65 +        throw std::runtime_error(msg);
  66      }
  67
  68      // Verify that the Common Name and (if present) Subject Alternative Names
  69      // are the same for all the certs specified for the SSL context.
  70      numCerts++;
  71      X509* x509 = getX509(sslCtx->getSSLCtx());
  72 +    if (!x509) {
  73 +      throw std::runtime_error(
  74 +          folly::to<std::string>(
  75 +              "Certificate: ", cert.certPath, " is invalid"));
  76 +    }
  77      auto guard = folly::makeGuard([x509] { X509_free(x509); });
  78      auto cn = SSLUtil::getCommonName(x509);
  79      if (!cn) {
  80 @@ -323,30 +347,6 @@ void SSLContextManager::addSSLContextConfig(
  81        }
  82      }
  83      lastCertPath = cert.certPath;
  84 -
  85 -    if (ctxConfig.isLocalPrivateKey
  86 -        || ctxConfig.keyOffloadParams.offloadType.empty()) {
  87 -      // The private key lives in the same process
  88 -      // This needs to be called before loadPrivateKey().
  89 -      if (!cert.passwordPath.empty()) {
  90 -        auto sslPassword = std::make_shared<PasswordInFile>(cert.passwordPath);
  91 -        sslCtx->passwordCollector(sslPassword);
  92 -      }
  93 -
  94 -      try {
  95 -        sslCtx->loadPrivateKey(cert.keyPath.c_str());
  96 -      } catch (const std::exception& ex) {
  97 -        // Throw an error that includes the key path, so the user can tell
  98 -        // which key had a problem.
  99 -        string msg = folly::to<string>("error loading private SSL key ",
 100 -                                       cert.keyPath, ": ",
 101 -                                       folly::exceptionStr(ex));
 102 -        LOG(ERROR) << msg;
 103 -        throw std::runtime_error(msg);
 104 -      }
 105 -    } else {
 106 -      enableAsyncCrypto(sslCtx, ctxConfig, cert.certPath);
 107 -    }
 108    }
 109
 110    overrideConfiguration(sslCtx, ctxConfig);
 111 @@ -596,6 +596,9 @@ SSLContextManager::insert(shared_ptr<ServerSSLContext> sslCtx,
 112                            bool defaultFallback,
 113                            SslContexts& contexts) {
 114    X509* x509 = getX509(sslCtx->getSSLCtx());
 115 +  if (!x509) {
 116 +    throw std::runtime_error("SSLCtx is invalid");
 117 +  }
 118    auto guard = folly::makeGuard([x509] { X509_free(x509); });
 119    auto cn = SSLUtil::getCommonName(x509);
 120    if (!cn) {