From 72e0b45fe41292227b6438e19e710b99976205b2 Mon Sep 17 00:00:00 2001 From: AssemblyJohn Date: Mon, 6 May 2024 11:40:32 +0300 Subject: [PATCH] Added filesystem error handling Signed-off-by: AssemblyJohn --- .../evse_security/utils/evse_filesystem.hpp | 2 + lib/evse_security/certificate/x509_bundle.cpp | 15 +--- lib/evse_security/evse_security.cpp | 43 ++++++++---- lib/evse_security/utils/evse_filesystem.cpp | 68 +++++++++++++++---- 4 files changed, 88 insertions(+), 40 deletions(-) diff --git a/include/evse_security/utils/evse_filesystem.hpp b/include/evse_security/utils/evse_filesystem.hpp index 67b9afb..997780b 100644 --- a/include/evse_security/utils/evse_filesystem.hpp +++ b/include/evse_security/utils/evse_filesystem.hpp @@ -14,6 +14,8 @@ bool is_subdirectory(const fs::path& base, const fs::path& subdir); /// @brief Should be used to ensure file exists, not for directories bool create_file_if_nonexistent(const fs::path& file_path); +/// @brief Ensure a file exists (if there's an extension), or a directory if no extension is found +bool create_file_or_dir_if_nonexistent(const fs::path& file_path); bool delete_file(const fs::path& file_path); bool read_from_file(const fs::path& file_path, std::string& out_data); diff --git a/lib/evse_security/certificate/x509_bundle.cpp b/lib/evse_security/certificate/x509_bundle.cpp index 1e8a43c..6cfea58 100644 --- a/lib/evse_security/certificate/x509_bundle.cpp +++ b/lib/evse_security/certificate/x509_bundle.cpp @@ -45,19 +45,8 @@ 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); - } - } + // Attempt creation + filesystem_utils::create_file_or_dir_if_nonexistent(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 8eb2227..d38ed67 100644 --- a/lib/evse_security/evse_security.cpp +++ b/lib/evse_security/evse_security.cpp @@ -103,13 +103,14 @@ static fs::path get_private_key_path_of_certificate(const X509Wrapper& certifica if (fs::exists(potential_keyfile)) { try { - fsstd::ifstream file(potential_keyfile, std::ios::binary); - std::string private_key((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + std::string private_key; - if (KeyValidationResult::Valid == - CryptoSupplier::x509_check_private_key(certificate.get(), private_key, password)) { - EVLOG_debug << "Key found for certificate at path: " << potential_keyfile; - return potential_keyfile; + if (filesystem_utils::read_from_file(potential_keyfile, private_key)) { + if (KeyValidationResult::Valid == + CryptoSupplier::x509_check_private_key(certificate.get(), private_key, password)) { + EVLOG_debug << "Key found for certificate at path: " << potential_keyfile; + return potential_keyfile; + } } } catch (const std::exception& e) { EVLOG_debug << "Could not load or verify private key at: " << potential_keyfile << ": " << e.what(); @@ -123,13 +124,14 @@ static fs::path get_private_key_path_of_certificate(const X509Wrapper& certifica auto key_file_path = entry.path(); if (is_keyfile(key_file_path)) { try { - fsstd::ifstream file(key_file_path, std::ios::binary); - std::string private_key((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + std::string private_key; - if (KeyValidationResult::Valid == - CryptoSupplier::x509_check_private_key(certificate.get(), private_key, password)) { - EVLOG_debug << "Key found for certificate at path: " << key_file_path; - return key_file_path; + if (filesystem_utils::read_from_file(key_file_path, private_key)) { + if (KeyValidationResult::Valid == + CryptoSupplier::x509_check_private_key(certificate.get(), private_key, password)) { + EVLOG_debug << "Key found for certificate at path: " << key_file_path; + return key_file_path; + } } } catch (const std::exception& e) { EVLOG_debug << "Could not load or verify private key at: " << key_file_path << ": " << e.what(); @@ -151,8 +153,11 @@ static fs::path get_private_key_path_of_certificate(const X509Wrapper& certifica /// present in a bundle too static std::set get_certificate_path_of_key(const fs::path& key, const fs::path& certificate_path_directory, const std::optional password) { - fsstd::ifstream file(key, std::ios::binary); - std::string private_key((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + std::string private_key; + + if (false == filesystem_utils::read_from_file(key, private_key)) { + throw NoPrivateKeyException("Could not read private key from path: " + private_key); + } // Before iterating all bundles, check by certificates from key filename fs::path cert_filename = key; @@ -819,7 +824,7 @@ void EvseSecurity::update_ocsp_cache(const CertificateHashData& certificate_hash const auto ocsp_path = cert.get_file().value().parent_path() / "ocsp"; if (false == fs::exists(ocsp_path)) { - fs::create_directories(ocsp_path); + filesystem_utils::create_file_or_dir_if_nonexistent(ocsp_path); } else { // Iterate existing hashes for (const auto& hash_entry : fs::directory_iterator(ocsp_path)) { @@ -1756,6 +1761,8 @@ void EvseSecurity::garbage_collect() { } if (is_keyfile(key_file_path)) { + bool error = false; + try { // Check if we have found any matching certificate get_certificate_path_of_key(key_file_path, keys_dir, this->private_key_password); @@ -1763,7 +1770,13 @@ void EvseSecurity::garbage_collect() { // If we did not found, add to the potential delete list EVLOG_debug << "Could not find matching certificate for key: " << key_file_path << " adding to potential deletes"; + error = true; + } catch (const NoPrivateKeyException& e) { + EVLOG_debug << "Could not load private key: " << key_file_path << " adding to potential deletes"; + error = true; + } + if (error) { // Give a chance to be fulfilled by the CSMS if (managed_csr.find(key_file_path) == managed_csr.end()) { managed_csr.emplace(key_file_path, std::chrono::steady_clock::now()); diff --git a/lib/evse_security/utils/evse_filesystem.cpp b/lib/evse_security/utils/evse_filesystem.cpp index 65dc20c..ed71fb5 100644 --- a/lib/evse_security/utils/evse_filesystem.cpp +++ b/lib/evse_security/utils/evse_filesystem.cpp @@ -18,21 +18,30 @@ bool is_subdirectory(const fs::path& base, const fs::path& subdir) { } bool delete_file(const fs::path& file_path) { - if (fs::is_regular_file(file_path)) - return fs::remove(file_path); + try { + if (fs::is_regular_file(file_path)) { + return fs::remove(file_path); + } + } catch (const std::exception& e) { + EVLOG_error << "Filesystem error: " << e.what(); + } EVLOG_error << "Error deleting file: " << file_path; return false; } bool read_from_file(const fs::path& file_path, std::string& out_data) { - if (fs::is_regular_file(file_path)) { - fsstd::ifstream file(file_path, std::ios::binary); + try { + if (fs::is_regular_file(file_path)) { + fsstd::ifstream file(file_path, std::ios::binary); - if (file.is_open()) { - out_data = std::string((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - return true; + if (file.is_open()) { + out_data = std::string((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + return true; + } } + } catch (const std::exception& e) { + EVLOG_error << "Error while reading from file: " << e.what(); } EVLOG_error << "Error reading file: " << file_path; @@ -40,17 +49,52 @@ bool read_from_file(const fs::path& file_path, std::string& out_data) { } bool create_file_if_nonexistent(const fs::path& file_path) { - if (!fs::exists(file_path)) { - std::ofstream file(file_path); - return true; - } else if (fs::is_directory(file_path)) { - EVLOG_error << "Attempting to create file over existing directory: " << file_path; + if (file_path.empty()) { + EVLOG_warning << "Provided empty path!"; return false; } + try { + if (!fs::exists(file_path)) { + std::ofstream file(file_path); + return true; + } else if (fs::is_directory(file_path)) { + EVLOG_error << "Attempting to create file over existing directory: " << file_path; + return false; + } + } catch (const std::exception& e) { + EVLOG_error << "Error while creating file: " << e.what(); + } + return true; } +bool create_file_or_dir_if_nonexistent(const fs::path& path) { + if (path.empty()) { + EVLOG_warning << "Provided empty path!"; + return false; + } + + try { + // In case the path is missing, create it + if (fs::exists(path) == false) { + if (path.has_extension()) { + std::ofstream new_file(path.c_str()); + new_file.close(); + return true; + } else { + // Else create a directory + fs::create_directories(path); + return true; + } + } + } catch (const std::exception& e) { + EVLOG_error << "Error while creating dir/file: " << e.what(); + } + + return false; +} + bool write_to_file(const fs::path& file_path, const std::string& data, std::ios::openmode mode) { try { fsstd::ofstream fs(file_path, mode | std::ios::binary);