From 980c3918add1cabc88e6a6cf32e436a9c29a7e17 Mon Sep 17 00:00:00 2001 From: aw Date: Sat, 9 Nov 2024 11:46:53 +0100 Subject: [PATCH 1/3] feature(codec): improvement on PnC related messages - variant for `authorization_mode` in `AuthorizationReq` has been introduced - sdp packet buffer size has been increased to 4k, in order to be large enough to contain sample data from josev - crucial bug has been fixed in `ServiceDetail` serialization, where the `ParameterSet` has not been initialized/zero-initialized properly -- this will need some follow up discussion - some PnC related message and state handling has been added/refactored - minor refactorings on conversion functions Signed-off-by: aw --- include/iso15118/io/sdp_packet.hpp | 4 +- include/iso15118/message/authorization.hpp | 12 ++--- include/iso15118/session/feedback.hpp | 1 + src/iso15118/d20/state/authorization.cpp | 45 +++++++------------ .../d20/state/authorization_setup.cpp | 14 +++++- src/iso15118/d20/state/dc_charge_loop.cpp | 37 +++------------ src/iso15118/message/authorization.cpp | 35 ++++++++++----- src/iso15118/message/service_detail.cpp | 14 +++--- test/exi/cb/iso20/authorization.cpp | 2 +- test/iso15118/states/authorization.cpp | 10 ++--- 10 files changed, 83 insertions(+), 91 deletions(-) diff --git a/include/iso15118/io/sdp_packet.hpp b/include/iso15118/io/sdp_packet.hpp index 3f1853d8..cd6626a2 100644 --- a/include/iso15118/io/sdp_packet.hpp +++ b/include/iso15118/io/sdp_packet.hpp @@ -65,7 +65,9 @@ class SdpPacket { void parse_header(); State state{State::EMPTY}; - uint8_t buffer[2048]; + // FIXME (aw): what buffer size to allow here? Could also be made + // dynamic + uint8_t buffer[4096]; size_t bytes_read{0}; size_t length; // length includes V2GTP_HEADER_SIZE }; diff --git a/include/iso15118/message/authorization.hpp b/include/iso15118/message/authorization.hpp index 5e86a7ca..3b8f9c85 100644 --- a/include/iso15118/message/authorization.hpp +++ b/include/iso15118/message/authorization.hpp @@ -2,8 +2,8 @@ // Copyright 2023 Pionix GmbH and Contributors to EVerest #pragma once -#include #include +#include #include #include "common.hpp" @@ -20,11 +20,8 @@ struct AuthorizationRequest { // Todo(sl): Refactor in common struct ContractCertificateChain { - struct Certificate { - std::vector certificate; - }; - Certificate certificate; - std::vector sub_certificates; + std::vector certificate; + std::vector> sub_certificates; }; struct EIM_ASReqAuthorizationMode {}; @@ -36,8 +33,7 @@ struct AuthorizationRequest { Header header; Authorization selected_authorization_service; - std::optional eim_as_req_authorization_mode; - std::optional pnc_as_req_authorization_mode; + std::variant authorization_mode; }; struct AuthorizationResponse { diff --git a/include/iso15118/session/feedback.hpp b/include/iso15118/session/feedback.hpp index ad0396cb..963c95fd 100644 --- a/include/iso15118/session/feedback.hpp +++ b/include/iso15118/session/feedback.hpp @@ -17,6 +17,7 @@ namespace feedback { enum class Signal { REQUIRE_AUTH_EIM, + REQUIRE_AUTH_PNC, START_CABLE_CHECK, SETUP_FINISHED, CHARGE_LOOP_STARTED, diff --git a/src/iso15118/d20/state/authorization.cpp b/src/iso15118/d20/state/authorization.cpp index feb6c36d..b3183f48 100644 --- a/src/iso15118/d20/state/authorization.cpp +++ b/src/iso15118/d20/state/authorization.cpp @@ -38,31 +38,24 @@ message_20::AuthorizationResponse handle_request(const message_20::Authorization auto response_code = message_20::ResponseCode::OK; - switch (req.selected_authorization_service) { - case message_20::Authorization::EIM: - switch (authorization_status) { - case AuthStatus::Accepted: - res.evse_processing = message_20::Processing::Finished; - response_code = message_20::ResponseCode::OK; - break; - case AuthStatus::Rejected: // Failure [V2G20-2230] - res.evse_processing = message_20::Processing::Finished; - response_code = message_20::ResponseCode::WARNING_EIMAuthorizationFailure; - break; - case AuthStatus::Pending: - default: - res.evse_processing = message_20::Processing::Ongoing; - response_code = message_20::ResponseCode::OK; - break; - } + const auto get_rejection_response_code = [](message_20::Authorization auth_method) { + return (auth_method == message_20::Authorization::EIM) + ? message_20::ResponseCode::WARNING_EIMAuthorizationFailure + : message_20::ResponseCode::WARNING_GeneralPnCAuthorizationError; + }; + + switch (authorization_status) { + case AuthStatus::Accepted: + res.evse_processing = message_20::Processing::Finished; + response_code = message_20::ResponseCode::OK; break; - - case message_20::Authorization::PnC: - // todo(SL): Handle PnC + case AuthStatus::Rejected: // Failure [V2G20-2230] + res.evse_processing = message_20::Processing::Finished; + response_code = get_rejection_response_code(req.selected_authorization_service); break; - - default: - // todo(SL): Fill + case AuthStatus::Pending: + res.evse_processing = message_20::Processing::Ongoing; + response_code = message_20::ResponseCode::OK; break; } @@ -82,11 +75,7 @@ FsmSimpleState::HandleEventReturnType Authorization::handle_event(AllocatorType& return sa.HANDLED_INTERNALLY; } - if (*control_data) { - authorization_status = AuthStatus::Accepted; - } else { - authorization_status = AuthStatus::Rejected; - } + authorization_status = (*control_data) ? AuthStatus::Accepted : AuthStatus::Rejected; return sa.HANDLED_INTERNALLY; } diff --git a/src/iso15118/d20/state/authorization_setup.cpp b/src/iso15118/d20/state/authorization_setup.cpp index 06ebcfe0..24604f3e 100644 --- a/src/iso15118/d20/state/authorization_setup.cpp +++ b/src/iso15118/d20/state/authorization_setup.cpp @@ -13,6 +13,16 @@ namespace iso15118::d20::state { +namespace { +auto required_auth_signal(const message_20::AuthorizationSetupResponse& res) { + using Signal = session::feedback::Signal; + return (std::holds_alternative( + res.authorization_mode)) + ? Signal::REQUIRE_AUTH_PNC + : Signal::REQUIRE_AUTH_EIM; +} +} // namespace + message_20::AuthorizationSetupResponse handle_request(const message_20::AuthorizationSetupRequest& req, d20::Session& session, bool cert_install_service, const std::vector& authorization_services) { @@ -40,6 +50,7 @@ handle_request(const message_20::AuthorizationSetupRequest& req, d20::Session& s auto& pnc_auth_mode = res.authorization_mode.emplace(); + // FIXME (aw): refactor to utilities std::random_device rd; std::mt19937 generator(rd()); std::uniform_int_distribution distribution(0x00, 0xff); @@ -77,8 +88,7 @@ FsmSimpleState::HandleEventReturnType AuthorizationSetup::handle_event(Allocator return sa.PASS_ON; } - // Todo(sl): PnC is currently not supported - ctx.feedback.signal(session::feedback::Signal::REQUIRE_AUTH_EIM); + ctx.feedback.signal(required_auth_signal(res)); return sa.create_simple(ctx); } else if (const auto req = variant->get_if()) { diff --git a/src/iso15118/d20/state/dc_charge_loop.cpp b/src/iso15118/d20/state/dc_charge_loop.cpp index 1a2f99ab..3f0fa6df 100644 --- a/src/iso15118/d20/state/dc_charge_loop.cpp +++ b/src/iso15118/d20/state/dc_charge_loop.cpp @@ -22,43 +22,18 @@ using Dynamic_BPT_DC_Res = message_20::DC_ChargeLoopResponse::BPT_Dynamic_DC_CLR template void convert(Out& out, const In& in); -template <> void convert(Scheduled_DC_Res& out, const d20::DcTransferLimits& in) { +template void convert(Out& out, const d20::DcTransferLimits& in) { out.max_charge_power = in.charge_limits.power.max; out.min_charge_power = in.charge_limits.power.min; out.max_charge_current = in.charge_limits.current.max; out.max_voltage = in.voltage.max; -} - -template <> void convert(Scheduled_BPT_DC_Res& out, const d20::DcTransferLimits& in) { - out.max_charge_power = in.charge_limits.power.max; - out.min_charge_power = in.charge_limits.power.min; - out.max_charge_current = in.charge_limits.current.max; - out.max_voltage = in.voltage.max; - out.min_voltage = in.voltage.min; - - if (in.discharge_limits.has_value()) { - auto& discharge_limits = in.discharge_limits.value(); - out.max_discharge_power = discharge_limits.power.max; - out.min_discharge_power = discharge_limits.power.min; - out.max_discharge_current = discharge_limits.current.max; - } -} - -template <> void convert(Dynamic_DC_Res& out, const d20::DcTransferLimits& in) { - out.max_charge_power = in.charge_limits.power.max; - out.min_charge_power = in.charge_limits.power.min; - out.max_charge_current = in.charge_limits.current.max; - out.max_voltage = in.voltage.max; -} -template <> void convert(Dynamic_BPT_DC_Res& out, const d20::DcTransferLimits& in) { - out.max_charge_power = in.charge_limits.power.max; - out.min_charge_power = in.charge_limits.power.min; - out.max_charge_current = in.charge_limits.current.max; - out.max_voltage = in.voltage.max; - out.min_voltage = in.voltage.min; + if constexpr (std::is_same_v || std::is_same_v) { + out.min_voltage = in.voltage.min; - if (in.discharge_limits.has_value()) { + if (not in.discharge_limits) { + return; + } auto& discharge_limits = in.discharge_limits.value(); out.max_discharge_power = discharge_limits.power.max; out.min_discharge_power = discharge_limits.power.min; diff --git a/src/iso15118/message/authorization.cpp b/src/iso15118/message/authorization.cpp index 5f8d8e47..88044026 100644 --- a/src/iso15118/message/authorization.cpp +++ b/src/iso15118/message/authorization.cpp @@ -10,22 +10,37 @@ namespace iso15118::message_20 { +// FIXME (aw): this template should be commonly reused +template void copy_bytes_to_vector(std::vector& out, const SourceType& in) { + static_assert(std::is_same_v>); + out.reserve(in.bytesLen); + std::copy(in.bytes, in.bytes + in.bytesLen, std::back_inserter(out)); +} + +template <> +void convert(const struct iso20_PnC_AReqAuthorizationModeType& in, + AuthorizationRequest::PnC_ASReqAuthorizationMode& out) { + out.id = CB2CPP_STRING(in.Id); + + copy_bytes_to_vector(out.gen_challenge, in.GenChallenge); + copy_bytes_to_vector(out.contract_certificate_chain.certificate, in.ContractCertificateChain.Certificate); + + for (std::size_t i = 0; i < in.ContractCertificateChain.SubCertificates.Certificate.arrayLen; ++i) { + const auto& sub_certificate_in = in.ContractCertificateChain.SubCertificates.Certificate.array[i]; + auto& sub_certificate_out = out.contract_certificate_chain.sub_certificates.emplace_back(); + copy_bytes_to_vector(sub_certificate_out, sub_certificate_in); + } +} + template <> void convert(const struct iso20_AuthorizationReqType& in, AuthorizationRequest& out) { convert(in.Header, out.header); out.selected_authorization_service = static_cast(in.SelectedAuthorizationService); if (in.EIM_AReqAuthorizationMode_isUsed) { - out.eim_as_req_authorization_mode.emplace(); + out.authorization_mode = AuthorizationRequest::EIM_ASReqAuthorizationMode{}; } else if (in.PnC_AReqAuthorizationMode_isUsed) { - - auto& pnc_out = out.pnc_as_req_authorization_mode.emplace(); - - pnc_out.id = CB2CPP_STRING(in.PnC_AReqAuthorizationMode.Id); - pnc_out.gen_challenge.reserve(in.PnC_AReqAuthorizationMode.GenChallenge.bytesLen); - pnc_out.gen_challenge.insert( - pnc_out.gen_challenge.end(), &in.PnC_AReqAuthorizationMode.GenChallenge.bytes[0], - &in.PnC_AReqAuthorizationMode.GenChallenge.bytes[in.PnC_AReqAuthorizationMode.GenChallenge.bytesLen]); - // Todo(sl): Adding certificate + auto& pnc_out = out.authorization_mode.emplace(); + convert(in.PnC_AReqAuthorizationMode, pnc_out); } } diff --git a/src/iso15118/message/service_detail.cpp b/src/iso15118/message/service_detail.cpp index 96f248e6..4d55f6d4 100644 --- a/src/iso15118/message/service_detail.cpp +++ b/src/iso15118/message/service_detail.cpp @@ -159,16 +159,20 @@ template <> void convert(const ServiceDetailResponse& in, iso20_ServiceDetailRes uint8_t index = 0; for (auto const& in_parameter_set : in.service_parameter_list) { - auto& out_paramater_set = out.ServiceParameterList.ParameterSet.array[index++]; - out_paramater_set.ParameterSetID = in_parameter_set.id; + auto& out_parameter_set = out.ServiceParameterList.ParameterSet.array[index++]; + init_iso20_ParameterSetType(&out_parameter_set); + + out_parameter_set.ParameterSetID = in_parameter_set.id; uint8_t t = 0; for (auto const& in_parameter : in_parameter_set.parameter) { - auto& out_parameter = out_paramater_set.Parameter.array[t++]; + auto& out_parameter = out_parameter_set.Parameter.array[t++]; + init_iso20_ParameterType(&out_parameter); + CPP2CB_STRING(in_parameter.name, out_parameter.Name); std::visit(ParamterValueVisitor(out_parameter), in_parameter.value); } - out_paramater_set.Parameter.arrayLen = in_parameter_set.parameter.size(); + out_parameter_set.Parameter.arrayLen = in_parameter_set.parameter.size(); } out.ServiceParameterList.ParameterSet.arrayLen = in.service_parameter_list.size(); @@ -179,7 +183,7 @@ template <> void insert_type(VariantAccess& va, const struct iso20_ServiceDetail } template <> int serialize_to_exi(const ServiceDetailResponse& in, exi_bitstream_t& out) { - iso20_exiDocument doc; + iso20_exiDocument doc {}; init_iso20_exiDocument(&doc); CB_SET_USED(doc.ServiceDetailRes); diff --git a/test/exi/cb/iso20/authorization.cpp b/test/exi/cb/iso20/authorization.cpp index 1df8c6f9..764452fe 100644 --- a/test/exi/cb/iso20/authorization.cpp +++ b/test/exi/cb/iso20/authorization.cpp @@ -28,7 +28,7 @@ SCENARIO("Se/Deserialize authorization messages") { REQUIRE(header.timestamp == 1691411798); REQUIRE(msg.selected_authorization_service == message_20::Authorization::EIM); - REQUIRE(msg.eim_as_req_authorization_mode.has_value() == true); + REQUIRE(std::holds_alternative(msg.authorization_mode)); } } diff --git a/test/iso15118/states/authorization.cpp b/test/iso15118/states/authorization.cpp index e73b9aa1..10b5efd6 100644 --- a/test/iso15118/states/authorization.cpp +++ b/test/iso15118/states/authorization.cpp @@ -17,7 +17,7 @@ SCENARIO("Authorization state handling") { req.header.timestamp = 1691411798; req.selected_authorization_service = message_20::Authorization::EIM; - req.eim_as_req_authorization_mode.emplace(); + req.authorization_mode = message_20::AuthorizationRequest::EIM_ASReqAuthorizationMode{}; const auto res = d20::state::handle_request(req, d20::Session(), AuthStatus::Pending); @@ -37,7 +37,7 @@ SCENARIO("Authorization state handling") { req.header.timestamp = 1691411798; req.selected_authorization_service = message_20::Authorization::EIM; - req.eim_as_req_authorization_mode.emplace(); + req.authorization_mode = message_20::AuthorizationRequest::EIM_ASReqAuthorizationMode{}; const auto res = d20::state::handle_request(req, session, AuthStatus::Pending); @@ -59,7 +59,7 @@ SCENARIO("Authorization state handling") { req.header.timestamp = 1691411798; req.selected_authorization_service = message_20::Authorization::EIM; - req.eim_as_req_authorization_mode.emplace(); + req.authorization_mode = message_20::AuthorizationRequest::EIM_ASReqAuthorizationMode{}; const auto res = d20::state::handle_request(req, session, AuthStatus::Rejected); @@ -79,7 +79,7 @@ SCENARIO("Authorization state handling") { req.header.timestamp = 1691411798; req.selected_authorization_service = message_20::Authorization::EIM; - req.eim_as_req_authorization_mode.emplace(); + req.authorization_mode = message_20::AuthorizationRequest::EIM_ASReqAuthorizationMode{}; const auto res = d20::state::handle_request(req, session, AuthStatus::Pending); @@ -99,7 +99,7 @@ SCENARIO("Authorization state handling") { req.header.timestamp = 1691411798; req.selected_authorization_service = message_20::Authorization::EIM; - req.eim_as_req_authorization_mode.emplace(); + req.authorization_mode = message_20::AuthorizationRequest::EIM_ASReqAuthorizationMode{}; const auto res = d20::state::handle_request(req, session, AuthStatus::Accepted); From fe33d6f175c909b856a5d3ac3f4354bc4b4d092e Mon Sep 17 00:00:00 2001 From: aw Date: Sat, 9 Nov 2024 11:47:21 +0100 Subject: [PATCH 2/3] chore(comments): future openssl improvements - some comments have been added, which should be handled in near future Signed-off-by: aw --- src/iso15118/io/connection_openssl.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/iso15118/io/connection_openssl.cpp b/src/iso15118/io/connection_openssl.cpp index d70954c9..f85a26f1 100644 --- a/src/iso15118/io/connection_openssl.cpp +++ b/src/iso15118/io/connection_openssl.cpp @@ -64,6 +64,8 @@ static SSL_CTX* init_ssl(const config::SSLConfig& ssl_config) { log_and_raise_openssl_error("Failed in SSL_CTX_new()"); } + // NOTE (aw): this could also be set to TLS1_3_VERSION in order to + // enforce TLS1.3 if (SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION) == 0) { log_and_raise_openssl_error("Failed in SSL_CTX_set_min_proto_version()"); } @@ -289,6 +291,8 @@ void ConnectionSSL::handle_data() { if ((ssl_error == SSL_ERROR_WANT_READ) or (ssl_error == SSL_ERROR_WANT_WRITE)) { return; } + + // FIXME (aw): need to handle this gracefully log_and_raise_openssl_error("Failed to SSL_accept(): " + std::to_string(ssl_error)); } else { logf_info("Handshake complete!"); From 97c8677fbb6bd815db5c0b9adb13e3c0766dd9a8 Mon Sep 17 00:00:00 2001 From: aw Date: Sat, 9 Nov 2024 12:14:50 +0100 Subject: [PATCH 3/3] clang-format Signed-off-by: aw --- src/iso15118/message/service_detail.cpp | 2 +- test/exi/cb/iso20/authorization.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/iso15118/message/service_detail.cpp b/src/iso15118/message/service_detail.cpp index 4d55f6d4..5d8705b2 100644 --- a/src/iso15118/message/service_detail.cpp +++ b/src/iso15118/message/service_detail.cpp @@ -183,7 +183,7 @@ template <> void insert_type(VariantAccess& va, const struct iso20_ServiceDetail } template <> int serialize_to_exi(const ServiceDetailResponse& in, exi_bitstream_t& out) { - iso20_exiDocument doc {}; + iso20_exiDocument doc{}; init_iso20_exiDocument(&doc); CB_SET_USED(doc.ServiceDetailRes); diff --git a/test/exi/cb/iso20/authorization.cpp b/test/exi/cb/iso20/authorization.cpp index 764452fe..cbd3235c 100644 --- a/test/exi/cb/iso20/authorization.cpp +++ b/test/exi/cb/iso20/authorization.cpp @@ -28,7 +28,8 @@ SCENARIO("Se/Deserialize authorization messages") { REQUIRE(header.timestamp == 1691411798); REQUIRE(msg.selected_authorization_service == message_20::Authorization::EIM); - REQUIRE(std::holds_alternative(msg.authorization_mode)); + REQUIRE(std::holds_alternative( + msg.authorization_mode)); } }