Skip to content

Commit

Permalink
Mg refactor verify certificate (#59)
Browse files Browse the repository at this point in the history 
* Change interface to internal verify_certificate to allow reuse for checking contract certificates
* Improve to install certificate result conversion
* Prevent use of root certificate from the chain but use the one in the bundle
* Update interface of verify_certificate
* Add comment
* Add method for retrieving OCSP data from a ceritificat chain
* Fix return type
* Add way of detecting and excluding root certificate
* Use hierarchy to retrieve OCSP data
* Fix conflict
* Fix Lint
* Interface adaptations for new result
* Removed deprecated provider tests, updated a invalid certificate install test
* Added invalid chain error
---------

Signed-off-by: Menno de Graaf <[email protected]>
Signed-off-by: AssemblyJohn <[email protected]>
Co-authored-by: Menno de Graaf <[email protected]>
Co-authored-by: AssemblyJohn <[email protected]>
  • Loading branch information
@mennodegraaf @AssemblyJohn
3 people authored Mar 15, 2024
1 parent 1418325 commit 5cd5f82
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 168 deletions.
10 changes: 5 additions & 5 deletions include/evse_security/crypto/interface/crypto_supplier.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ 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 CertificateValidationError 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,
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 bool x509_check_private_key(X509Handle* handle, std::string private_key,
Expand Down
10 changes: 0 additions & 10 deletions include/evse_security/crypto/interface/crypto_types.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,6 @@

namespace evse_security {

enum class CertificateValidationError {
NoError,
Expired,
InvalidSignature,
IssuerNotFound,
InvalidLeafSignature,
InvalidChain,
Unknown,
};

enum class CryptoKeyType {
EC_prime256v1, // Default EC. P-256, ~equiv to rsa 3072
EC_secp384r1, // P-384, ~equiv to rsa 7680
Expand Down
10 changes: 5 additions & 5 deletions include/evse_security/crypto/openssl/openssl_supplier.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ 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 CertificateValidationError 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,
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 bool x509_verify_signature(X509Handle* handle, const std::vector<std::byte>& signature,
Expand Down
18 changes: 13 additions & 5 deletions include/evse_security/evse_security.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <everest/timer.hpp>

#include <evse_security/crypto/evse_crypto.hpp>
#include <evse_security/evse_types.hpp>
#include <evse_security/utils/evse_filesystem_types.hpp>

Expand Down Expand Up @@ -86,10 +87,10 @@ class EvseSecurity {
/// @brief Verifies the given \p certificate_chain for the given \p certificate_type against the respective CA
/// certificates for the leaf.
/// @param certificate_chain PEM formatted certificate or certificate chain
/// @param certificate_type type of the leaf certificate
/// @param certificate_type type of the root certificate for which the chain is verified
/// @return result of the operation
InstallCertificateResult verify_certificate(const std::string& certificate_chain,
const LeafCertificateType certificate_type);
CertificateValidationResult verify_certificate(const std::string& certificate_chain,
const LeafCertificateType certificate_type);

/// @brief Verifies the given \p certificate_chain for the given \p certificate_type against the respective CA
/// certificates for the leaf and if valid installs the certificate on the filesystem. Before installing on the
Expand Down Expand Up @@ -117,6 +118,13 @@ class EvseSecurity {
/// @return contains OCSP request data
OCSPRequestDataList get_ocsp_request_data();

/// @brief Retrieves the OCSP request data of the given \p certificate_chain
/// @param certificate_chain PEM formatted certificate or certificate chain
/// @param certificate_type type of the leaf certificate
/// @return contains OCSP request data
OCSPRequestDataList get_ocsp_request_data(const std::string& certificate_chain,
const CaCertificateType certificate_type);

/// @brief Updates the OCSP cache for the given \p certificate_hash_data with the given \p ocsp_response
/// @param certificate_hash_data identifies the certificate for which the \p ocsp_response is specified
/// @param ocsp_response the actual OCSP data
Expand Down Expand Up @@ -189,8 +197,8 @@ class EvseSecurity {

private:
// Internal versions of the functions do not lock the mutex
InstallCertificateResult verify_certificate_internal(const std::string& certificate_chain,
LeafCertificateType certificate_type);
CertificateValidationResult verify_certificate_internal(const std::string& certificate_chain,
LeafCertificateType certificate_type);
GetKeyPairResult get_key_pair_internal(LeafCertificateType certificate_type, EncodingFormat encoding);

/// @brief Determines if the total filesize of certificates is > than the max_filesystem_usage bytes
Expand Down
15 changes: 12 additions & 3 deletions include/evse_security/evse_types.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ enum class CaCertificateType {
enum class LeafCertificateType {
CSMS,
V2G,
MF
MF,
MO,
};

enum class CertificateType {
Expand All @@ -42,7 +43,16 @@ enum class HashAlgorithm {
SHA512,
};

// the following 3 enum classes should go into evse_security
enum class CertificateValidationResult {
Valid,
Expired,
InvalidSignature,
IssuerNotFound,
InvalidLeafSignature,
InvalidChain,
Unknown,
};

enum class InstallCertificateResult {
InvalidSignature,
InvalidCertificateChain,
Expand All @@ -61,7 +71,6 @@ enum class DeleteCertificateResult {
NotFound,
};

// why Status here and not Result as before?
enum class GetInstalledCertificatesStatus {
Accepted,
NotFound,
Expand Down
4 changes: 2 additions & 2 deletions lib/evse_security/crypto/interface/crypto_supplier.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ static X509Handle_ptr x509_duplicate_unique(X509Handle* handle) {
default_crypto_supplier_usage_error() return {};
}

CertificateValidationError AbstractCryptoSupplier::x509_verify_certificate_chain(
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) {
default_crypto_supplier_usage_error() return CertificateValidationError::Unknown;
default_crypto_supplier_usage_error() return CertificateValidationResult::Unknown;
}

bool AbstractCryptoSupplier::x509_check_private_key(X509Handle* handle, std::string private_key,
Expand Down
33 changes: 18 additions & 15 deletions lib/evse_security/crypto/openssl/openssl_supplier.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,22 @@ static EVP_PKEY* get(KeyHandle* handle) {
return nullptr;
}

static CertificateValidationError to_certificate_error(const int ec) {
static CertificateValidationResult to_certificate_error(const int ec) {
switch (ec) {
case X509_V_ERR_CERT_HAS_EXPIRED:
return CertificateValidationError::Expired;
return CertificateValidationResult::Expired;
case X509_V_ERR_CERT_SIGNATURE_FAILURE:
return CertificateValidationError::InvalidSignature;
return CertificateValidationResult::InvalidSignature;
case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT:
return CertificateValidationError::IssuerNotFound;
return CertificateValidationResult::IssuerNotFound;
case X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE:
return CertificateValidationError::InvalidLeafSignature;
return CertificateValidationResult::InvalidLeafSignature;
case X509_V_ERR_CERT_CHAIN_TOO_LONG:
case X509_V_ERR_CERT_UNTRUSTED:
return CertificateValidationResult::InvalidChain;
default:
EVLOG_warning << X509_verify_cert_error_string(ec);
return CertificateValidationError::Unknown;
return CertificateValidationResult::Unknown;
}
}

Expand Down Expand Up @@ -556,11 +559,11 @@ X509Handle_ptr OpenSSLSupplier::x509_duplicate_unique(X509Handle* handle) {
return std::make_unique<X509HandleOpenSSL>(X509_dup(get(handle)));
}

CertificateValidationError 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,
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());
Expand All @@ -575,12 +578,12 @@ CertificateValidationError OpenSSLSupplier::x509_verify_certificate_chain(X509Ha
const char* c_file_path = file_path.has_value() ? file_path.value().c_str() : nullptr;

if (1 != X509_STORE_load_locations(store_ptr.get(), c_file_path, c_dir_path)) {
return CertificateValidationError::Unknown;
return CertificateValidationResult::Unknown;
}

if (dir_path.has_value()) {
if (X509_STORE_add_lookup(store_ptr.get(), X509_LOOKUP_file()) == nullptr) {
return CertificateValidationError::Unknown;
return CertificateValidationResult::Unknown;
}
}
}
Expand All @@ -593,7 +596,7 @@ CertificateValidationError OpenSSLSupplier::x509_verify_certificate_chain(X509Ha
ASN1_TIME_diff(&day, &sec, nullptr, X509_get_notAfter(get(target)));
if (day < 0 || sec < 0) {
// certificate is expired
return CertificateValidationError::Expired;
return CertificateValidationResult::Expired;
}
// certificate is not expired, but may not be valid yet. Since we allow future certs, disable time checks.
X509_STORE_CTX_set_flags(store_ctx_ptr.get(), X509_V_FLAG_NO_CHECK_TIME);
Expand All @@ -606,7 +609,7 @@ CertificateValidationError OpenSSLSupplier::x509_verify_certificate_chain(X509Ha
return to_certificate_error(ec);
}

return CertificateValidationError::NoError;
return CertificateValidationResult::Valid;
}

bool OpenSSLSupplier::x509_check_private_key(X509Handle* handle, std::string private_key,
Expand Down
Loading

0 comments on commit 5cd5f82

Please sign in to comment.