Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When veryfing a certificate the intermediary subcas #71

Merged
merged 1 commit into from
May 2, 2024
Merged

When veryfing a certificate the intermediary subcas #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions include/evse_security/crypto/interface/crypto_supplier.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,10 @@ class AbstractCryptoSupplier {
/// @param dir_path Optional directory path that can be used for certificate store lookup
/// @param file_path Optional certificate file path that can be used for certificate store lookup
/// @return
static CertificateValidationResult x509_verify_certificate_chain(X509Handle* target,
const std::vector<X509Handle*>& parents,
bool allow_future_certificates,
const std::optional<fs::path> dir_path,
const std::optional<fs::path> file_path);
static CertificateValidationResult
x509_verify_certificate_chain(X509Handle* target, const std::vector<X509Handle*>& parents,
const std::vector<X509Handle*>& untrusted_subcas, bool allow_future_certificates,
const std::optional<fs::path> dir_path, const std::optional<fs::path> file_path);

/// @brief Checks if the private key is consistent with the provided handle
static KeyValidationResult x509_check_private_key(X509Handle* handle, std::string private_key,
Expand Down
9 changes: 4 additions & 5 deletions include/evse_security/crypto/openssl/openssl_supplier.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,10 @@ class OpenSSLSupplier : public AbstractCryptoSupplier {
static bool x509_is_child(X509Handle* child, X509Handle* parent);
static bool x509_is_equal(X509Handle* a, X509Handle* b);
static X509Handle_ptr x509_duplicate_unique(X509Handle* handle);
static CertificateValidationResult x509_verify_certificate_chain(X509Handle* target,
const std::vector<X509Handle*>& parents,
bool allow_future_certificates,
const std::optional<fs::path> dir_path,
const std::optional<fs::path> file_path);
static CertificateValidationResult
x509_verify_certificate_chain(X509Handle* target, const std::vector<X509Handle*>& parents,
const std::vector<X509Handle*>& untrusted_subcas, bool allow_future_certificates,
const std::optional<fs::path> dir_path, const std::optional<fs::path> file_path);
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,
Expand Down
10 changes: 10 additions & 0 deletions include/evse_security/detail/openssl/openssl_types.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ template <> class std::default_delete<X509_REQ> {
}
};

template <> class std::default_delete<STACK_OF(X509)> {
public:
void operator()(STACK_OF(X509) * ptr) const {
sk_X509_free(ptr);
james-ctc marked this conversation as resolved.
Show resolved Hide resolved
}
};

template <> class std::default_delete<EVP_PKEY> {
public:
void operator()(EVP_PKEY* ptr) const {
Expand Down Expand Up @@ -87,6 +94,9 @@ namespace evse_security {
using X509_ptr = std::unique_ptr<X509>;
using X509_STORE_ptr = std::unique_ptr<X509_STORE>;
using X509_STORE_CTX_ptr = std::unique_ptr<X509_STORE_CTX>;
// Unsafe since it does not free contained elements, only the stack, the element
// cleanup has to be done manually
using X509_STACK_UNSAFE_ptr = std::unique_ptr<STACK_OF(X509)>;
using X509_REQ_ptr = std::unique_ptr<X509_REQ>;
using EVP_PKEY_ptr = std::unique_ptr<EVP_PKEY>;
using EVP_PKEY_CTX_ptr = std::unique_ptr<EVP_PKEY_CTX>;
Expand Down
4 changes: 2 additions & 2 deletions lib/evse_security/crypto/interface/crypto_supplier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ static X509Handle_ptr x509_duplicate_unique(X509Handle* handle) {
}

CertificateValidationResult AbstractCryptoSupplier::x509_verify_certificate_chain(
X509Handle* target, const std::vector<X509Handle*>& parents, bool allow_future_certificates,
const std::optional<fs::path> dir_path, const std::optional<fs::path> file_path) {
X509Handle* target, const std::vector<X509Handle*>& parents, const std::vector<X509Handle*>& untrusted_subcas,
bool allow_future_certificates, const std::optional<fs::path> dir_path, const std::optional<fs::path> file_path) {
default_crypto_supplier_usage_error() return CertificateValidationResult::Unknown;
}

Expand Down
29 changes: 23 additions & 6 deletions lib/evse_security/crypto/openssl/openssl_supplier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,13 +560,12 @@ X509Handle_ptr OpenSSLSupplier::x509_duplicate_unique(X509Handle* handle) {
return std::make_unique<X509HandleOpenSSL>(X509_dup(get(handle)));
}

CertificateValidationResult OpenSSLSupplier::x509_verify_certificate_chain(X509Handle* target,
const std::vector<X509Handle*>& parents,
bool allow_future_certificates,
const std::optional<fs::path> dir_path,
const std::optional<fs::path> file_path) {
CertificateValidationResult OpenSSLSupplier::x509_verify_certificate_chain(
X509Handle* target, const std::vector<X509Handle*>& parents, const std::vector<X509Handle*>& untrusted_subcas,
bool allow_future_certificates, const std::optional<fs::path> dir_path, const std::optional<fs::path> file_path) {
OpenSSLProvider provider;
provider.set_global_mode(OpenSSLProvider::mode_t::default_provider);

X509_STORE_ptr store_ptr(X509_STORE_new());
X509_STORE_CTX_ptr store_ctx_ptr(X509_STORE_CTX_new());

Expand All @@ -591,7 +590,25 @@ CertificateValidationResult OpenSSLSupplier::x509_verify_certificate_chain(X509H
}
}

X509_STORE_CTX_init(store_ctx_ptr.get(), store_ptr.get(), get(target), NULL);
X509_STACK_UNSAFE_ptr untrusted = nullptr;

// Build potentially untrusted intermediary (subca) certificates
if (false == untrusted_subcas.empty()) {
untrusted = X509_STACK_UNSAFE_ptr(sk_X509_new_null());
int flags = X509_ADD_FLAG_NO_DUP | X509_ADD_FLAG_NO_SS;

for (auto& untrusted_cert : untrusted_subcas) {
if (1 != X509_add_cert(untrusted.get(), get(untrusted_cert), flags)) {
EVLOG_error << "X509 could not create untrusted store stack!";
return CertificateValidationResult::Unknown;
}
}
}

if (1 != X509_STORE_CTX_init(store_ctx_ptr.get(), store_ptr.get(), get(target), untrusted.get())) {
EVLOG_error << "X509 could not init x509 store ctx!";
return CertificateValidationResult::Unknown;
}

if (allow_future_certificates) {
// Manually check if cert is expired
Expand Down
35 changes: 22 additions & 13 deletions lib/evse_security/evse_security.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1261,27 +1261,35 @@ CertificateValidationResult EvseSecurity::verify_certificate_internal(const std:

try {
X509CertificateBundle certificate(certificate_chain, EncodingFormat::PEM);

std::vector<X509Wrapper> _certificate_chain = certificate.split();
if (_certificate_chain.empty()) {
return CertificateValidationResult::Unknown;
}

// The leaf is to be verified
const auto leaf_certificate = _certificate_chain.at(0);
std::vector<X509Handle*> parent_certificates;

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

// Make sure that an added root certificate is excluded and taken from the bundle
for (size_t i = 1; i < _certificate_chain.size(); i++) {
const auto& cert = _certificate_chain[i];
if (hierarchy.is_root(cert)) {
EVLOG_warning << "Ignore root certificate: " << cert.get_common_name();
} else {
parent_certificates.emplace_back(cert.get());
// 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)) {
EVLOG_warning << "Ignore root certificate: " << cert.get_common_name();
} else {
untrusted_subcas.emplace_back(cert.get());
}
}
}

// Build the trusted parent certificates from our internal store
std::vector<X509Handle*> trusted_parent_certificates;

fs::path root_store = this->ca_bundle_path_map.at(ca_certificate_type);
CertificateValidationResult validated{};

Expand All @@ -1296,16 +1304,17 @@ CertificateValidationResult EvseSecurity::verify_certificate_internal(const std:
std::vector<X509Wrapper> root_chain{roots.split()};

for (size_t i = 0; i < root_chain.size(); i++) {
parent_certificates.emplace_back(root_chain[i].get());
trusted_parent_certificates.emplace_back(root_chain[i].get());
}

// The root_chain stores the X509Handler pointers, if this goes out of scope then
// parent_certificates will point to nothing.
validated = CryptoSupplier::x509_verify_certificate_chain(leaf_certificate.get(), parent_certificates, true,
std::nullopt, std::nullopt);
validated =
CryptoSupplier::x509_verify_certificate_chain(leaf_certificate.get(), trusted_parent_certificates,
untrusted_subcas, true, std::nullopt, std::nullopt);
} else {
validated = CryptoSupplier::x509_verify_certificate_chain(leaf_certificate.get(), parent_certificates, true,
std::nullopt, root_store);
validated = CryptoSupplier::x509_verify_certificate_chain(
leaf_certificate.get(), trusted_parent_certificates, untrusted_subcas, true, std::nullopt, root_store);
}

return validated;
Expand Down
5 changes: 3 additions & 2 deletions tests/openssl_supplier_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,14 @@ TEST_F(OpenSSLSupplierTest, x509_verify_certificate_chain) {
auto res_leaf = OpenSSLSupplier::load_certificates(cert_leaf, EncodingFormat::PEM);

std::vector<X509Handle*> parents;
std::vector<X509Handle*> empty_untrusted;

for (auto& i : res_path) {
parents.push_back(i.get());
}

auto res = OpenSSLSupplier::x509_verify_certificate_chain(res_leaf[0].get(), parents, true, std::nullopt,
"pki/root_cert.pem");
auto res = OpenSSLSupplier::x509_verify_certificate_chain(res_leaf[0].get(), parents, empty_untrusted, true,
std::nullopt, "pki/root_cert.pem");
ASSERT_EQ(res, CertificateValidationResult::Valid);
}

Expand Down
Loading