Skip to content

Commit

Permalink
Requirements update (#95)
Browse files Browse the repository at this point in the history
* Fixed a function name typo
* Added case insensitive certificate deletion for better compatibility
* Ignore any roots received in the intermediate chain for a certificate update
---------

Signed-off-by: AssemblyJohn <[email protected]>
  • Loading branch information
AssemblyJohn authored Nov 13, 2024
1 parent 049d691 commit 46f0187
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 29 deletions.
4 changes: 2 additions & 2 deletions include/evse_security/certificate/x509_bundle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class X509CertificateBundle {
bool contains_certificate(const CertificateHashData& certificate_hash);

/// @brief Finds a certificate based on its hash
X509Wrapper find_certificate(const CertificateHashData& certificate_hash);
X509Wrapper find_certificate(const CertificateHashData& certificate_hash, bool case_insensitive_comparison = false);

/// @brief Adds a single certificate in the bundle. Only in memory, use @ref export_certificates to filesystem
void add_certificate(X509Wrapper&& certificate);
Expand Down Expand Up @@ -169,7 +169,7 @@ class X509CertificateBundle {

/// @brief Utility that returns current the certificate hierarchy of this bundle
/// Invalidated on any add/delete operation
X509CertificateHierarchy& get_certficate_hierarchy();
X509CertificateHierarchy& get_certificate_hierarchy();

public:
X509CertificateBundle& operator=(X509CertificateBundle&& other) = default;
Expand Down
4 changes: 2 additions & 2 deletions include/evse_security/certificate/x509_hierarchy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class X509CertificateHierarchy {

/// @brief Checks if the provided certificate is a self-signed root CA certificate
/// contained in our hierarchy
bool is_root(const X509Wrapper& certificate) const;
bool is_internal_root(const X509Wrapper& certificate) const;

/// @brief Collects all the descendants of the provided certificate
/// @param top Certificate that issued the descendants
Expand All @@ -62,7 +62,7 @@ class X509CertificateHierarchy {
X509Wrapper find_certificate_root(const X509Wrapper& leaf);

/// @brief Searches for the provided hash, throwing a NoCertificateFound if not found
X509Wrapper find_certificate(const CertificateHashData& hash);
X509Wrapper find_certificate(const CertificateHashData& hash, bool case_insensitive_comparison = false);

/// @brief Searches for all the certificates with the provided hash, throwing a NoCertificateFound
// if none were found. Can be useful when we have SUB-CAs in multiple bundles
Expand Down
32 changes: 32 additions & 0 deletions include/evse_security/evse_types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Copyright Pionix GmbH and Contributors to EVerest
#pragma once

#include <algorithm>
#include <cctype>
#include <optional>
#include <string>
#include <vector>
Expand Down Expand Up @@ -97,6 +99,16 @@ enum class GetCertificateSignRequestStatus {
GenerationError, ///< Any other error when creating the CSR
};

inline bool str_cast_insensitive_cmp(const std::string& a, const std::string& b) {
if (a.size() != b.size()) {
return false;
}

return std::equal(a.begin(), a.end(), b.begin(), b.end(), [](std::string::value_type a, std::string::value_type b) {
return (std::tolower(a) == std::tolower(b));
});
}

// types of evse_security

struct CertificateHashData {
Expand All @@ -113,6 +125,26 @@ struct CertificateHashData {
issuer_key_hash == Other.issuer_key_hash && serial_number == Other.serial_number;
}

bool case_insensitive_comparison(const CertificateHashData& Other) const {
if (hash_algorithm != Other.hash_algorithm) {
return false;
}

if (false == str_cast_insensitive_cmp(issuer_name_hash, Other.issuer_name_hash)) {
return false;
}

if (false == str_cast_insensitive_cmp(issuer_key_hash, Other.issuer_key_hash)) {
return false;
}

if (false == str_cast_insensitive_cmp(serial_number, Other.serial_number)) {
return false;
}

return true;
}

bool is_valid() {
return (false == issuer_name_hash.empty()) && (false == issuer_key_hash.empty()) &&
(false == serial_number.empty());
Expand Down
32 changes: 22 additions & 10 deletions lib/evse_security/certificate/x509_bundle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,31 +132,43 @@ bool X509CertificateBundle::contains_certificate(const CertificateHashData& cert
}

// Nothing found, build the hierarchy and search by the issued hash
X509CertificateHierarchy& hierarchy = get_certficate_hierarchy();
X509CertificateHierarchy& hierarchy = get_certificate_hierarchy();
return hierarchy.contains_certificate_hash(certificate_hash);
}

X509Wrapper X509CertificateBundle::find_certificate(const CertificateHashData& certificate_hash) {
X509Wrapper X509CertificateBundle::find_certificate(const CertificateHashData& certificate_hash,
bool case_insensitive_comparison) {
// Try an initial search for root certificates, else a hierarchy build will be required
for (const auto& chain : certificates) {
for (const auto& certif : chain.second) {
if (certif.is_selfsigned() && certif == certificate_hash) {
return certif;
if (certif.is_selfsigned()) {
bool matches = false;

if (case_insensitive_comparison) {
CertificateHashData certif_hash = certif.get_certificate_hash_data();
matches = certif_hash.case_insensitive_comparison(certificate_hash);
} else {
matches = (certif == certificate_hash);
}

if (matches) {
return certif;
}
}
}
}

// Nothing found, build the hierarchy and search by the issued hash
X509CertificateHierarchy& hierarchy = get_certficate_hierarchy();
return hierarchy.find_certificate(certificate_hash);
X509CertificateHierarchy& hierarchy = get_certificate_hierarchy();
return hierarchy.find_certificate(certificate_hash, case_insensitive_comparison);
}

int X509CertificateBundle::delete_certificate(const X509Wrapper& certificate, bool include_issued) {
std::vector<X509Wrapper> to_delete;

if (include_issued) {
// Include all descendants in the delete list
auto& hierarchy = get_certficate_hierarchy();
auto& hierarchy = get_certificate_hierarchy();
to_delete = hierarchy.collect_descendants(certificate);
}

Expand Down Expand Up @@ -189,11 +201,11 @@ int X509CertificateBundle::delete_certificate(const X509Wrapper& certificate, bo
}

int X509CertificateBundle::delete_certificate(const CertificateHashData& data, bool include_issued) {
auto& hierarchy = get_certficate_hierarchy();
auto& hierarchy = get_certificate_hierarchy();

try {
// Try to find the certificate by correct hierarchy hash
X509Wrapper to_delete = hierarchy.find_certificate(data);
X509Wrapper to_delete = hierarchy.find_certificate(data, true /* = Case insensitive search */);
return delete_certificate(to_delete, include_issued);
} catch (NoCertificateFound& e) {
}
Expand Down Expand Up @@ -344,7 +356,7 @@ void X509CertificateBundle::invalidate_hierarchy() {
hierarchy_invalidated = true;
}

X509CertificateHierarchy& X509CertificateBundle::get_certficate_hierarchy() {
X509CertificateHierarchy& X509CertificateBundle::get_certificate_hierarchy() {
if (hierarchy_invalidated) {
EVLOG_info << "Building new certificate hierarchy!";
hierarchy_invalidated = false;
Expand Down
15 changes: 12 additions & 3 deletions lib/evse_security/certificate/x509_hierarchy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace evse_security {

bool X509CertificateHierarchy::is_root(const X509Wrapper& certificate) const {
bool X509CertificateHierarchy::is_internal_root(const X509Wrapper& certificate) const {
if (certificate.is_selfsigned()) {
return (std::find_if(hierarchy.begin(), hierarchy.end(), [&certificate](const X509Node& node) {
return node.certificate == certificate;
Expand Down Expand Up @@ -101,11 +101,20 @@ X509Wrapper X509CertificateHierarchy::find_certificate_root(const X509Wrapper& l
throw NoCertificateFound("Could not find a certificate root for leaf: " + leaf.get_common_name());
}

X509Wrapper X509CertificateHierarchy::find_certificate(const CertificateHashData& hash) {
X509Wrapper X509CertificateHierarchy::find_certificate(const CertificateHashData& hash,
bool case_insensitive_comparison) {
X509Wrapper* certificate = nullptr;

for_each([&](X509Node& node) {
if (node.hash == hash) {
bool matches = false;

if (case_insensitive_comparison) {
matches = (node.hash.case_insensitive_comparison(hash));
} else {
matches = (node.hash == hash);
}

if (matches) {
certificate = &node.certificate;
return false;
}
Expand Down
16 changes: 9 additions & 7 deletions lib/evse_security/evse_security.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,8 @@ DeleteCertificateResult EvseSecurity::delete_certificate(const CertificateHashDa
<< hierarchy.to_debug_string();

try {
X509Wrapper to_delete = hierarchy.find_certificate(certificate_hash_data);
X509Wrapper to_delete =
hierarchy.find_certificate(certificate_hash_data, true /* case-insensitive search */);

if (leaf_bundle.delete_certificate(to_delete, true)) {
found_certificate = true;
Expand Down Expand Up @@ -563,7 +564,7 @@ EvseSecurity::get_installed_certificates(const std::vector<CertificateType>& cer
auto ca_bundle_path = this->ca_bundle_path_map.at(ca_certificate_type);
try {
X509CertificateBundle ca_bundle(ca_bundle_path, EncodingFormat::PEM);
X509CertificateHierarchy& hierarchy = ca_bundle.get_certficate_hierarchy();
X509CertificateHierarchy& hierarchy = ca_bundle.get_certificate_hierarchy();

EVLOG_debug << "Hierarchy:(" << conversions::ca_certificate_type_to_string(ca_certificate_type) << ")\n"
<< hierarchy.to_debug_string();
Expand Down Expand Up @@ -620,7 +621,7 @@ EvseSecurity::get_installed_certificates(const std::vector<CertificateType>& cer
}

// Create the certificate hierarchy
X509CertificateHierarchy& hierarchy = ca_bundle.get_certficate_hierarchy();
X509CertificateHierarchy& hierarchy = ca_bundle.get_certificate_hierarchy();
EVLOG_debug << "Hierarchy:(V2GCertificateChain)\n" << hierarchy.to_debug_string();

for (auto& root : hierarchy.get_hierarchy()) {
Expand Down Expand Up @@ -970,7 +971,7 @@ bool EvseSecurity::is_ca_certificate_installed_internal(CaCertificateType certif
X509CertificateBundle bundle(this->ca_bundle_path_map.at(certificate_type), EncodingFormat::PEM);

// Search for a valid self-signed root
auto& hierarchy = bundle.get_certficate_hierarchy();
auto& hierarchy = bundle.get_certificate_hierarchy();

// Get all roots and search for a valid self-signed
for (auto& root : hierarchy.get_hierarchy()) {
Expand Down Expand Up @@ -1499,7 +1500,7 @@ GetCertificateInfoResult EvseSecurity::get_ca_certificate_info_internal(CaCertif

// If we are using a directory, search for the first valid root file
if (verify_file.is_using_directory()) {
auto& hierarchy = verify_file.get_certficate_hierarchy();
auto& hierarchy = verify_file.get_certificate_hierarchy();

// Get all roots and search for a valid self-signed
for (auto& root : hierarchy.get_hierarchy()) {
Expand Down Expand Up @@ -1708,15 +1709,16 @@ CertificateValidationResult EvseSecurity::verify_certificate_internal(const std:
const auto leaf_certificate = _certificate_chain.at(0);

// Retrieve the hierarchy in order to check if the chain contains a root certificate
X509CertificateHierarchy& hierarchy = certificate.get_certficate_hierarchy();
X509CertificateHierarchy& hierarchy = certificate.get_certificate_hierarchy();

// Build all untrusted intermediary certificates, and exclude any root
std::vector<X509Handle*> untrusted_subcas;

if (_certificate_chain.size() > 1) {
for (size_t i = 1; i < _certificate_chain.size(); i++) {
const auto& cert = _certificate_chain[i];
if (hierarchy.is_root(cert)) {
// Ignore the received certificate is somehow self-signed
if (cert.is_selfsigned()) {
EVLOG_warning << "Ignore root certificate: " << cert.get_common_name();
} else {
untrusted_subcas.emplace_back(cert.get());
Expand Down
10 changes: 5 additions & 5 deletions tests/tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ TEST_F(EvseSecurityTests, verify_basics) {
X509CertificateBundle bundle(fs::path(bundle_path), EncodingFormat::PEM);
ASSERT_TRUE(bundle.is_using_bundle_file());

std::cout << "Bundle hierarchy: " << std::endl << bundle.get_certficate_hierarchy().to_debug_string();
std::cout << "Bundle hierarchy: " << std::endl << bundle.get_certificate_hierarchy().to_debug_string();

auto certificates = bundle.split();
ASSERT_TRUE(certificates.size() == 3);
Expand Down Expand Up @@ -246,18 +246,18 @@ TEST_F(EvseSecurityTests, verify_bundle_management) {
X509CertificateBundle bundle(fs::path(directory_path), EncodingFormat::PEM);
ASSERT_TRUE(bundle.split().size() == 2);

std::cout << "Bundle hierarchy: " << std::endl << bundle.get_certficate_hierarchy().to_debug_string();
std::cout << "Bundle hierarchy: " << std::endl << bundle.get_certificate_hierarchy().to_debug_string();

// Lowest in hierarchy
X509Wrapper intermediate_cert = bundle.get_certficate_hierarchy().get_hierarchy().at(0).children.at(0).certificate;
X509Wrapper intermediate_cert = bundle.get_certificate_hierarchy().get_hierarchy().at(0).children.at(0).certificate;

CertificateHashData hash = bundle.get_certficate_hierarchy().get_certificate_hash(intermediate_cert);
CertificateHashData hash = bundle.get_certificate_hierarchy().get_certificate_hash(intermediate_cert);
bundle.delete_certificate(hash, true);

// Sync deleted
bundle.sync_to_certificate_store();

std::cout << "Deleted intermediate: " << std::endl << bundle.get_certficate_hierarchy().to_debug_string();
std::cout << "Deleted intermediate: " << std::endl << bundle.get_certificate_hierarchy().to_debug_string();

int items = 0;
for (const auto& entry : fs::recursive_directory_iterator(directory_path)) {
Expand Down

0 comments on commit 46f0187

Please sign in to comment.