Bugfix/certificate cleanup (#65)

* Added extra check for when deleting potentially orphaned private keys, only if a matching certificate is not found Signed-off-by: AssemblyJohn <[email protected]> * Updated get_key_pair to iterate multiple valid certificates in search for a private key Signed-off-by: AssemblyJohn <[email protected]> * Added mandatory kept key protection Signed-off-by: AssemblyJohn <[email protected]> * Fixes and added relevant tests Signed-off-by: AssemblyJohn <[email protected]> * Bump version Signed-off-by: AssemblyJohn <[email protected]> --------- Signed-off-by: AssemblyJohn <[email protected]>
EVerest · Apr 23, 2024 · 34e89b2 · 34e89b2
1 parent 4a4cf5a
commit 34e89b2
Show file tree

Hide file tree

Showing 11 changed files with 238 additions and 71 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.14)
 
-project(everest-evse_security VERSION 0.5
+project(everest-evse_security VERSION 0.6
         DESCRIPTION "Implementation of EVSE related security operations"
 		LANGUAGES CXX C
 )

diff --git a/include/evse_security/crypto/interface/crypto_supplier.hpp b/include/evse_security/crypto/interface/crypto_supplier.hpp
@@ -64,8 +64,8 @@ class AbstractCryptoSupplier {
                                                                      const std::optional<fs::path> file_path);
 
     /// @brief Checks if the private key is consistent with the provided handle
-    static bool x509_check_private_key(X509Handle* handle, std::string private_key,
-                                       std::optional<std::string> password);
+    static KeyValidationResult x509_check_private_key(X509Handle* handle, std::string private_key,
+                                                      std::optional<std::string> password);
 
     /// @brief Verifies the signature with the certificate handle public key against the data
     static bool x509_verify_signature(X509Handle* handle, const std::vector<std::byte>& signature,

diff --git a/include/evse_security/crypto/interface/crypto_types.hpp b/include/evse_security/crypto/interface/crypto_types.hpp
@@ -18,6 +18,13 @@ enum class CryptoKeyType {
     RSA_7680,      // Protection lifetime: >2031
 };
 
+enum class KeyValidationResult {
+    Valid,
+    KeyLoadFailure, // The key could not be loaded, might be an password or invalid string
+    Invalid,        // The key is not linked to the specified certificate
+    Unknown,        // Unknown error, not related to provider validation
+};
+
 struct KeyGenerationInfo {
     CryptoKeyType key_type;
 

diff --git a/include/evse_security/crypto/openssl/openssl_supplier.hpp b/include/evse_security/crypto/openssl/openssl_supplier.hpp
@@ -36,8 +36,8 @@ class OpenSSLSupplier : public AbstractCryptoSupplier {
                                                                      bool allow_future_certificates,
                                                                      const std::optional<fs::path> dir_path,
                                                                      const std::optional<fs::path> file_path);
-    static bool x509_check_private_key(X509Handle* handle, std::string private_key,
-                                       std::optional<std::string> password);
+    static KeyValidationResult x509_check_private_key(X509Handle* handle, std::string private_key,
+                                                      std::optional<std::string> password);
     static bool x509_verify_signature(X509Handle* handle, const std::vector<std::byte>& signature,
                                       const std::vector<std::byte>& data);
 

diff --git a/include/evse_security/crypto/openssl/openssl_tpm.hpp b/include/evse_security/crypto/openssl/openssl_tpm.hpp
@@ -9,6 +9,8 @@
 #include <mutex>
 #include <string>
 
+#include <evse_security/utils/evse_filesystem_types.hpp>
+
 // opaque types (from OpenSSL)
 struct ossl_lib_ctx_st;  // OpenSSL OSSL_LIB_CTX;
 struct ossl_provider_st; // OpenSSL OSSL_PROVIDER
@@ -25,7 +27,7 @@ bool is_tpm_key_string(const std::string& private_key_pem);
 /// @param private_key_file_pem filename of the PEM file
 /// @return true when file starts "-----BEGIN TSS2 PRIVATE KEY-----"
 /// @note works irrespective of OpenSSL version
-bool is_tpm_key_filename(const std::string& private_key_file_pem);
+bool is_tpm_key_file(const fs::path& private_key_file_pem);
 
 /// @brief Manage the loading and configuring of OpenSSL providers
 ///

diff --git a/lib/evse_security/crypto/interface/crypto_supplier.cpp b/lib/evse_security/crypto/interface/crypto_supplier.cpp
@@ -81,9 +81,9 @@ CertificateValidationResult AbstractCryptoSupplier::x509_verify_certificate_chai
     default_crypto_supplier_usage_error() return CertificateValidationResult::Unknown;
 }
 
-bool AbstractCryptoSupplier::x509_check_private_key(X509Handle* handle, std::string private_key,
-                                                    std::optional<std::string> password) {
-    default_crypto_supplier_usage_error() return false;
+KeyValidationResult AbstractCryptoSupplier::x509_check_private_key(X509Handle* handle, std::string private_key,
+                                                                   std::optional<std::string> password) {
+    default_crypto_supplier_usage_error() return KeyValidationResult::Unknown;
 }
 
 bool AbstractCryptoSupplier::x509_verify_signature(X509Handle* handle, const std::vector<std::byte>& signature,

diff --git a/lib/evse_security/crypto/openssl/openssl_supplier.cpp b/lib/evse_security/crypto/openssl/openssl_supplier.cpp
@@ -615,12 +615,13 @@ CertificateValidationResult OpenSSLSupplier::x509_verify_certificate_chain(X509H
     return CertificateValidationResult::Valid;
 }
 
-bool OpenSSLSupplier::x509_check_private_key(X509Handle* handle, std::string private_key,
-                                             std::optional<std::string> password) {
+KeyValidationResult OpenSSLSupplier::x509_check_private_key(X509Handle* handle, std::string private_key,
+                                                            std::optional<std::string> password) {
     X509* x509 = get(handle);
 
-    if (x509 == nullptr)
-        return false;
+    if (x509 == nullptr) {
+        return KeyValidationResult::Unknown;
+    }
 
     OpenSSLProvider provider;
 
@@ -630,7 +631,7 @@ bool OpenSSLSupplier::x509_check_private_key(X509Handle* handle, std::string pri
     } else {
         provider.set_global_mode(OpenSSLProvider::mode_t::default_provider);
     }
-    EVLOG_info << "TPM Key: " << tpm_key;
+    EVLOG_debug << "TPM Key: " << tpm_key;
 
     BIO_ptr bio(BIO_new_mem_buf(private_key.c_str(), -1));
     // Passing password string since if NULL is provided, the password CB will be called
@@ -642,11 +643,14 @@ bool OpenSSLSupplier::x509_check_private_key(X509Handle* handle, std::string pri
                       << " Password configured correctly?";
         ERR_print_errors_fp(stderr);
 
-        bResult = false;
+        return KeyValidationResult::KeyLoadFailure;
     }
 
-    bResult = bResult && X509_check_private_key(x509, evp_pkey.get()) == 1;
-    return bResult;
+    if (X509_check_private_key(x509, evp_pkey.get()) == 1) {
+        return KeyValidationResult::Valid;
+    } else {
+        return KeyValidationResult::Invalid;
+    }
 }
 
 bool OpenSSLSupplier::x509_verify_signature(X509Handle* handle, const std::vector<std::byte>& signature,

diff --git a/lib/evse_security/crypto/openssl/openssl_tpm.cpp b/lib/evse_security/crypto/openssl/openssl_tpm.cpp
@@ -29,12 +29,17 @@ bool is_tpm_key_string(const std::string& private_key_pem) {
     return private_key_pem.find("-----BEGIN TSS2 PRIVATE KEY-----") != std::string::npos;
 }
 
-bool is_tpm_key_filename(const std::string& private_key_file_pem) {
-    std::ifstream key_file(private_key_file_pem);
-    std::string line;
-    std::getline(key_file, line);
-    key_file.close();
-    return line == "-----BEGIN TSS2 PRIVATE KEY-----";
+bool is_tpm_key_file(const fs::path& private_key_file_pem) {
+    if (fs::is_regular_file(private_key_file_pem)) {
+        std::ifstream key_file(private_key_file_pem);
+        std::string line;
+        std::getline(key_file, line);
+        key_file.close();
+
+        return line.find("-----BEGIN TSS2 PRIVATE KEY-----") != std::string::npos;
+    }
+
+    return false;
 }
 
 #ifdef USING_OPENSSL_3_TPM