szaszm commented on a change in pull request #947: URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r538636172
########## File path: libminifi/src/controllers/SSLContextService.cpp ########## @@ -128,16 +149,239 @@ bool SSLContextService::configure_ssl_context(SSL_CTX *ctx) { } SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, nullptr); - int retp = SSL_CTX_load_verify_locations(ctx, ca_certificate_.c_str(), 0); - if (retp == 0) { - logging::LOG_ERROR(logger_) << "Can not load CA certificate, Exiting, " << getLatestOpenSSLErrorString(); + if (!IsNullOrEmpty(ca_certificate_)) { + if (SSL_CTX_load_verify_locations(ctx, ca_certificate_.c_str(), 0) == 0) { + logging::LOG_ERROR(logger_) << "Cannot load CA certificate, exiting, " << getLatestOpenSSLErrorString(); + return false; + } + } + + if (use_system_cert_store_ && IsNullOrEmpty(certificate_)) { + if (!addClientCertificateFromSystemStoreToSSLContext(ctx)) { + return false; + } + } + + if (use_system_cert_store_ && IsNullOrEmpty(ca_certificate_)) { + if (!addServerCertificatesFromSystemStoreToSSLContext(ctx)) { + return false; + } + } + + return true; +} + +bool SSLContextService::addP12CertificateToSSLContext(SSL_CTX* ctx) const { + const auto fp_deleter = [](BIO* ptr) { BIO_free(ptr); }; + std::unique_ptr<BIO, decltype(fp_deleter)> fp(BIO_new(BIO_s_file()), fp_deleter); + if (fp == nullptr) { + logging::LOG_ERROR(logger_) << "Failed create new file BIO, " << getLatestOpenSSLErrorString(); + return false; + } + if (BIO_read_filename(fp.get(), certificate_.c_str()) <= 0) { + logging::LOG_ERROR(logger_) << "Failed to read certificate file " << certificate_ << ", " << getLatestOpenSSLErrorString(); + return false; + } + const auto p12_deleter = [](PKCS12* ptr) { PKCS12_free(ptr); }; + std::unique_ptr<PKCS12, decltype(p12_deleter)> p12(d2i_PKCS12_bio(fp.get(), nullptr), p12_deleter); + if (p12 == nullptr) { + logging::LOG_ERROR(logger_) << "Failed to DER decode certificate file " << certificate_ << ", " << getLatestOpenSSLErrorString(); + return false; + } + + EVP_PKEY* pkey = nullptr; + X509* cert = nullptr; + STACK_OF(X509)* ca = nullptr; + if (!PKCS12_parse(p12.get(), passphrase_.c_str(), &pkey, &cert, &ca)) { + logging::LOG_ERROR(logger_) << "Failed to parse certificate file " << certificate_ << " as PKCS#12, " << getLatestOpenSSLErrorString(); + return false; + } + utils::tls::EVP_PKEY_unique_ptr pkey_ptr{pkey}; + utils::tls::X509_unique_ptr cert_ptr{cert}; + const auto ca_deleter = gsl::finally([ca] { sk_X509_pop_free(ca, X509_free); }); + + if (SSL_CTX_use_certificate(ctx, cert) != 1) { + logging::LOG_ERROR(logger_) << "Failed to set certificate from " << certificate_ << ", " << getLatestOpenSSLErrorString(); + return false; + } + while (ca != nullptr && sk_X509_num(ca) > 0) { + utils::tls::X509_unique_ptr cacert{sk_X509_pop(ca)}; + if (SSL_CTX_add_extra_chain_cert(ctx, cacert.get()) != 1) { + logging::LOG_ERROR(logger_) << "Failed to set additional certificate from " << certificate_ << ", " << getLatestOpenSSLErrorString(); + return false; + } + cacert.release(); // a successful SSL_CTX_add_extra_chain_cert() takes ownership of cacert + } + if (SSL_CTX_use_PrivateKey(ctx, pkey) != 1) { + logging::LOG_ERROR(logger_) << "Failed to set private key from " << certificate_ << ", " << getLatestOpenSSLErrorString(); return false; } return true; } -#endif + +bool SSLContextService::addPemCertificateToSSLContext(SSL_CTX* ctx) const { + if (SSL_CTX_use_certificate_chain_file(ctx, certificate_.c_str()) <= 0) { + logging::LOG_ERROR(logger_) << "Could not load client certificate " << certificate_ << ", " << getLatestOpenSSLErrorString(); + return false; + } + + if (!IsNullOrEmpty(passphrase_)) { + void* passphrase = const_cast<std::string*>(&passphrase_); + SSL_CTX_set_default_passwd_cb_userdata(ctx, passphrase); + SSL_CTX_set_default_passwd_cb(ctx, minifi::utils::tls::pemPassWordCb); + } + + if (!IsNullOrEmpty(private_key_)) { + int retp = SSL_CTX_use_PrivateKey_file(ctx, private_key_.c_str(), SSL_FILETYPE_PEM); + if (retp != 1) { + logging::LOG_ERROR(logger_) << "Could not load private key, " << retp << " on " << private_key_ << ", " << getLatestOpenSSLErrorString(); + return false; + } + } + + return true; +} + +bool SSLContextService::addClientCertificateFromSystemStoreToSSLContext(SSL_CTX* ctx) const { +#ifdef WIN32 + utils::tls::WindowsCertStoreLocation store_location{cert_store_location_}; + HCERTSTORE hCertStore = CertOpenStore(CERT_STORE_PROV_SYSTEM_A, 0, NULL, + CERT_STORE_OPEN_EXISTING_FLAG | CERT_STORE_READONLY_FLAG | store_location.getBitfieldValue(), + client_cert_store_.data()); + if (!hCertStore) { + logger_->log_error("Could not open system certificate store %s/%s (client certificates)", cert_store_location_, client_cert_store_); + return false; + } + const auto store_close = gsl::finally([hCertStore](){ CertCloseStore(hCertStore, 0); }); + + logger_->log_debug("Looking for client certificate in sytem store %s/%s", cert_store_location_, client_cert_store_); + + PCCERT_CONTEXT pCertContext = nullptr; + while (pCertContext = CertEnumCertificatesInStore(hCertStore, pCertContext)) { + bool certificateIsAcceptableAndWasSuccessfullyAddedToSSLContext = useClientCertificate(ctx, pCertContext); + if (certificateIsAcceptableAndWasSuccessfullyAddedToSSLContext) { + CertFreeCertificateContext(pCertContext); + return true; + } + } + + logger_->log_error("Could not find any suitable client certificate in sytem store %s/%s", cert_store_location_, client_cert_store_); + return false; +#else + logger_->log_error("Getting client certificate from the system store is only supported on Windows"); + return false; +#endif // WIN32 +} + +#ifdef WIN32 +bool SSLContextService::useClientCertificate(SSL_CTX* ctx, PCCERT_CONTEXT certificate) const { + utils::tls::X509_unique_ptr x509_cert = utils::tls::convertWindowsCertificate(certificate); + if (!x509_cert) { + logger_->log_error("Failed to convert system store client certificate to X.509 format"); + return false; + } + + utils::tls::EVP_PKEY_unique_ptr private_key = utils::tls::extractPrivateKey(certificate); + if (!private_key) { + logger_->log_debug("Skipping client certificate %s because it has no exportable private key", x509_cert->name); + return false; + } + + if (!client_cert_cn_.empty()) { + utils::tls::DistinguishedName dn = utils::tls::DistinguishedName::fromSlashSeparated(x509_cert->name); + utils::optional<std::string> cn = dn.getCN(); + if (!cn || *cn != client_cert_cn_) { + logger_->log_debug("Skipping client certificate %s because it doesn't match CN=%s", x509_cert->name, client_cert_cn_); + return false; + } + } + + utils::tls::EXTENDED_KEY_USAGE_unique_ptr key_usage{static_cast<EXTENDED_KEY_USAGE*>(X509_get_ext_d2i(x509_cert.get(), NID_ext_key_usage, nullptr, nullptr))}; + if (!key_usage) { + logger_->log_error("Skipping client certificate %s because it has no extended key usage", x509_cert->name); + return false; + } + + if (!(client_cert_key_usage_.isSubsetOf(utils::tls::ExtendedKeyUsage{*key_usage}))) { + logger_->log_debug("Skipping client certificate %s because its extended key usage set does not contain all usages specified in %s", + x509_cert->name, Configuration::nifi_security_windows_client_cert_key_usage); + return false; + } Review comment: Oh, that explains it, thanks. The switching sounds like a mid to longterm objective, so I'm marking this resolved now. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org