szaszm commented on a change in pull request #947: URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r538274424
########## File path: libminifi/src/utils/tls/ExtendedKeyUsage.cpp ########## @@ -0,0 +1,104 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#ifdef OPENSSL_SUPPORT + +#include "utils/tls/ExtendedKeyUsage.h" + +#include <openssl/x509v3.h> + +#include <array> +#include <cassert> +#include <climits> + +#include "core/logging/LoggerConfiguration.h" +#include "utils/StringUtils.h" + +namespace org { +namespace apache { +namespace nifi { +namespace minifi { +namespace utils { +namespace tls { + +namespace { + +struct KeyValuePair { + const char* key; + uint8_t value; +}; +constexpr std::array<KeyValuePair, 6> EXT_KEY_USAGE_NAME_TO_BIT_POS{{ + KeyValuePair{"Server Authentication", 1}, + KeyValuePair{"Client Authentication", 2}, + KeyValuePair{"Code Signing", 3}, + KeyValuePair{"Secure Email", 4}, + KeyValuePair{"Time Stamping", 8}, + KeyValuePair{"OCSP Signing", 9} +}}; Review comment: A source for these constants would be nice. I've found them in RFC3280 on page 109. ########## File path: libminifi/src/utils/tls/ExtendedKeyUsage.cpp ########## @@ -0,0 +1,104 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#ifdef OPENSSL_SUPPORT + +#include "utils/tls/ExtendedKeyUsage.h" + +#include <openssl/x509v3.h> + +#include <array> +#include <cassert> +#include <climits> + +#include "core/logging/LoggerConfiguration.h" +#include "utils/StringUtils.h" + +namespace org { +namespace apache { +namespace nifi { +namespace minifi { +namespace utils { +namespace tls { + +namespace { + +struct KeyValuePair { + const char* key; + uint8_t value; +}; +constexpr std::array<KeyValuePair, 6> EXT_KEY_USAGE_NAME_TO_BIT_POS{{ + KeyValuePair{"Server Authentication", 1}, + KeyValuePair{"Client Authentication", 2}, + KeyValuePair{"Code Signing", 3}, + KeyValuePair{"Secure Email", 4}, + KeyValuePair{"Time Stamping", 8}, + KeyValuePair{"OCSP Signing", 9} +}}; + +} // namespace + +void EXTENDED_KEY_USAGE_deleter::operator()(EXTENDED_KEY_USAGE* key_usage) const { EXTENDED_KEY_USAGE_free(key_usage); } + +ExtendedKeyUsage::ExtendedKeyUsage() : logger_(core::logging::LoggerFactory<ExtendedKeyUsage>::getLogger()) {} + +ExtendedKeyUsage::ExtendedKeyUsage(const EXTENDED_KEY_USAGE& key_usage_asn1) : ExtendedKeyUsage{} { + const int num_oids = sk_ASN1_OBJECT_num(&key_usage_asn1); + for (int i = 0; i < num_oids; ++i) { + const ASN1_OBJECT* const oid = sk_ASN1_OBJECT_value(&key_usage_asn1, i); + assert(oid && oid->length > 0); + const unsigned char bit_pos = oid->data[oid->length - 1]; Review comment: Could you provide some structure references for mortals like myself to be able to see what makes e.g. `oid->data[oid->length - 1]` be the bit to be set in the extended key usage bitset? ########## 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: Wouldn't using [`X509_get_extended_key_usage`](https://www.openssl.org/docs/man1.1.1/man3/X509_get_extended_key_usage.html) simplify code here? ---------------------------------------------------------------- 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
