diff --git a/include/evse_security/crypto/interface/crypto_supplier.hpp b/include/evse_security/crypto/interface/crypto_supplier.hpp index d8119c4..243c40c 100644 --- a/include/evse_security/crypto/interface/crypto_supplier.hpp +++ b/include/evse_security/crypto/interface/crypto_supplier.hpp @@ -57,11 +57,10 @@ class AbstractCryptoSupplier { /// @param dir_path Optional directory path that can be used for certificate store lookup /// @param file_path Optional certificate file path that can be used for certificate store lookup /// @return - static CertificateValidationResult x509_verify_certificate_chain(X509Handle* target, - const std::vector& parents, - bool allow_future_certificates, - const std::optional dir_path, - const std::optional file_path); + static CertificateValidationResult + x509_verify_certificate_chain(X509Handle* target, const std::vector& parents, + const std::vector& untrusted_subcas, bool allow_future_certificates, + const std::optional dir_path, const std::optional file_path); /// @brief Checks if the private key is consistent with the provided handle static KeyValidationResult x509_check_private_key(X509Handle* handle, std::string private_key, diff --git a/include/evse_security/crypto/openssl/openssl_supplier.hpp b/include/evse_security/crypto/openssl/openssl_supplier.hpp index 3c091c3..cc67dc2 100644 --- a/include/evse_security/crypto/openssl/openssl_supplier.hpp +++ b/include/evse_security/crypto/openssl/openssl_supplier.hpp @@ -31,11 +31,10 @@ class OpenSSLSupplier : public AbstractCryptoSupplier { static bool x509_is_child(X509Handle* child, X509Handle* parent); static bool x509_is_equal(X509Handle* a, X509Handle* b); static X509Handle_ptr x509_duplicate_unique(X509Handle* handle); - static CertificateValidationResult x509_verify_certificate_chain(X509Handle* target, - const std::vector& parents, - bool allow_future_certificates, - const std::optional dir_path, - const std::optional file_path); + static CertificateValidationResult + x509_verify_certificate_chain(X509Handle* target, const std::vector& parents, + const std::vector& untrusted_subcas, bool allow_future_certificates, + const std::optional dir_path, const std::optional file_path); static KeyValidationResult x509_check_private_key(X509Handle* handle, std::string private_key, std::optional password); static bool x509_verify_signature(X509Handle* handle, const std::vector& signature, diff --git a/include/evse_security/detail/openssl/openssl_types.hpp b/include/evse_security/detail/openssl/openssl_types.hpp index 6426f03..4e652f6 100644 --- a/include/evse_security/detail/openssl/openssl_types.hpp +++ b/include/evse_security/detail/openssl/openssl_types.hpp @@ -33,6 +33,13 @@ template <> class std::default_delete { } }; +template <> class std::default_delete { +public: + void operator()(STACK_OF(X509) * ptr) const { + sk_X509_free(ptr); + } +}; + template <> class std::default_delete { public: void operator()(EVP_PKEY* ptr) const { @@ -87,6 +94,9 @@ namespace evse_security { using X509_ptr = std::unique_ptr; using X509_STORE_ptr = std::unique_ptr; using X509_STORE_CTX_ptr = std::unique_ptr; +// Unsafe since it does not free contained elements, only the stack, the element +// cleanup has to be done manually +using X509_STACK_UNSAFE_ptr = std::unique_ptr; using X509_REQ_ptr = std::unique_ptr; using EVP_PKEY_ptr = std::unique_ptr; using EVP_PKEY_CTX_ptr = std::unique_ptr; diff --git a/lib/evse_security/crypto/interface/crypto_supplier.cpp b/lib/evse_security/crypto/interface/crypto_supplier.cpp index 3c1dea5..8fb04dc 100644 --- a/lib/evse_security/crypto/interface/crypto_supplier.cpp +++ b/lib/evse_security/crypto/interface/crypto_supplier.cpp @@ -76,8 +76,8 @@ static X509Handle_ptr x509_duplicate_unique(X509Handle* handle) { } CertificateValidationResult AbstractCryptoSupplier::x509_verify_certificate_chain( - X509Handle* target, const std::vector& parents, bool allow_future_certificates, - const std::optional dir_path, const std::optional file_path) { + X509Handle* target, const std::vector& parents, const std::vector& untrusted_subcas, + bool allow_future_certificates, const std::optional dir_path, const std::optional file_path) { default_crypto_supplier_usage_error() return CertificateValidationResult::Unknown; } diff --git a/lib/evse_security/crypto/openssl/openssl_supplier.cpp b/lib/evse_security/crypto/openssl/openssl_supplier.cpp index 543721c..a776947 100644 --- a/lib/evse_security/crypto/openssl/openssl_supplier.cpp +++ b/lib/evse_security/crypto/openssl/openssl_supplier.cpp @@ -560,13 +560,12 @@ X509Handle_ptr OpenSSLSupplier::x509_duplicate_unique(X509Handle* handle) { return std::make_unique(X509_dup(get(handle))); } -CertificateValidationResult OpenSSLSupplier::x509_verify_certificate_chain(X509Handle* target, - const std::vector& parents, - bool allow_future_certificates, - const std::optional dir_path, - const std::optional file_path) { +CertificateValidationResult OpenSSLSupplier::x509_verify_certificate_chain( + X509Handle* target, const std::vector& parents, const std::vector& untrusted_subcas, + bool allow_future_certificates, const std::optional dir_path, const std::optional file_path) { OpenSSLProvider provider; provider.set_global_mode(OpenSSLProvider::mode_t::default_provider); + X509_STORE_ptr store_ptr(X509_STORE_new()); X509_STORE_CTX_ptr store_ctx_ptr(X509_STORE_CTX_new()); @@ -591,7 +590,25 @@ CertificateValidationResult OpenSSLSupplier::x509_verify_certificate_chain(X509H } } - X509_STORE_CTX_init(store_ctx_ptr.get(), store_ptr.get(), get(target), NULL); + X509_STACK_UNSAFE_ptr untrusted = nullptr; + + // Build potentially untrusted intermediary (subca) certificates + if (false == untrusted_subcas.empty()) { + untrusted = X509_STACK_UNSAFE_ptr(sk_X509_new_null()); + int flags = X509_ADD_FLAG_NO_DUP | X509_ADD_FLAG_NO_SS; + + for (auto& untrusted_cert : untrusted_subcas) { + if (1 != X509_add_cert(untrusted.get(), get(untrusted_cert), flags)) { + EVLOG_error << "X509 could not create untrusted store stack!"; + return CertificateValidationResult::Unknown; + } + } + } + + if (1 != X509_STORE_CTX_init(store_ctx_ptr.get(), store_ptr.get(), get(target), untrusted.get())) { + EVLOG_error << "X509 could not init x509 store ctx!"; + return CertificateValidationResult::Unknown; + } if (allow_future_certificates) { // Manually check if cert is expired diff --git a/lib/evse_security/evse_security.cpp b/lib/evse_security/evse_security.cpp index 888d5f1..95818e0 100644 --- a/lib/evse_security/evse_security.cpp +++ b/lib/evse_security/evse_security.cpp @@ -1261,27 +1261,35 @@ CertificateValidationResult EvseSecurity::verify_certificate_internal(const std: try { X509CertificateBundle certificate(certificate_chain, EncodingFormat::PEM); + std::vector _certificate_chain = certificate.split(); if (_certificate_chain.empty()) { return CertificateValidationResult::Unknown; } + // The leaf is to be verified const auto leaf_certificate = _certificate_chain.at(0); - std::vector parent_certificates; // Retrieve the hierarchy in order to check if the chain contains a root certificate X509CertificateHierarchy& hierarchy = certificate.get_certficate_hierarchy(); - // Make sure that an added root certificate is excluded and taken from the bundle - for (size_t i = 1; i < _certificate_chain.size(); i++) { - const auto& cert = _certificate_chain[i]; - if (hierarchy.is_root(cert)) { - EVLOG_warning << "Ignore root certificate: " << cert.get_common_name(); - } else { - parent_certificates.emplace_back(cert.get()); + // Build all untrusted intermediary certificates, and exclude any root + std::vector untrusted_subcas; + + if (_certificate_chain.size() > 1) { + for (size_t i = 1; i < _certificate_chain.size(); i++) { + const auto& cert = _certificate_chain[i]; + if (hierarchy.is_root(cert)) { + EVLOG_warning << "Ignore root certificate: " << cert.get_common_name(); + } else { + untrusted_subcas.emplace_back(cert.get()); + } } } + // Build the trusted parent certificates from our internal store + std::vector trusted_parent_certificates; + fs::path root_store = this->ca_bundle_path_map.at(ca_certificate_type); CertificateValidationResult validated{}; @@ -1296,16 +1304,17 @@ CertificateValidationResult EvseSecurity::verify_certificate_internal(const std: std::vector root_chain{roots.split()}; for (size_t i = 0; i < root_chain.size(); i++) { - parent_certificates.emplace_back(root_chain[i].get()); + trusted_parent_certificates.emplace_back(root_chain[i].get()); } // The root_chain stores the X509Handler pointers, if this goes out of scope then // parent_certificates will point to nothing. - validated = CryptoSupplier::x509_verify_certificate_chain(leaf_certificate.get(), parent_certificates, true, - std::nullopt, std::nullopt); + validated = + CryptoSupplier::x509_verify_certificate_chain(leaf_certificate.get(), trusted_parent_certificates, + untrusted_subcas, true, std::nullopt, std::nullopt); } else { - validated = CryptoSupplier::x509_verify_certificate_chain(leaf_certificate.get(), parent_certificates, true, - std::nullopt, root_store); + validated = CryptoSupplier::x509_verify_certificate_chain( + leaf_certificate.get(), trusted_parent_certificates, untrusted_subcas, true, std::nullopt, root_store); } return validated; diff --git a/tests/openssl_supplier_test.cpp b/tests/openssl_supplier_test.cpp index c8fd659..bd3c537 100644 --- a/tests/openssl_supplier_test.cpp +++ b/tests/openssl_supplier_test.cpp @@ -91,13 +91,14 @@ TEST_F(OpenSSLSupplierTest, x509_verify_certificate_chain) { auto res_leaf = OpenSSLSupplier::load_certificates(cert_leaf, EncodingFormat::PEM); std::vector parents; + std::vector empty_untrusted; for (auto& i : res_path) { parents.push_back(i.get()); } - auto res = OpenSSLSupplier::x509_verify_certificate_chain(res_leaf[0].get(), parents, true, std::nullopt, - "pki/root_cert.pem"); + auto res = OpenSSLSupplier::x509_verify_certificate_chain(res_leaf[0].get(), parents, empty_untrusted, true, + std::nullopt, "pki/root_cert.pem"); ASSERT_EQ(res, CertificateValidationResult::Valid); }