From 57ba46d6bf3429aa6aab27a42488806bc3500c91 Mon Sep 17 00:00:00 2001 From: AssemblyJohn Date: Fri, 26 Apr 2024 15:05:19 +0300 Subject: [PATCH] Fixes for OCSP data cleanup and test updates Signed-off-by: AssemblyJohn --- include/evse_security/evse_security.hpp | 6 +- include/evse_security/evse_types.hpp | 19 +- lib/evse_security/certificate/x509_bundle.cpp | 15 + lib/evse_security/evse_security.cpp | 260 +++++++++++------- lib/evse_security/evse_types.cpp | 2 +- tests/tests.cpp | 79 +++++- 6 files changed, 269 insertions(+), 112 deletions(-) diff --git a/include/evse_security/evse_security.hpp b/include/evse_security/evse_security.hpp index d3f797a..ad87902 100644 --- a/include/evse_security/evse_security.hpp +++ b/include/evse_security/evse_security.hpp @@ -142,10 +142,11 @@ class EvseSecurity { /// @param ocsp_response the actual OCSP data void update_ocsp_cache(const CertificateHashData& certificate_hash_data, const std::string& ocsp_response); + // TODO: Switch to path /// @brief Retrieves from the OCSP cache for the given \p certificate_hash_data /// @param certificate_hash_data identifies the certificate for which the \p ocsp_response is specified /// @return the actual OCSP data or an empty value - std::optional retrieve_ocsp_cache(const CertificateHashData& certificate_hash_data); + std::optional retrieve_ocsp_cache(const CertificateHashData& certificate_hash_data); /// @brief Indicates if a CA certificate for the given \p certificate_type is installed on the filesystem /// Supports both CA certificate bundles and directories @@ -247,7 +248,7 @@ class EvseSecurity { LeafCertificateType certificate_type); GetCertificateInfoResult get_leaf_certificate_info_internal(LeafCertificateType certificate_type, EncodingFormat encoding, bool include_ocsp = false); - std::optional retrieve_ocsp_cache_internal(const CertificateHashData& certificate_hash_data); + std::optional retrieve_ocsp_cache_internal(const CertificateHashData& certificate_hash_data); bool is_ca_certificate_installed_internal(CaCertificateType certificate_type); /// @brief Determines if the total filesize of certificates is > than the max_filesystem_usage bytes @@ -287,6 +288,7 @@ class EvseSecurity { FRIEND_TEST(EvseSecurityTests, verify_full_filesystem_install_reject); FRIEND_TEST(EvseSecurityTests, verify_full_filesystem); FRIEND_TEST(EvseSecurityTests, verify_expired_csr_deletion); + FRIEND_TEST(EvseSecurityTests, verify_ocsp_garbage_collect); FRIEND_TEST(EvseSecurityTestsExpired, verify_expired_leaf_deletion); #endif }; diff --git a/include/evse_security/evse_types.hpp b/include/evse_security/evse_types.hpp index dcc28ab..e06cbca 100644 --- a/include/evse_security/evse_types.hpp +++ b/include/evse_security/evse_types.hpp @@ -10,6 +10,12 @@ namespace evse_security { +const fs::path PEM_EXTENSION = ".pem"; +const fs::path DER_EXTENSION = ".der"; +const fs::path KEY_EXTENSION = ".key"; +const fs::path TPM_KEY_EXTENSION = ".tkey"; +const fs::path CERT_HASH_EXTENSION = ".hash"; + enum class EncodingFormat { DER, PEM, @@ -125,8 +131,8 @@ struct OCSPRequestDataList { }; struct CertificateOCSP { - CertificateHashData hash; - std::optional oscsp_data; + CertificateHashData hash; ///< Hash of the certificate for which the OCSP data is held + std::optional ocsp_path; ///< Path to the file in which the certificate OCSP data is held }; struct CertificateInfo { @@ -142,13 +148,6 @@ struct GetCertificateInfoResult { GetCertificateInfoStatus status; std::optional info; }; - -const fs::path PEM_EXTENSION = ".pem"; -const fs::path DER_EXTENSION = ".der"; -const fs::path KEY_EXTENSION = ".key"; -const fs::path TPM_KEY_EXTENSION = ".tkey"; -const fs::path CERT_HASH_EXTENSION = ".hash"; - namespace conversions { std::string encoding_format_to_string(EncodingFormat e); std::string ca_certificate_type_to_string(CaCertificateType e); @@ -156,7 +155,7 @@ std::string leaf_certificate_type_to_string(LeafCertificateType e); std::string leaf_certificate_type_to_filename(LeafCertificateType e); std::string certificate_type_to_string(CertificateType e); std::string hash_algorithm_to_string(HashAlgorithm e); -HashAlgorithm string_to_hash_algorithm(std::string s); +HashAlgorithm string_to_hash_algorithm(const std::string& s); std::string install_certificate_result_to_string(InstallCertificateResult e); std::string delete_certificate_result_to_string(DeleteCertificateResult e); std::string get_installed_certificates_status_to_string(GetInstalledCertificatesStatus e); diff --git a/lib/evse_security/certificate/x509_bundle.cpp b/lib/evse_security/certificate/x509_bundle.cpp index 0889812..1e8a43c 100644 --- a/lib/evse_security/certificate/x509_bundle.cpp +++ b/lib/evse_security/certificate/x509_bundle.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -44,6 +45,20 @@ X509CertificateBundle::X509CertificateBundle(const fs::path& path, const Encodin hierarchy_invalidated(true) { this->path = path; + // In case the path is missing, create it + if (fs::exists(path) == false) { + if (path.has_extension()) { + if (path.extension() == PEM_EXTENSION) { + // Create file if we have an PEM extension + std::ofstream new_file(path.c_str()); + new_file.close(); + } + } else { + // Else create a directory + fs::create_directories(path); + } + } + if (fs::is_directory(path)) { source = X509CertificateSource::DIRECTORY; diff --git a/lib/evse_security/evse_security.cpp b/lib/evse_security/evse_security.cpp index dcb5b7f..4345339 100644 --- a/lib/evse_security/evse_security.cpp +++ b/lib/evse_security/evse_security.cpp @@ -865,14 +865,13 @@ void EvseSecurity::update_ocsp_cache(const CertificateHashData& certificate_hash } } -std::optional EvseSecurity::retrieve_ocsp_cache(const CertificateHashData& certificate_hash_data) { +std::optional EvseSecurity::retrieve_ocsp_cache(const CertificateHashData& certificate_hash_data) { std::lock_guard guard(EvseSecurity::security_mutex); return retrieve_ocsp_cache_internal(certificate_hash_data); } -std::optional -EvseSecurity::retrieve_ocsp_cache_internal(const CertificateHashData& certificate_hash_data) { +std::optional EvseSecurity::retrieve_ocsp_cache_internal(const CertificateHashData& certificate_hash_data) { // TODO(ioan): shouldn't we also do this for the MO? const auto ca_bundle_path = this->ca_bundle_path_map.at(CaCertificateType::V2G); const auto leaf_path = this->directories.secc_leaf_key_directory; @@ -903,13 +902,8 @@ EvseSecurity::retrieve_ocsp_cache_internal(const CertificateHashData& certificat fs::path replaced_ext = ocsp_entry.path(); replaced_ext.replace_extension(DER_EXTENSION); - std::ifstream in_fs(replaced_ext.c_str()); - std::string ocsp_response; - - in_fs >> ocsp_response; - in_fs.close(); - - return std::make_optional(std::move(ocsp_response)); + // Return the data file's path + return std::make_optional(replaced_ext); } } } @@ -1182,7 +1176,7 @@ GetCertificateInfoResult EvseSecurity::get_leaf_certificate_info_internal(LeafCe for (const auto& chain_certif : *leaf_fullchain) { try { CertificateHashData hash = hierarchy.get_certificate_hash(chain_certif); - std::optional data = retrieve_ocsp_cache_internal(hash); + std::optional data = retrieve_ocsp_cache_internal(hash); certificate_ocsp.push_back({hash, data}); } catch (const NoCertificateFound& e) { @@ -1326,8 +1320,10 @@ std::string EvseSecurity::get_verify_file(CaCertificateType certificate_type) { << this->ca_bundle_path_map.at(certificate_type) << " with error: " << e.what(); } - throw NoCertificateFound("Could not find any CA certificate for: " + - conversions::ca_certificate_type_to_string(certificate_type)); + EVLOG_error << "Could not find any CA certificate for: " + << conversions::ca_certificate_type_to_string(certificate_type); + + return {}; } int EvseSecurity::get_leaf_expiry_days_count(LeafCertificateType certificate_type) { @@ -1554,107 +1550,112 @@ void EvseSecurity::garbage_collect() { // Order by latest valid, and keep newest with a safety limit for (auto const& [cert_dir, key_dir, ca_type] : leaf_paths) { // Root bundle required for hash of OCSP cache - X509CertificateBundle root_bundle(ca_bundle_path_map[ca_type], EncodingFormat::PEM); - X509CertificateBundle expired_certs(cert_dir, EncodingFormat::PEM); - - // Only handle if we have more than the minimum certificates entry - if (expired_certs.get_certificate_chains_count() > DEFAULT_MINIMUM_CERTIFICATE_ENTRIES) { - fs::path key_directory = key_dir; - int skipped = 0; - - // Order by expiry date, and keep even expired certificates with a minimum of 10 certificates - expired_certs.for_each_chain_ordered( - [this, &invalid_certificate_files, &skipped, &key_directory, &protected_private_keys, - &root_bundle](const fs::path& file, const std::vector& chain) { - // By default delete all empty - if (chain.size() <= 0) { - invalid_certificate_files.emplace(file); - } - - if (++skipped > DEFAULT_MINIMUM_CERTIFICATE_ENTRIES) { - if (chain.empty()) { - return true; - } - - // If the chain contains the first expired (leafs are the first) - if (chain[0].is_expired()) { + try { + X509CertificateBundle root_bundle(ca_bundle_path_map[ca_type], EncodingFormat::PEM); + X509CertificateBundle expired_certs(cert_dir, EncodingFormat::PEM); + + // Only handle if we have more than the minimum certificates entry + if (expired_certs.get_certificate_chains_count() > DEFAULT_MINIMUM_CERTIFICATE_ENTRIES) { + fs::path key_directory = key_dir; + int skipped = 0; + + // Order by expiry date, and keep even expired certificates with a minimum of 10 certificates + expired_certs.for_each_chain_ordered( + [this, &invalid_certificate_files, &skipped, &key_directory, &protected_private_keys, + &root_bundle](const fs::path& file, const std::vector& chain) { + // By default delete all empty + if (chain.size() <= 0) { invalid_certificate_files.emplace(file); + } - // Also attempt to add the key for deletion - try { - fs::path key_file = get_private_key_path_of_certificate(chain[0], key_directory, - this->private_key_password); - invalid_certificate_files.emplace(key_file); - } catch (NoPrivateKeyException& e) { + if (++skipped > DEFAULT_MINIMUM_CERTIFICATE_ENTRIES) { + if (chain.empty()) { + return true; } - auto leaf_chain = chain; - X509CertificateHierarchy hierarchy = - std::move(X509CertificateHierarchy::build_hierarchy(root_bundle.split(), leaf_chain)); + // If the chain contains the first expired (leafs are the first) + if (chain[0].is_expired()) { + invalid_certificate_files.emplace(file); - try { - CertificateHashData ocsp_hash = hierarchy.get_certificate_hash(chain[0]); + // Also attempt to add the key for deletion + try { + fs::path key_file = get_private_key_path_of_certificate(chain[0], key_directory, + this->private_key_password); + invalid_certificate_files.emplace(key_file); + } catch (NoPrivateKeyException& e) { + } - // Find OCSP cache with hash - if (chain[0].get_file().has_value()) { - const auto ocsp_path = chain[0].get_file().value().parent_path() / "ocsp"; + auto leaf_chain = chain; + X509CertificateHierarchy hierarchy = std::move( + X509CertificateHierarchy::build_hierarchy(root_bundle.split(), leaf_chain)); - if (fs::exists(ocsp_path)) { - for (const auto& hash_entry : fs::directory_iterator(ocsp_path)) { - if (hash_entry.is_regular_file() == false) { - continue; - } - // Attempt hash read - CertificateHashData read_hash; + try { + CertificateHashData ocsp_hash = hierarchy.get_certificate_hash(chain[0]); + + // Find OCSP cache with hash + if (chain[0].get_file().has_value()) { + const auto ocsp_path = chain[0].get_file().value().parent_path() / "ocsp"; - if (filesystem_utils::read_hash_from_file(hash_entry.path(), read_hash) && - read_hash == ocsp_hash) { + if (fs::exists(ocsp_path)) { + for (const auto& hash_entry : fs::directory_iterator(ocsp_path)) { + if (hash_entry.is_regular_file() == false) { + continue; + } + // Attempt hash read + CertificateHashData read_hash; - auto oscp_data_path = hash_entry.path(); - oscp_data_path.replace_extension(CERT_HASH_EXTENSION); + if (filesystem_utils::read_hash_from_file(hash_entry.path(), + read_hash) && + read_hash == ocsp_hash) { - invalid_certificate_files.emplace(hash_entry.path()); - invalid_certificate_files.emplace(oscp_data_path); + auto oscp_data_path = hash_entry.path(); + oscp_data_path.replace_extension(DER_EXTENSION); + + invalid_certificate_files.emplace(hash_entry.path()); + invalid_certificate_files.emplace(oscp_data_path); + } } } } + } catch (const NoCertificateFound& e) { } - } catch (const NoCertificateFound& e) { } - } - } else { - // Add to protected certificate list - try { - fs::path key_file = get_private_key_path_of_certificate(chain[0], key_directory, - this->private_key_password); - protected_private_keys.emplace(key_file); - - // Erase all protected keys from the managed CRSs - auto it = managed_csr.find(key_file); - if (it != managed_csr.end()) { - managed_csr.erase(it); + } else { + // Add to protected certificate list + try { + fs::path key_file = get_private_key_path_of_certificate(chain[0], key_directory, + this->private_key_password); + protected_private_keys.emplace(key_file); + + // Erase all protected keys from the managed CRSs + auto it = managed_csr.find(key_file); + if (it != managed_csr.end()) { + managed_csr.erase(it); + } + } catch (NoPrivateKeyException& e) { } - } catch (NoPrivateKeyException& e) { } - } - return true; - }, - [](const std::vector& a, const std::vector& b) { - // Order from newest to oldest (newest DEFAULT_MINIMUM_CERTIFICATE_ENTRIES) are kept - // even if they are expired - if (a.size() && b.size()) { - return a.at(0).get_valid_to() > b.at(0).get_valid_to(); - } else { - return false; - } - }); + return true; + }, + [](const std::vector& a, const std::vector& b) { + // Order from newest to oldest (newest DEFAULT_MINIMUM_CERTIFICATE_ENTRIES) are kept + // even if they are expired + if (a.size() && b.size()) { + return a.at(0).get_valid_to() > b.at(0).get_valid_to(); + } else { + return false; + } + }); + } + } catch (const CertificateLoadException& e) { + EVLOG_warning << "Could not load bundle from file: " << e.what(); } - } + } // End leaf for iteration for (const auto& expired_certificate_file : invalid_certificate_files) { if (filesystem_utils::delete_file(expired_certificate_file)) - EVLOG_debug << "Deleted expired certificate file: " << expired_certificate_file; + EVLOG_info << "Deleted expired certificate file: " << expired_certificate_file; else EVLOG_warning << "Error deleting expired certificate file: " << expired_certificate_file; } @@ -1710,6 +1711,81 @@ void EvseSecurity::garbage_collect() { ++it; } } + + std::set invalid_ocsp_files; + + // Delete all non-owned OCSP data + for (const auto& leaf_certificate_path : + {directories.secc_leaf_cert_directory, directories.csms_leaf_cert_directory}) { + try { + bool secc = (leaf_certificate_path == directories.secc_leaf_cert_directory); + bool csms = (leaf_certificate_path == directories.csms_leaf_cert_directory) || + (directories.csms_leaf_cert_directory == directories.secc_leaf_cert_directory); + + CaCertificateType load; + + if (secc) + load = CaCertificateType::V2G; + else if (csms) + load = CaCertificateType::CSMS; + + // Also load the roots since we need to build the hierarchy for correct certificate hashes + X509CertificateBundle root_bundle(ca_bundle_path_map[load], EncodingFormat::PEM); + X509CertificateBundle leaf_bundle(leaf_certificate_path, EncodingFormat::PEM); + + fs::path leaf_ocsp; + fs::path root_ocsp; + + if (root_bundle.is_using_bundle_file()) { + root_ocsp = root_bundle.get_path().parent_path() / "ocsp"; + } else { + root_ocsp = root_bundle.get_path() / "ocsp"; + } + + if (leaf_bundle.is_using_bundle_file()) { + leaf_ocsp = leaf_bundle.get_path().parent_path() / "ocsp"; + } else { + leaf_ocsp = leaf_bundle.get_path() / "ocsp"; + } + + X509CertificateHierarchy hierarchy = + std::move(X509CertificateHierarchy::build_hierarchy(root_bundle.split(), leaf_bundle.split())); + + // Iterate all hashes folders and see if any are missing + for (auto& ocsp_dir : {leaf_ocsp, root_ocsp}) { + if (fs::exists(ocsp_dir)) { + for (auto& ocsp_entry : fs::directory_iterator(ocsp_dir)) { + if (ocsp_entry.is_regular_file() == false) { + continue; + } + + // Attempt hash read + CertificateHashData read_hash; + + if (filesystem_utils::read_hash_from_file(ocsp_entry.path(), read_hash)) { + // If we can't find the has, it means it was deleted somehow, add to delete list + if (hierarchy.contains_certificate_hash(read_hash) == false) { + auto oscp_data_path = ocsp_entry.path(); + oscp_data_path.replace_extension(DER_EXTENSION); + + invalid_ocsp_files.emplace(ocsp_entry.path()); + invalid_ocsp_files.emplace(oscp_data_path); + } + } + } + } + } + } catch (const CertificateLoadException& e) { + EVLOG_warning << "Could not load ca bundle from file: " << leaf_certificate_path; + } + } + + for (const auto& invalid_ocsp : invalid_ocsp_files) { + if (filesystem_utils::delete_file(invalid_ocsp)) + EVLOG_info << "Deleted invalid ocsp file: " << invalid_ocsp; + else + EVLOG_warning << "Error deleting invalid ocsp file: " << invalid_ocsp; + } } bool EvseSecurity::is_filesystem_full() { diff --git a/lib/evse_security/evse_types.cpp b/lib/evse_security/evse_types.cpp index 3ae13b7..c9d227e 100644 --- a/lib/evse_security/evse_types.cpp +++ b/lib/evse_security/evse_types.cpp @@ -93,7 +93,7 @@ std::string hash_algorithm_to_string(HashAlgorithm e) { } }; -HashAlgorithm string_to_hash_algorithm(std::string s) { +HashAlgorithm string_to_hash_algorithm(const std::string& s) { if (s == "SHA256") return HashAlgorithm::SHA256; else if (s == "SHA384") diff --git a/tests/tests.cpp b/tests/tests.cpp index 3beee71..136e52a 100644 --- a/tests/tests.cpp +++ b/tests/tests.cpp @@ -21,7 +21,7 @@ #if USING_OPENSSL_3 // provider management has changed - ensure tests still work -#ifndef USING_TPM2 +#ifndef USING_TPM2contains_certificate_hash #include #else @@ -808,7 +808,7 @@ TEST_F(EvseSecurityTests, verify_oscp_cache) { for (auto& ocsp : data.ocsp_request_data_list) { std::optional data = this->evse_security->retrieve_ocsp_cache(ocsp.certificate_hash_data.value()); ASSERT_TRUE(data.has_value()); - ASSERT_EQ(data.value(), ocsp_mock_response_data); + ASSERT_EQ(read_file_to_string(data.value()), ocsp_mock_response_data); } int entries = 0; @@ -847,7 +847,7 @@ TEST_F(EvseSecurityTests, verify_oscp_cache) { for (auto& ocsp : data.ocsp_request_data_list) { std::optional data = this->evse_security->retrieve_ocsp_cache(ocsp.certificate_hash_data.value()); ASSERT_TRUE(data.has_value()); - ASSERT_EQ(data.value(), ocsp_mock_response_data_v2); + ASSERT_EQ(read_file_to_string(data.value()), ocsp_mock_response_data_v2); } // Make sure the info was over-written @@ -895,11 +895,78 @@ TEST_F(EvseSecurityTests, verify_oscp_cache) { for (int i = 1; i < info.ocsp.size(); ++i) { auto& ocsp = info.ocsp[i]; - ASSERT_TRUE(ocsp.oscsp_data.has_value()); - ASSERT_EQ(ocsp.oscsp_data.value(), ocsp_mock_response_data_v2); + ASSERT_TRUE(ocsp.ocsp_path.has_value()); + ASSERT_EQ(read_file_to_string(ocsp.ocsp_path.value()), ocsp_mock_response_data_v2); } } +TEST_F(EvseSecurityTests, verify_ocsp_garbage_collect) { + std::string ocsp_mock_response_data = "OCSP_MOCK_RESPONSE_DATA"; + + OCSPRequestDataList data = this->evse_security->get_v2g_ocsp_request_data(); + ASSERT_EQ(data.ocsp_request_data_list.size(), 2); + + // Mock a response + for (auto& ocsp : data.ocsp_request_data_list) { + this->evse_security->update_ocsp_cache(ocsp.certificate_hash_data.value(), ocsp_mock_response_data); + } + + // Make sure all info was written and that it is correct + fs::path ocsp_path = "certs/ca/v2g/ocsp"; + fs::path ocsp_path2 = "certs/client/cso/ocsp"; + + ASSERT_TRUE(fs::exists(ocsp_path)); + + for (auto& ocsp : data.ocsp_request_data_list) { + std::optional data = this->evse_security->retrieve_ocsp_cache(ocsp.certificate_hash_data.value()); + ASSERT_TRUE(data.has_value()); + ASSERT_EQ(read_file_to_string(data.value()), ocsp_mock_response_data); + } + + evse_security->max_fs_certificate_store_entries = 1; + ASSERT_TRUE(evse_security->is_filesystem_full()); + + // Garbage collect to see that we don't delete improper data + this->evse_security->garbage_collect(); + + ASSERT_TRUE(fs::exists(ocsp_path)); + ASSERT_TRUE(fs::exists(ocsp_path2)); + + // Check existence of OCSP data + int existing = 0; + for (auto& ocsp_path : {ocsp_path, ocsp_path2}) { + for (auto& ocsp_entry : fs::directory_iterator(ocsp_path)) { + auto ext = ocsp_entry.path().extension(); + if (ext == DER_EXTENSION || ext == CERT_HASH_EXTENSION) { + existing++; + } + } + } + + ASSERT_EQ(existing, 8); + + // Delete the certificates that had their OCSP data appended + fs::remove("certs/ca/v2g/V2G_CA_BUNDLE.pem"); + fs::remove("certs/ca/v2g/V2G_ROOT_CA.pem"); + fs::remove("certs/client/cso/CPO_CERT_CHAIN.pem"); + + // Garbage collect again + this->evse_security->garbage_collect(); + + // Check deletion + existing = 0; + for (auto& ocsp_path : {ocsp_path, ocsp_path2}) { + for (auto& ocsp_entry : fs::directory_iterator(ocsp_path)) { + auto ext = ocsp_entry.path().extension(); + if (ext == DER_EXTENSION || ext == CERT_HASH_EXTENSION) { + existing++; + } + } + } + + ASSERT_EQ(existing, 0); +} + TEST_F(EvseSecurityTestsExpired, verify_expired_leaf_deletion) { // Check that the FS is not full ASSERT_FALSE(evse_security->is_filesystem_full()); @@ -946,8 +1013,6 @@ TEST_F(EvseSecurityTestsExpired, verify_expired_leaf_deletion) { // Garbage collect evse_security->garbage_collect(); - // TODO: (ioan) test OCSP cache deletion - // Ensure that we have 10 certificates, since we only keep 10, the newest { X509CertificateBundle full_certs(fs::path("certs/client/cso"), EncodingFormat::PEM);