diff --git a/lib/evse_security/crypto/openssl/openssl_supplier.cpp b/lib/evse_security/crypto/openssl/openssl_supplier.cpp index a75ebd6..543721c 100644 --- a/lib/evse_security/crypto/openssl/openssl_supplier.cpp +++ b/lib/evse_security/crypto/openssl/openssl_supplier.cpp @@ -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 diff --git a/lib/evse_security/evse_security.cpp b/lib/evse_security/evse_security.cpp index afca6d3..c45511c 100644 --- a/lib/evse_security/evse_security.cpp +++ b/lib/evse_security/evse_security.cpp @@ -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 { @@ -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"); } @@ -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; } @@ -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& chain) { + [this, &invalid_certificate_files, &skipped, &key_directory, + &protected_private_keys](const fs::path& file, const std::vector& chain) { // By default delete all empty if (chain.size() <= 0) { invalid_certificate_files.emplace(file); diff --git a/tests/tests.cpp b/tests/tests.cpp index e34b6eb..55a7e53 100644 --- a/tests/tests.cpp +++ b/tests/tests.cpp @@ -22,6 +22,7 @@ #if USING_OPENSSL_3 // provider management has changed - ensure tests still work #ifndef USING_TPM2 + #include #else @@ -790,8 +791,8 @@ TEST_F(EvseSecurityTestsExpired, verify_expired_leaf_deletion) { // List of date sorted certificates std::vector sorted; - std::vector sorded_should_delete; - std::vector sorded_should_keep; + std::vector sorted_should_delete; + std::vector sorted_should_keep; // Ensure that we have GEN_CERTIFICATES + 2 (CPO_CERT_CHAIN.pem + SECC_LEAF.pem) { @@ -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()); } } @@ -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(file)), std::istreambuf_iterator()); + + ASSERT_EQ(KeyValidationResult::Valid, CryptoSupplier::x509_check_private_key( + cert.get(), private_key, evse_security->private_key_password)); } } } @@ -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.) \ No newline at end of file