Skip to content

Commit

Permalink
Fixes and added relevant tests
Browse files Browse the repository at this point in the history 
Signed-off-by: AssemblyJohn <[email protected]>
  • Loading branch information
@AssemblyJohn
AssemblyJohn committed Apr 23, 2024
1 parent e21294f commit 1cf19dc
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 30 deletions.
2 changes: 1 addition & 1 deletion lib/evse_security/crypto/openssl/openssl_supplier.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ KeyValidationResult OpenSSLSupplier::x509_check_private_key(X509Handle* handle,
} 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
Expand Down
37 changes: 16 additions & 21 deletions lib/evse_security/evse_security.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,21 +512,15 @@ InstallCertificateResult EvseSecurity::update_leaf_certificate(const std::string
if (filesystem_utils::write_to_file(file_path, str_cert, std::ios::out) &&
filesystem_utils::write_to_file(chain_file_path, str_chain_cert, std::ios::out)) {

// Remove from managed certificate keys, it is safe, no need to delete
if (managed_csr.find(private_key_path) != managed_csr.end()) {
managed_csr.erase(managed_csr.find(private_key_path));
// Remove from managed certificate keys, the CSR is fulfilled, no need to delete the key
// since it is not orphaned any more
auto it = managed_csr.find(private_key_path);
if (it != managed_csr.end()) {
managed_csr.erase(it);
}

// Rename private key file to certificate name for a possible quicker retrieval
// TODO(ioan): properly rename key path here for fast retrieval
// @see 'get_private_key_path_of_certificate' and 'get_certificate_path_of_key'
fs::path new_private_key_path = file_path;
new_private_key_path.replace_extension(private_key_path.extension());

if (fs::exists(new_private_key_path) == false) {
EVLOG_debug << "Updated private keypath: " << private_key_path
<< " to new keypath: " << new_private_key_path;
fs::rename(private_key_path, new_private_key_path);
}

return InstallCertificateResult::Accepted;
} else {
Expand Down Expand Up @@ -861,7 +855,7 @@ std::string EvseSecurity::generate_certificate_signing_request(LeafCertificateTy
} else if (certificate_type == LeafCertificateType::V2G) {
key_path = this->directories.secc_leaf_key_directory / file_name;
} else {
EVLOG_error << "generate_certificate_signing_request: create filename failed";
EVLOG_error << "Generate CSR: create filename failed";
throw std::runtime_error("Attempt to generate CSR for MF certificate");
}

Expand Down Expand Up @@ -891,15 +885,16 @@ std::string EvseSecurity::generate_certificate_signing_request(LeafCertificateTy
info.key_info.private_key_pass = private_key_password;
}

EVLOG_info << "generate_certificate_signing_request: start";
EVLOG_debug << "Generate CSR start";

if (false == CryptoSupplier::x509_generate_csr(info, csr)) {
throw std::runtime_error("Failed to generate certificate signing request!");
EVLOG_error << "Failed to generate certificate signing request!";
} else {
// Add the key to the managed CRS that we will delete if we can't find a certificate pair within the time
managed_csr.emplace(key_path, std::chrono::steady_clock::now());
}

// Add the key to the managed CRS that we will delete if we can't find a certificate pair within the time
managed_csr.emplace(key_path, std::chrono::steady_clock::now());

EVLOG_debug << csr;
EVLOG_debug << "Generated CSR end. CSR: " << csr;
return csr;
}

Expand Down Expand Up @@ -1355,8 +1350,8 @@ void EvseSecurity::garbage_collect() {

// 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](const fs::path& file,
const std::vector<X509Wrapper>& chain) {
[this, &invalid_certificate_files, &skipped, &key_directory,
&protected_private_keys](const fs::path& file, const std::vector<X509Wrapper>& chain) {
// By default delete all empty
if (chain.size() <= 0) {
invalid_certificate_files.emplace(file);
Expand Down
37 changes: 29 additions & 8 deletions tests/tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#if USING_OPENSSL_3
// provider management has changed - ensure tests still work
#ifndef USING_TPM2

#include <evse_security/detail/openssl/openssl_providers.hpp>
#else

Expand Down Expand Up @@ -790,8 +791,8 @@ TEST_F(EvseSecurityTestsExpired, verify_expired_leaf_deletion) {

// List of date sorted certificates
std::vector<X509Wrapper> sorted;
std::vector<fs::path> sorded_should_delete;
std::vector<fs::path> sorded_should_keep;
std::vector<fs::path> sorted_should_delete;
std::vector<fs::path> sorted_should_keep;

// Ensure that we have GEN_CERTIFICATES + 2 (CPO_CERT_CHAIN.pem + SECC_LEAF.pem)
{
Expand All @@ -816,9 +817,9 @@ TEST_F(EvseSecurityTestsExpired, verify_expired_leaf_deletion) {

for (const auto& cert : sorted) {
if (++skipped > DEFAULT_MINIMUM_CERTIFICATE_ENTRIES) {
sorded_should_delete.push_back(cert.get_file().value());
sorted_should_delete.push_back(cert.get_file().value());
} else {
sorded_should_keep.push_back(cert.get_file().value());
sorted_should_keep.push_back(cert.get_file().value());
}
}

Expand All @@ -836,12 +837,32 @@ TEST_F(EvseSecurityTestsExpired, verify_expired_leaf_deletion) {
ASSERT_EQ(full_certs.get_certificate_chains_count(), DEFAULT_MINIMUM_CERTIFICATE_ENTRIES);

// Ensure that we only have the newest ones
for (const auto& deleted : sorded_should_delete) {
for (const auto& deleted : sorted_should_delete) {
ASSERT_FALSE(fs::exists(deleted));
}

for (const auto& deleted : sorded_should_keep) {
ASSERT_TRUE(fs::exists(deleted));
for (const auto& not_deleted : sorted_should_keep) {
fs::path key_file = not_deleted;
key_file.replace_extension(".key");

ASSERT_TRUE(fs::exists(not_deleted));

// Ignore the CPO chain that does not have a properly
if (not_deleted.string().find("CPO_CERT_CHAIN") != std::string::npos) {
key_file = "certs/client/cso/SECC_LEAF.key";
}

// Check their respective keys exist
EVLOG_info << key_file;
ASSERT_TRUE(fs::exists(key_file));

X509Wrapper cert = X509CertificateBundle(not_deleted, EncodingFormat::PEM).split().at(0);

fsstd::ifstream file(key_file, std::ios::binary);
std::string private_key((std::istreambuf_iterator<char>(file)), std::istreambuf_iterator<char>());

ASSERT_EQ(KeyValidationResult::Valid, CryptoSupplier::x509_check_private_key(
cert.get(), private_key, evse_security->private_key_password));
}
}
}
Expand Down Expand Up @@ -896,4 +917,4 @@ TEST_F(EvseSecurityTests, verify_expired_csr_deletion) {

} // namespace evse_security

// FIXME(piet): Add more tests for getRootCertificateHashData (incl. V2GCertificateChain etc.)
// FIXME(piet): Add more tests for getRootCertificateHashData (incl. V2GCertificateChain etc.)

0 comments on commit 1cf19dc

Please sign in to comment.