From a91d2d13303e6535fe49abc1a4379ccec9fbdae5 Mon Sep 17 00:00:00 2001 From: Kai Hermann Date: Fri, 29 Sep 2023 13:19:42 +0200 Subject: [PATCH] Add clang-format linter workflow (#4) * Add clang-format linter workflow * clang-format --------- Signed-off-by: Kai-Uwe Hermann --- .github/workflows/lint.yaml | 21 ++++++++++++ include/evse_security.hpp | 8 ++--- include/types.hpp | 15 ++++----- include/x509_wrapper.hpp | 10 +++--- lib/evse_security.cpp | 65 +++++++++++++++++++------------------ lib/x509_wrapper.cpp | 36 ++++++++++---------- tests/tests.cpp | 25 +++++++------- 7 files changed, 98 insertions(+), 82 deletions(-) create mode 100644 .github/workflows/lint.yaml diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml new file mode 100644 index 0000000..862b5a6 --- /dev/null +++ b/.github/workflows/lint.yaml @@ -0,0 +1,21 @@ +name: Lint +on: [workflow_dispatch, pull_request] + +jobs: + lint: + name: Lint + strategy: + matrix: + os: [ubuntu-22.04] + runs-on: ${{ matrix.os }} + steps: + - name: Checkout libocpp + uses: actions/checkout@v3 + with: + path: source + - name: Run clang-format + uses: everest/everest-ci/github-actions/run-clang-format@v1.0.0 + with: + source-dir: source + extensions: hpp,cpp + exclude: cache diff --git a/include/evse_security.hpp b/include/evse_security.hpp index e3ad0e2..5d224ba 100644 --- a/include/evse_security.hpp +++ b/include/evse_security.hpp @@ -76,7 +76,7 @@ class EvseSecurity { /// @brief Retrieves all certificates installed on the filesystem applying the \p certificate_types filter. /// @param certificate_types - /// @return contains the certificate hash data chains of the requested \p certificate_types + /// @return contains the certificate hash data chains of the requested \p certificate_types GetInstalledCertificatesResult get_installed_certificates(const std::vector& certificate_types); /// @brief Retrieves the OCSP request data of the V2G certificates @@ -114,12 +114,12 @@ class EvseSecurity { /// @brief Retrieves the PEM formatted CA bundle file for the given \p certificate_type /// @param certificate_type - /// @return CA certificate file + /// @return CA certificate file std::string get_verify_file(CaCertificateType certificate_type); - + /// @brief Gets the expiry day count for the leaf certificate of the given \p certificate_type /// @param certificate_type - /// @return day count until the leaf certificate expires + /// @return day count until the leaf certificate expires int get_leaf_expiry_days_count(LeafCertificateType certificate_type); private: diff --git a/include/types.hpp b/include/types.hpp index 22d101d..304ee17 100644 --- a/include/types.hpp +++ b/include/types.hpp @@ -87,22 +87,21 @@ struct CertificateHashData { std::string serial_number; ///< The string representation of the hexadecimal value of the serial number without the ///< prefix "0x" and without leading zeroes. - bool operator==(const CertificateHashData &Other) { - return hash_algorithm == Other.hash_algorithm - && issuer_name_hash == Other.issuer_name_hash - && issuer_key_hash == Other.issuer_key_hash - && serial_number == Other.serial_number; + bool operator==(const CertificateHashData& Other) { + return hash_algorithm == Other.hash_algorithm && issuer_name_hash == Other.issuer_name_hash && + issuer_key_hash == Other.issuer_key_hash && serial_number == Other.serial_number; } }; struct CertificateHashDataChain { CertificateType certificate_type; ///< Indicates the type of the certificate for which the hash data is provided - CertificateHashData certificate_hash_data; ///< Contains the hash data of the certificate - std::vector child_certificate_hash_data; ///< Contains the hash data of the child's certificates + CertificateHashData certificate_hash_data; ///< Contains the hash data of the certificate + std::vector + child_certificate_hash_data; ///< Contains the hash data of the child's certificates }; struct GetInstalledCertificatesResult { GetInstalledCertificatesStatus status; ///< Indicates the status of the request std::vector - certificate_hash_data_chain; ///< the hashed certificate data for each requested certificates + certificate_hash_data_chain; ///< the hashed certificate data for each requested certificates }; struct OCSPRequestData { std::optional certificate_hash_data; ///< Contains the hash data of the certificate diff --git a/include/x509_wrapper.hpp b/include/x509_wrapper.hpp index 676d92f..93843ae 100644 --- a/include/x509_wrapper.hpp +++ b/include/x509_wrapper.hpp @@ -22,7 +22,8 @@ class CertificateLoadException : public std::runtime_error { /// @brief Convenience wrapper around openssl X509 certificate. Can contain multiple certificates class X509Wrapper { - using X509_ptr = std::unique_ptr; + using X509_ptr = std::unique_ptr; + public: // Constructors X509Wrapper(const std::string& certificate, const EncodingFormat encoding); @@ -35,7 +36,7 @@ class X509Wrapper { X509Wrapper(const X509Wrapper& other); X509Wrapper(X509Wrapper&& other) = default; - + ~X509Wrapper(); /// @brief Gets raw X509 pointer @@ -96,8 +97,7 @@ class X509Wrapper { /// @brief Gets if this certificate file is containing multiple certificates /// @return - bool is_bundle() - { + bool is_bundle() { return x509.size() > 1; } @@ -109,7 +109,7 @@ class X509Wrapper { std::vector x509; int valid_in; // seconds; if > 0 cert is not yet valid int valid_to; // seconds; if < 0 cert has expired - std::optional path; + std::optional path; }; } // namespace evse_security diff --git a/lib/evse_security.cpp b/lib/evse_security.cpp index aae6ad9..1bfc219 100644 --- a/lib/evse_security.cpp +++ b/lib/evse_security.cpp @@ -182,14 +182,13 @@ static CertificateType get_certificate_type(const CaCertificateType ca_certifica static std::string get_random_file_name(const std::string& extension) { char path[] = "XXXXXX"; mktemp(path); - + return std::string(path) + extension; } - -std::vector get_leaf_certificates(const std::filesystem::path &cert_dir) { +std::vector get_leaf_certificates(const std::filesystem::path& cert_dir) { std::vector certificates; - + for (const auto& entry : std::filesystem::recursive_directory_iterator(cert_dir)) { if (std::filesystem::is_regular_file(entry)) { const auto cert_path = entry.path(); @@ -212,10 +211,10 @@ std::vector get_leaf_certificates(const std::filesystem::path &cert std::vector get_leaf_certificates(std::vector paths) { std::vector certificates; - for(const auto &path : paths) { + for (const auto& path : paths) { auto certifs = get_leaf_certificates(path); - if(certifs.size() > 0) { + if (certifs.size() > 0) { certificates.reserve(certificates.size() + certifs.size()); std::move(std::begin(certifs), std::end(certifs), std::back_inserter(certificates)); } @@ -224,7 +223,8 @@ std::vector get_leaf_certificates(std::vector get_ca_certificates(const std::map &ca_bundle_path_map) { +std::vector +get_ca_certificates(const std::map& ca_bundle_path_map) { // x509 wrapper specific std::vector ca_certificates; for (auto const& [certificate_type, ca_bundle_path] : ca_bundle_path_map) { @@ -232,9 +232,10 @@ std::vector get_ca_certificates(const std::map 0) { + if (certificates_of_bundle.size() > 0) { ca_certificates.reserve(ca_certificates.size() + certificates_of_bundle.size()); - std::move(std::begin(certificates_of_bundle), std::end(certificates_of_bundle), std::back_inserter(ca_certificates)); + std::move(std::begin(certificates_of_bundle), std::end(certificates_of_bundle), + std::back_inserter(ca_certificates)); } } catch (const CertificateLoadException& e) { EVLOG_info << "Could not load ca bundle from file: " << ca_bundle_path; @@ -245,17 +246,18 @@ std::vector get_ca_certificates(const std::map& private_key_password) : private_key_password(private_key_password) { - - std::vector dirs = { file_paths.directories.csms_leaf_cert_directory, - file_paths.directories.csms_leaf_key_directory, - file_paths.directories.secc_leaf_cert_directory, - file_paths.directories.secc_leaf_key_directory, }; - for(const auto &path : dirs) { + std::vector dirs = { + file_paths.directories.csms_leaf_cert_directory, + file_paths.directories.csms_leaf_key_directory, + file_paths.directories.secc_leaf_cert_directory, + file_paths.directories.secc_leaf_key_directory, + }; + + for (const auto& path : dirs) { if (!std::filesystem::exists(path)) { throw std::runtime_error("Could not find configured leaf directory at: " + path.string()); - } - else if(!std::filesystem::is_directory(path)) { + } else if (!std::filesystem::is_directory(path)) { throw std::runtime_error(path.string() + " is not a directory."); } } @@ -265,16 +267,15 @@ EvseSecurity::EvseSecurity(const FilePaths& file_paths, const std::optionalca_bundle_path_map[CaCertificateType::MO] = file_paths.mo_ca_bundle; this->ca_bundle_path_map[CaCertificateType::V2G] = file_paths.v2g_ca_bundle; - for(const auto& pair : this->ca_bundle_path_map) { - if(!std::filesystem::exists(pair.second)) { - throw std::runtime_error("Could not find configured " + std::to_string((int)pair.first) + " bundle file at: " + - file_paths.csms_ca_bundle.string()); - } - else if(std::filesystem::is_directory(pair.second)) { - throw std::runtime_error("Provided bundle " + std::to_string((int)pair.first) + " is directory: " + - file_paths.csms_ca_bundle.string()); + for (const auto& pair : this->ca_bundle_path_map) { + if (!std::filesystem::exists(pair.second)) { + throw std::runtime_error("Could not find configured " + std::to_string((int)pair.first) + + " bundle file at: " + file_paths.csms_ca_bundle.string()); + } else if (std::filesystem::is_directory(pair.second)) { + throw std::runtime_error("Provided bundle " + std::to_string((int)pair.first) + + " is directory: " + file_paths.csms_ca_bundle.string()); } - } + } this->directories = file_paths.directories; } @@ -321,7 +322,8 @@ DeleteCertificateResult EvseSecurity::delete_certificate(const CertificateHashDa } } - const auto leaf_certificates = get_leaf_certificates({directories.secc_leaf_cert_directory, directories.csms_leaf_cert_directory}); + const auto leaf_certificates = + get_leaf_certificates({directories.secc_leaf_cert_directory, directories.csms_leaf_cert_directory}); for (const auto& leaf_certificate : leaf_certificates) { if (leaf_certificate.get_issuer_name_hash() == certificate_hash_data.issuer_name_hash and leaf_certificate.get_issuer_key_hash() == certificate_hash_data.issuer_key_hash and @@ -372,7 +374,7 @@ InstallCertificateResult EvseSecurity::update_leaf_certificate(const std::string return result; } - const auto &leaf_certificate = _certificate_chain[0]; + const auto& leaf_certificate = _certificate_chain[0]; // check if a private key belongs to the provided certificate try { @@ -401,8 +403,7 @@ InstallCertificateResult EvseSecurity::update_leaf_certificate(const std::string return InstallCertificateResult::Accepted; } -GetInstalledCertificatesResult -EvseSecurity::get_installed_certificate(CertificateType certificate_type) { +GetInstalledCertificatesResult EvseSecurity::get_installed_certificate(CertificateType certificate_type) { return get_installed_certificates({certificate_type}); } @@ -667,8 +668,8 @@ int EvseSecurity::get_leaf_expiry_days_count(LeafCertificateType certificate_typ } InstallCertificateResult EvseSecurity::verify_certificate(const std::string& certificate_chain, - LeafCertificateType certificate_type) { - try { + LeafCertificateType certificate_type) { + try { X509Wrapper certificate(certificate_chain, EncodingFormat::PEM); std::vector _certificate_chain = certificate.split(); if (_certificate_chain.empty()) { diff --git a/lib/x509_wrapper.cpp b/lib/x509_wrapper.cpp index acbc723..6deef44 100644 --- a/lib/x509_wrapper.cpp +++ b/lib/x509_wrapper.cpp @@ -8,16 +8,16 @@ #include #include -#include #include +#include namespace evse_security { using BIO_ptr = std::unique_ptr; -std::string x509_to_string(X509 *x509) { +std::string x509_to_string(X509* x509) { if (x509) { - BIO_ptr bio_write(BIO_new(BIO_s_mem()), ::BIO_free); + BIO_ptr bio_write(BIO_new(BIO_s_mem()), ::BIO_free); int rc = PEM_write_bio_X509(bio_write.get(), x509); @@ -32,8 +32,7 @@ std::string x509_to_string(X509 *x509) { return {}; } -void X509Wrapper::update_validity() -{ +void X509Wrapper::update_validity() { // For valid_in and valid_to ASN1_TIME* notBefore = X509_get_notBefore(x509[0].get()); ASN1_TIME* notAfter = X509_get_notAfter(x509[0].get()); @@ -41,7 +40,7 @@ void X509Wrapper::update_validity() ASN1_TIME_diff(&day, &sec, notBefore, nullptr); this->valid_in = day * 86400 + sec; // Convert days to seconds ASN1_TIME_diff(&day, &sec, nullptr, notAfter); - this->valid_to = day * 86400 + sec; // Convert days to seconds + this->valid_to = day * 86400 + sec; // Convert days to seconds } // Load a certificate from a string using the specified encoding. @@ -60,26 +59,26 @@ void X509Wrapper::load_certificate(const std::string& data, const EncodingFormat X509_INFO* xi = sk_X509_INFO_value(allcerts, i); if (xi && xi->x509) { - // Transfer owneship + // Transfer owneship x509.emplace_back(xi->x509, ::X509_free); xi->x509 = nullptr; } } - sk_X509_INFO_pop_free(allcerts, X509_INFO_free); + sk_X509_INFO_pop_free(allcerts, X509_INFO_free); } else { throw CertificateLoadException("Unsupported encoding format"); } } else if (encoding == EncodingFormat::DER) { x509.emplace_back(d2i_X509_bio(bio.get(), nullptr), ::X509_free); - } else { + } else { throw CertificateLoadException("Unsupported encoding format"); } if (!x509.size()) { throw CertificateLoadException("Failed to read X509 from BIO"); } - + update_validity(); } @@ -89,8 +88,8 @@ X509Wrapper::X509Wrapper(const std::string& certificate, const EncodingFormat en X509Wrapper::X509Wrapper(const std::filesystem::path& path, const EncodingFormat encoding) { // TODO: Directory support - if(std::filesystem::is_directory(path)) { - + if (std::filesystem::is_directory(path)) { + } else { std::ifstream file(path, std::ios::binary); std::string certificate((std::istreambuf_iterator(file)), std::istreambuf_iterator()); @@ -105,22 +104,21 @@ X509Wrapper::X509Wrapper(X509* x509) { update_validity(); } -X509Wrapper::X509Wrapper(const X509Wrapper& other) { - for(const auto &cert : other.x509) { - X509 *dup_cert = X509_dup(cert.get()); +X509Wrapper::X509Wrapper(const X509Wrapper& other) { + for (const auto& cert : other.x509) { + X509* dup_cert = X509_dup(cert.get()); x509.emplace_back(dup_cert, ::X509_free); } if (other.path) { path = other.path.value(); } - + valid_in = other.valid_in; valid_to = other.valid_to; } X509Wrapper::~X509Wrapper() { - } X509* X509Wrapper::get() const { @@ -134,9 +132,9 @@ void X509Wrapper::reset(X509* _x509) { std::vector X509Wrapper::split() { std::vector certificates; - for(const auto &cert : x509) { + for (const auto& cert : x509) { // Duplicates since a X509Wrapper requires exclusive ownership - X509 *dup_cert = X509_dup(cert.get()); + X509* dup_cert = X509_dup(cert.get()); certificates.emplace_back(dup_cert); if (this->path.has_value()) { diff --git a/tests/tests.cpp b/tests/tests.cpp index 846f14c..691d43b 100644 --- a/tests/tests.cpp +++ b/tests/tests.cpp @@ -4,14 +4,14 @@ #include #include #include -#include #include +#include #include "evse_security.hpp" #include -#include #include +#include std::string read_file_to_string(const std::filesystem::path filepath) { std::ifstream t(filepath.string()); @@ -20,13 +20,10 @@ std::string read_file_to_string(const std::filesystem::path filepath) { return buffer.str(); } -bool equal_certificate_strings(const std::string &cert1, const std::string &cert2) -{ - for(int i = 0; i < cert1.length(); ++i) - { - if(i < cert1.length() && i < cert2.length()) - { - if(isalnum(cert1[i]) && isalnum(cert2[i]) && cert1[i] != cert2[i]) +bool equal_certificate_strings(const std::string& cert1, const std::string& cert2) { + for (int i = 0; i < cert1.length(); ++i) { + if (i < cert1.length() && i < cert2.length()) { + if (isalnum(cert1[i]) && isalnum(cert2[i]) && cert1[i] != cert2[i]) return false; } } @@ -66,7 +63,7 @@ class EvseSecurityTests : public ::testing::Test { }; TEST_F(EvseSecurityTests, verify_basics) { - const char *bundle_path = "certs/ca/v2g/V2G_CA_BUNDLE.pem"; + const char* bundle_path = "certs/ca/v2g/V2G_CA_BUNDLE.pem"; std::ifstream file(bundle_path, std::ios::binary); std::string certificate_file((std::istreambuf_iterator(file)), std::istreambuf_iterator()); @@ -79,7 +76,7 @@ TEST_F(EvseSecurityTests, verify_basics) { std::smatch match; while (std::regex_search(search_start, certificate_file.cend(), match, cert_regex)) { std::string cert_data = match.str(); - try { + try { certificate_strings.emplace_back(cert_data); } catch (const CertificateLoadException& e) { std::cout << "Could not load single certificate while splitting CA bundle: " << e.what() << std::endl; @@ -94,15 +91,15 @@ TEST_F(EvseSecurityTests, verify_basics) { auto certificates = bundle.split(); ASSERT_TRUE(certificates.size() == 3); - ASSERT_TRUE(certificates[0].get_certificate_hash_data() == bundle.get_certificate_hash_data()); + ASSERT_TRUE(certificates[0].get_certificate_hash_data() == bundle.get_certificate_hash_data()); ASSERT_TRUE(equal_certificate_strings(bundle.get_str(), certificates[0].get_str())); ASSERT_TRUE(equal_certificate_strings(bundle.get_str(), certificate_strings[0])); - for(int i = 0; i < certificate_strings.size(); ++i) { + for (int i = 0; i < certificate_strings.size(); ++i) { X509Wrapper cert(certificate_strings[i], EncodingFormat::PEM); - ASSERT_TRUE(certificates[i].get_certificate_hash_data() == cert.get_certificate_hash_data()); + ASSERT_TRUE(certificates[i].get_certificate_hash_data() == cert.get_certificate_hash_data()); ASSERT_TRUE(equal_certificate_strings(cert.get_str(), certificate_strings[i])); } }