Added security limits for certificate deletion to prevent bricking th…

…e charger

Signed-off-by: AssemblyJohn <[email protected]>

include/evse_security/certificate/x509_bundle.hpp

@@ -2,6 +2,7 @@
 // Copyright Pionix GmbH and Contributors to EVerest
 #pragma once
 
+#include <algorithm>
 #include <map>
 
 #include <evse_security/certificate/x509_hierarchy.hpp>
@@ -66,6 +67,9 @@ class X509CertificateBundle {
     /// @return Contained certificate count
     int get_certificate_count() const;
 
+    /// @return Contained certificate chains count
+    int get_certificate_chains_count() const;
+
     fs::path get_path() const {
         return path;
     }
@@ -83,6 +87,28 @@ class X509CertificateBundle {
         }
     }
 
+    /// @brief Same as 'for_each_chain' but it also uses a predicate for ordering
+    template <typename function, typename ordering> void for_each_chain_ordered(function func, ordering order) {
+        struct Chain {
+            const fs::path* path;
+            const std::vector<X509Wrapper>* certificates;
+        };
+
+        std::vector<Chain> ordered;
+
+        for (auto& [path, certs] : certificates) {
+            ordered.push_back(Chain{&path, &certs});
+        }
+
+        std::sort(std::begin(ordered), std::end(ordered),
+                  [&order](Chain& a, Chain& b) { return order(*a.certificates, *b.certificates); });
+
+        for (const auto& chain : ordered) {
+            if (!func(*chain.path, *chain.certificates))
+                break;
+        }
+    }
+
 public:
     /// @brief Splits the certificate (chain) into single certificates
     /// @return vector containing single certificates

include/evse_security/evse_security.hpp

@@ -37,13 +37,15 @@ struct FilePaths {
     LinkPaths links;
 };
 
+// Unchangeable security limit for certificate deletion, a min entry count will be always kept (newest)
+static constexpr std::size_t DEFAULT_MINIMUM_CERTIFICATE_ENTRIES = 10;
 // 50 MB default limit for filesystem usage
 static constexpr std::uintmax_t DEFAULT_MAX_FILESYSTEM_SIZE = 1024 * 1024 * 50;
 // Default maximum 2000 certificate entries
 static constexpr std::uintmax_t DEFAULT_MAX_CERTIFICATE_ENTRIES = 2000;
 
-// Expiry for CSRs that did not receive a response CSR
-static std::chrono::seconds DEFAULT_CSR_EXPIRY(5 * 60);
+// Expiry for CSRs that did not receive a response CSR, 10 minutes or reboot
+static std::chrono::seconds DEFAULT_CSR_EXPIRY(10 * 60);
 
 /// @brief This class holds filesystem paths to CA bundle file locations and directories for leaf certificates
 class EvseSecurity {

lib/evse_security/certificate/x509_bundle.cpp

@@ -87,6 +87,10 @@ int X509CertificateBundle::get_certificate_count() const {
     return count;
 }
 
+int X509CertificateBundle::get_certificate_chains_count() const {
+    return certificates.size();
+}
+
 void X509CertificateBundle::add_certificates(const std::string& data, const EncodingFormat encoding,
                                              const std::optional<fs::path>& path) {
     auto loaded = CryptoSupplier::load_certificates(data, encoding);

lib/evse_security/evse_security.cpp

@@ -200,6 +200,17 @@ EvseSecurity::EvseSecurity(const FilePaths& file_paths, const std::optional<std:
         }
     }
 
+    // Check that the leafs directory is not related to the bundle directory because
+    // on garbage collect that can delete relevant CA certificates instead of leaf ones
+    for (const auto& leaf_dir : dirs) {
+        for (auto const& [certificate_type, ca_bundle_path] : ca_bundle_path_map) {
+            if (ca_bundle_path == leaf_dir) {
+                throw std::runtime_error(leaf_dir.string() +
+                                         " leaf directory can not overlap CA directory: " + ca_bundle_path.string());
+            }
+        }
+    }
+
     this->directories = file_paths.directories;
     this->links = file_paths.links;
 
@@ -1012,23 +1023,44 @@ void EvseSecurity::garbage_collect(bool delete_expired_certificates) {
 
         std::set<fs::path> invalid_certificate_files;
 
-        // TODO: Order by latest valid, and keep newest with a safety limit
+        // Order by latest valid, and keep newest with a safety limit
         for (auto const& [cert_dir, key_dir] : leaf_paths) {
             X509CertificateBundle expired_certs(cert_dir, EncodingFormat::PEM);
 
-            expired_certs.for_each_chain(
-                [&invalid_certificate_files](const fs::path& file, const std::vector<X509Wrapper>& chain) {
-                    // If the chain contains the first expired (leafs are the first)
-                    if (chain.size()) {
-                        if (chain[0].is_expired()) {
+            // Only handle if we have more than the minimum certificates entry
+            if (expired_certs.get_certificate_chains_count() > DEFAULT_MINIMUM_CERTIFICATE_ENTRIES) {
+                int skipped = 0;
+
+                // Order by expiry date, and keep even expired certificates with a minimum of 10 certificates
+                expired_certs.for_each_chain_ordered(
+                    [&invalid_certificate_files, &skipped](const fs::path& file,
+                                                           const std::vector<X509Wrapper>& chain) {
+                        // By default delete all empty
+                        if (chain.size() <= 0) {
                             invalid_certificate_files.emplace(file);
                         }
-                    } else {
-                        invalid_certificate_files.emplace(file);
-                    }
 
-                    return true;
-                });
+                        if (skipped++ > DEFAULT_MINIMUM_CERTIFICATE_ENTRIES) {
+                            // If the chain contains the first expired (leafs are the first)
+                            if (chain.size()) {
+                                if (chain[0].is_expired()) {
+                                    invalid_certificate_files.emplace(file);
+                                }
+                            }
+                        }
+
+                        return true;
+                    },
+                    [](const std::vector<X509Wrapper>& a, const std::vector<X509Wrapper>& 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;
+                        }
+                    });
+            }
         }
 
         for (const auto& expired_certificate_file : invalid_certificate_files) {

tests/tests.cpp

@@ -731,6 +731,9 @@ TEST_F(EvseSecurityTests, verify_expired_csr_deletion) {
         evse_security->generate_certificate_signing_request(LeafCertificateType::CSMS, "DE", "Pionix", "NA");
     fs::path csr_key_path = evse_security->managed_csr.begin()->first;
 
+    // Simulate a full fs else no deletion will take place
+    evse_security->max_fs_usage_bytes = 1;
+
     ASSERT_EQ(evse_security->managed_csr.size(), 1);
     ASSERT_TRUE(fs::exists(csr_key_path));