Implemented code comments

Signed-off-by: AssemblyJohn <[email protected]>
EVerest · May 7, 2024 · e68e61f · e68e61f
1 parent 3055563
commit e68e61f
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 29 deletions.
diff --git a/include/ocpp/common/evse_security.hpp b/include/ocpp/common/evse_security.hpp
@@ -98,8 +98,8 @@ class EvseSecurity {
     /// \param encoding specifies PEM or DER format
     /// \param include_ocsp if we should include certificate ocsp data
     /// \return info of certificate and key if present, else std::nullopt
-    virtual std::optional<CertificateInfo> get_leaf_certificate_info(const CertificateSigningUseEnum& certificate_type,
-                                                                     bool include_ocsp = false) = 0;
+    virtual GetCertificateInfoResult get_leaf_certificate_info(const CertificateSigningUseEnum& certificate_type,
+                                                               bool include_ocsp = false) = 0;
 
     /// \brief Updates the certificate and key links for the given \p certificate_type
     virtual bool update_certificate_links(const CertificateSigningUseEnum& certificate_type) = 0;

diff --git a/include/ocpp/common/evse_security_impl.hpp b/include/ocpp/common/evse_security_impl.hpp
@@ -52,8 +52,8 @@ class EvseSecurityImpl : public EvseSecurity {
     generate_certificate_signing_request(const CertificateSigningUseEnum& certificate_type, const std::string& country,
                                          const std::string& organization, const std::string& common,
                                          bool use_tpm) override;
-    std::optional<CertificateInfo> get_leaf_certificate_info(const CertificateSigningUseEnum& certificate_type,
-                                                             bool include_ocsp = false) override;
+    GetCertificateInfoResult get_leaf_certificate_info(const CertificateSigningUseEnum& certificate_type,
+                                                       bool include_ocsp = false) override;
     bool update_certificate_links(const CertificateSigningUseEnum& certificate_type) override;
     std::string get_verify_file(const CaCertificateType& certificate_type) override;
     int get_leaf_expiry_days_count(const CertificateSigningUseEnum& certificate_type) override;
@@ -66,6 +66,7 @@ CaCertificateType to_ocpp(evse_security::CaCertificateType other);
 CertificateSigningUseEnum to_ocpp(evse_security::LeafCertificateType other);
 CertificateType to_ocpp(evse_security::CertificateType other);
 HashAlgorithmEnumType to_ocpp(evse_security::HashAlgorithm other);
+GetCertificateInfoStatus to_ocpp(evse_security::GetCertificateInfoStatus other);
 InstallCertificateResult to_ocpp(evse_security::InstallCertificateResult other);
 CertificateValidationResult to_ocpp(evse_security::CertificateValidationResult other);
 DeleteCertificateResult to_ocpp(evse_security::DeleteCertificateResult other);

diff --git a/include/ocpp/common/types.hpp b/include/ocpp/common/types.hpp
@@ -538,6 +538,14 @@ enum class GetCertificateSignRequestStatus {
     GenerationError,      ///< Any other error when creating the CSR
 };
 
+enum class GetCertificateInfoStatus {
+    Accepted,
+    Rejected,
+    NotFound,
+    NotFoundValid,
+    PrivateKeyNotFound,
+};
+
 struct GetCertificateSignRequestResult {
     GetCertificateSignRequestStatus status;
     std::optional<std::string> csr;
@@ -557,6 +565,11 @@ struct CertificateInfo {
     std::vector<CertificateOCSP> ocsp;               // OCSP data if requested
 };
 
+struct GetCertificateInfoResult {
+    GetCertificateInfoStatus status;
+    std::optional<CertificateInfo> info;
+};
+
 enum class LeafCertificateType {
     CSMS, // Charging Station Management System
     V2G,  // Vehicle to grid

diff --git a/lib/ocpp/common/evse_security_impl.cpp b/lib/ocpp/common/evse_security_impl.cpp
@@ -104,25 +104,26 @@ EvseSecurityImpl::generate_certificate_signing_request(const CertificateSigningU
         conversions::from_ocpp(certificate_type), country, organization, common, use_tpm);
 
     GetCertificateSignRequestResult result;
-    result.status = conversions::to_ocpp(csr_response.status);
 
-    if (csr_response.csr.has_value()) {
-        result.csr = csr_response.csr;
-    }
+    result.status = conversions::to_ocpp(csr_response.status);
+    result.csr = csr_response.csr;
 
     return result;
 }
 
-std::optional<CertificateInfo>
-EvseSecurityImpl::get_leaf_certificate_info(const CertificateSigningUseEnum& certificate_type, bool include_ocsp) {
+GetCertificateInfoResult EvseSecurityImpl::get_leaf_certificate_info(const CertificateSigningUseEnum& certificate_type,
+                                                                     bool include_ocsp) {
     const auto info_response = this->evse_security->get_leaf_certificate_info(
         conversions::from_ocpp(certificate_type), evse_security::EncodingFormat::PEM, include_ocsp);
 
-    if (info_response.status == evse_security::GetCertificateInfoStatus::Accepted && info_response.info.has_value()) {
-        return conversions::to_ocpp(info_response.info.value());
-    } else {
-        return std::nullopt;
+    GetCertificateInfoResult result;
+
+    result.status = conversions::to_ocpp(info_response.status);
+    if (info_response.info.has_value()) {
+        result.info = conversions::to_ocpp(info_response.info.value());
     }
+
+    return result;
 }
 
 bool EvseSecurityImpl::update_certificate_links(const CertificateSigningUseEnum& certificate_type) {
@@ -213,6 +214,24 @@ HashAlgorithmEnumType to_ocpp(evse_security::HashAlgorithm other) {
     }
 }
 
+GetCertificateInfoStatus to_ocpp(evse_security::GetCertificateInfoStatus other) {
+    switch (other) {
+    case evse_security::GetCertificateInfoStatus::Accepted:
+        return GetCertificateInfoStatus::Accepted;
+    case evse_security::GetCertificateInfoStatus::Rejected:
+        return GetCertificateInfoStatus::Rejected;
+    case evse_security::GetCertificateInfoStatus::NotFound:
+        return GetCertificateInfoStatus::NotFound;
+    case evse_security::GetCertificateInfoStatus::NotFoundValid:
+        return GetCertificateInfoStatus::NotFoundValid;
+    case evse_security::GetCertificateInfoStatus::PrivateKeyNotFound:
+        return GetCertificateInfoStatus::PrivateKeyNotFound;
+    default:
+        throw std::runtime_error(
+            "Could not convert evse_security::GetCertificateInfoStatus to GetCertificateInfoStatus");
+    }
+}
+
 InstallCertificateResult to_ocpp(evse_security::InstallCertificateResult other) {
     switch (other) {
     case evse_security::InstallCertificateResult::InvalidSignature:

diff --git a/lib/ocpp/common/websocket/websocket_libwebsockets.cpp b/lib/ocpp/common/websocket/websocket_libwebsockets.cpp
@@ -449,22 +449,28 @@ void WebsocketTlsTPM::client_loop() {
 
         if (this->connection_options.security_profile == 3) {
 
-            const auto certificate_info =
+            const auto certificate_response =
                 this->evse_security->get_leaf_certificate_info(CertificateSigningUseEnum::ChargingStationCertificate);
 
-            if (!certificate_info.has_value()) {
+            if (certificate_response.status != ocpp::GetCertificateInfoStatus::Accepted &&
+                !certificate_response.info.has_value()) {
                 EVLOG_AND_THROW(std::runtime_error(
                     "Connecting with security profile 3 but no client side certificate is present or valid"));
             }
 
-            if (certificate_info.value().certificate_path.has_value()) {
-                path_chain = certificate_info.value().certificate_path.value();
+            const auto& certificate_info = certificate_response.info.value();
+
+            if (certificate_info.certificate_path.has_value()) {
+                path_chain = certificate_info.certificate_path.value();
+            } else if (certificate_info.certificate_single_path.has_value()) {
+                path_chain = certificate_info.certificate_single_path.value();
             } else {
-                path_chain = certificate_info.value().certificate_single_path.value();
+                EVLOG_AND_THROW(std::runtime_error(
+                    "Connecting with security profile 3 but no client side certificate is present or valid"));
             }
 
-            path_key = certificate_info.value().key_path;
-            password = certificate_info.value().password;
+            path_key = certificate_info.key_path;
+            password = certificate_info.password;
         }
 
         SSL_CTX* ssl_ctx = nullptr;

diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp
@@ -1426,10 +1426,10 @@ void ChargePointImpl::handleChangeConfigurationRequest(ocpp::Call<ChangeConfigur
                                 << "New security level set to 2 or 3 but no CentralSystemRootCertificateInstalled";
                             response.status = ConfigurationStatus::Rejected;
                         } else if (security_profile == 3 &&
-                                   !this->evse_security
-                                        ->get_leaf_certificate_info(
-                                            ocpp::CertificateSigningUseEnum::ChargingStationCertificate)
-                                        .has_value()) {
+                                   this->evse_security
+                                           ->get_leaf_certificate_info(
+                                               ocpp::CertificateSigningUseEnum::ChargingStationCertificate)
+                                           .status != ocpp::GetCertificateInfoStatus::Accepted) {
                             EVLOG_warning << "New security level set to 3 but no Client Certificate is installed";
                             response.status = ConfigurationStatus::Rejected;
                         } else if (security_profile > 3) {

diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp
@@ -1623,9 +1623,9 @@ bool ChargePoint::validate_set_variable(const SetVariableData& set_variable_data
                 }
 
                 if (network_profile.securityProfile == 3 and
-                    !this->evse_security
-                         ->get_leaf_certificate_info(ocpp::CertificateSigningUseEnum::ChargingStationCertificate)
-                         .has_value()) {
+                    this->evse_security
+                            ->get_leaf_certificate_info(ocpp::CertificateSigningUseEnum::ChargingStationCertificate)
+                            .status != ocpp::GetCertificateInfoStatus::Accepted) {
                     EVLOG_warning << "SecurityProfile of configurationSlot: " << configuration_slot
                                   << " is 3 but no CSMS Leaf Certificate is installed";
                     return false;

diff --git a/tests/evse_security_mock.hpp b/tests/evse_security_mock.hpp
@@ -29,7 +29,7 @@ class EvseSecurityMock : public EvseSecurity {
     MOCK_METHOD(GetCertificateSignRequestResult, generate_certificate_signing_request,
                 (const CertificateSigningUseEnum&, const std::string&, const std::string&, const std::string&, bool),
                 (override));
-    MOCK_METHOD(std::optional<CertificateInfo>, get_leaf_certificate_info, (const CertificateSigningUseEnum&, bool),
+    MOCK_METHOD(GetCertificateInfoResult, get_leaf_certificate_info, (const CertificateSigningUseEnum&, bool),
                 (override));
     MOCK_METHOD(bool, update_certificate_links, (const CertificateSigningUseEnum&), (override));
     MOCK_METHOD(std::string, get_verify_file, (const CaCertificateType&), (override));