dexter has submitted this change. ( https://gerrit.osmocom.org/c/onomondo-eim/+/43016?usp=email ) Change subject: esipa_asn1_handler_utils: clean up handling of EuiccPackageResult ...................................................................... esipa_asn1_handler_utils: clean up handling of EuiccPackageResult The function that handles the parsing and processing of the EuiccPackageResult is written in a confusing way and also seems to be slightly wrong (we do not want to re-bind any work item to a eimTransactionId here). Let's clean it up in a way that is easier to follow. Change-Id: Ic2f64c254f3e07ce8dfdf43af977decf3514627b Related: SYS#8100 --- M src/crypto_utils.erl M src/esipa_asn1_handler_utils.erl 2 files changed, 83 insertions(+), 84 deletions(-) Approvals: dexter: Looks good to me, approved Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve diff --git a/src/crypto_utils.erl b/src/crypto_utils.erl index 21929b5..7d051a1 100644 --- a/src/crypto_utils.erl +++ b/src/crypto_utils.erl @@ -12,6 +12,7 @@ -export([ sign_euiccPackageSigned/2, verify_euiccPackageResultSigned/2, + verify_euiccPackageErrorSigned/2, store_euicc_pubkey_from_authenticateResponseOk/2, store_euicc_pubkey_from_ipaEuiccDataResponse/2 ]). @@ -107,52 +108,57 @@ error end. -verify_euiccPackageResultSigned(EuiccPackageResult, EidValue) -> +verify_euiccPackageResultSigned(EuiccPackageResultSigned, EidValue) -> {ok, ConsumerEuicc} = mnesia_db_euicc:state_get(EidValue, consumerEuicc), case ConsumerEuicc of false -> % Read the AssociationToken {ok, AssociationToken} = mnesia_db_euicc:state_get(EidValue, associationToken), - case EuiccPackageResult of - {euiccPackageResultSigned, EuiccPackageResultSigned} -> - EuiccPackageResultDataSigned = maps:get( - euiccPackageResultDataSigned, EuiccPackageResultSigned - ), - EuiccSignEPR = maps:get(euiccSignEPR, EuiccPackageResultSigned), - {ok, EuiccPackageResultDataSigned_enc} = 'SGP32Definitions':encode( - 'EuiccPackageResultDataSigned', - EuiccPackageResultDataSigned - ), - % "euiccSignEPR SHALL apply on the concatenated data objects euiccPackageResultDataSigned and - % eimSignature." (see also GSMA SGP.32, section 2.11.2.1) - MsgToBeVerfied = utils:join_binary_list([ - EuiccPackageResultDataSigned_enc, - enc_association_token(AssociationToken) - ]), - verify_signature(MsgToBeVerfied, EuiccSignEPR, EidValue); - {euiccPackageErrorSigned, EuiccPackageErrorSigned} -> - EuiccPackageErrorDataSigned = maps:get( - euiccPackageErrorDataSigned, EuiccPackageErrorSigned - ), - EuiccSignEPE = maps:get(euiccSignEPE, EuiccPackageErrorSigned), - {ok, EuiccPackageErrorDataSigned_enc} = 'SGP32Definitions':encode( - 'EuiccPackageErrorDataSigned', - EuiccPackageErrorDataSigned - ), - % "euiccSignEPE SHALL apply on the concatenated data objects euiccPackageErrorDataSigned and - % eimSignature." (see also GSMA SGP.32, section 2.11.2.1) - MsgToBeVerfied = utils:join_binary_list([ - EuiccPackageErrorDataSigned_enc, - enc_association_token(AssociationToken) - ]), - verify_signature(MsgToBeVerfied, EuiccSignEPE, EidValue); - {euiccPackageErrorUnsigned, _} -> - % This result has no signature - ok; - _ -> - error - end; + EuiccPackageResultDataSigned = maps:get( + euiccPackageResultDataSigned, EuiccPackageResultSigned + ), + EuiccSignEPR = maps:get(euiccSignEPR, EuiccPackageResultSigned), + {ok, EuiccPackageResultDataSigned_enc} = 'SGP32Definitions':encode( + 'EuiccPackageResultDataSigned', + EuiccPackageResultDataSigned + ), + % "euiccSignEPR SHALL apply on the concatenated data objects euiccPackageResultDataSigned and + % eimSignature." (see also GSMA SGP.32, section 2.11.2.1) + MsgToBeVerfied = utils:join_binary_list([ + EuiccPackageResultDataSigned_enc, + enc_association_token(AssociationToken) + ]), + verify_signature(MsgToBeVerfied, EuiccSignEPR, EidValue); + _ -> + logger:info( + "omitting signature check for euiccPackageResultSigned from eID ~p (consumer eUICC)~n", + [utils:binary_to_hex(EidValue)] + ), + ok + end. + +verify_euiccPackageErrorSigned(EuiccPackageErrorSigned, EidValue) -> + {ok, ConsumerEuicc} = mnesia_db_euicc:state_get(EidValue, consumerEuicc), + case ConsumerEuicc of + false -> + % Read the AssociationToken + {ok, AssociationToken} = mnesia_db_euicc:state_get(EidValue, associationToken), + EuiccPackageErrorDataSigned = maps:get( + euiccPackageErrorDataSigned, EuiccPackageErrorSigned + ), + EuiccSignEPE = maps:get(euiccSignEPE, EuiccPackageErrorSigned), + {ok, EuiccPackageErrorDataSigned_enc} = 'SGP32Definitions':encode( + 'EuiccPackageErrorDataSigned', + EuiccPackageErrorDataSigned + ), + % "euiccSignEPE SHALL apply on the concatenated data objects euiccPackageErrorDataSigned and + % eimSignature." (see also GSMA SGP.32, section 2.11.2.1) + MsgToBeVerfied = utils:join_binary_list([ + EuiccPackageErrorDataSigned_enc, + enc_association_token(AssociationToken) + ]), + verify_signature(MsgToBeVerfied, EuiccSignEPE, EidValue); _ -> logger:info( "omitting signature check for euiccPackageResultSigned from eID ~p (consumer eUICC)~n", diff --git a/src/esipa_asn1_handler_utils.erl b/src/esipa_asn1_handler_utils.erl index 8d6e59a..373922f 100644 --- a/src/esipa_asn1_handler_utils.erl +++ b/src/esipa_asn1_handler_utils.erl @@ -40,18 +40,13 @@ none end. -process_euiccPackageResult(Pid, EuiccPackageResult, Debuginfo, EimTransactionId) -> - WorkBind = fun(Map) -> - case maps:find(eimTransactionId, Map) of - {ok, Value} -> - mnesia_db_work:bind(Pid, Value); - _ -> - ok - end - end, +% Handle an EuiccPackageResult, this includes everything from the handling of the work items in mnesia_db, down to +% signature checks and the generation of an appropriate outcome for the REST API. +handle_euiccPackageResult(Pid, EuiccPackageResult, Debuginfo) -> + EimTransactionId = eimTransactionId_from_euiccPackageResult(EuiccPackageResult), + {EidValue, _, _} = mnesia_db_work:pickup(Pid, EimTransactionId), CheckCounterValue = fun(Map) -> - {EidValue, _, _} = mnesia_db_work:pickup(Pid, EimTransactionId), CounterValueIpad = maps:get(counterValue, Map), {ok, CounterValueEim} = mnesia_db_euicc:state_get(EidValue, counterValue), case CounterValueIpad of @@ -59,7 +54,7 @@ ok; _ -> logger:error( - "invalid euiccPackageResultSigned, counterValue mismatch: CounterValueIpad=~p, CounterValueEim=~p~n", + "invalid eUICC signature, counterValue mismatch: CounterValueIpad=~p, CounterValueEim=~p~n", [CounterValueIpad, CounterValueEim] ), error @@ -69,31 +64,43 @@ Outcome = case EuiccPackageResult of {euiccPackageResultSigned, EuiccPackageResultSigned} -> - EuiccPackageResultDataSigned = maps:get( - euiccPackageResultDataSigned, EuiccPackageResultSigned - ), - WorkBind(EuiccPackageResultDataSigned), - case CheckCounterValue(EuiccPackageResultDataSigned) of + case + crypto_utils:verify_euiccPackageResultSigned(EuiccPackageResultSigned, EidValue) + of ok -> - esipa_rest_utils:euiccPackageResultDataSigned_to_outcome( - EuiccPackageResultDataSigned - ); + EuiccPackageResultDataSigned = maps:get( + euiccPackageResultDataSigned, EuiccPackageResultSigned + ), + case CheckCounterValue(EuiccPackageResultDataSigned) of + ok -> + esipa_rest_utils:euiccPackageResultDataSigned_to_outcome( + EuiccPackageResultDataSigned + ); + _ -> + [{[{euiccPackageErrorCode, counterValueMismatch}]}] + end; _ -> - [{[{euiccPackageErrorCode, counterValueMismatch}]}] + [{[{procedureError, euiccSignatureInvalid}]}] end; {euiccPackageErrorSigned, EuiccPackageErrorSigned} -> - EuiccPackageErrorDataSigned = maps:get( - euiccPackageErrorDataSigned, EuiccPackageErrorSigned - ), - WorkBind(EuiccPackageErrorDataSigned), - EuiccPackageErrorCode = maps:get( - euiccPackageErrorCode, EuiccPackageErrorDataSigned - ), - case CheckCounterValue(EuiccPackageErrorDataSigned) of + case + crypto_utils:verify_euiccPackageErrorSigned(EuiccPackageErrorSigned, EidValue) + of ok -> - [{[{euiccPackageErrorCode, EuiccPackageErrorCode}]}]; + EuiccPackageErrorDataSigned = maps:get( + euiccPackageErrorDataSigned, EuiccPackageErrorSigned + ), + EuiccPackageErrorCode = maps:get( + euiccPackageErrorCode, EuiccPackageErrorDataSigned + ), + case CheckCounterValue(EuiccPackageErrorDataSigned) of + ok -> + [{[{euiccPackageErrorCode, EuiccPackageErrorCode}]}]; + _ -> + [{[{euiccPackageErrorCode, counterValueMismatch}]}] + end; _ -> - [{[{euiccPackageErrorCode, counterValueMismatch}]}] + [{[{procedureError, euiccSignatureInvalid}]}] end; {euiccPackageErrorUnsigned, _} -> [{[{euiccPackageErrorCode, undefinedError}]}] @@ -101,20 +108,6 @@ mnesia_db_work:finish(Pid, Outcome, Debuginfo). -% Handle an EuiccPackageResult, this includes everything from the handling of the work items in mnesia_db, down to -% signature checks and the generation of an appropriate outcome for the REST API. -handle_euiccPackageResult(Pid, EuiccPackageResult, Debuginfo) -> - EimTransactionId = eimTransactionId_from_euiccPackageResult(EuiccPackageResult), - {EidValue, _, _} = mnesia_db_work:pickup(Pid, EimTransactionId), - case crypto_utils:verify_euiccPackageResultSigned(EuiccPackageResult, EidValue) of - ok -> - process_euiccPackageResult(Pid, EuiccPackageResult, Debuginfo, EimTransactionId); - _ -> - mnesia_db_work:finish( - Pid, [{[{procedureError, euiccSignatureInvalid}]}], Debuginfo - ) - end. - % Handle an IpaEuiccDataResponse, this includes everything from the handling of the work items in mnesia_db as well % as the generation of an appropriate outcome for the REST API. In case IpaEuiccDataResponse contains an eUICC public % key, we will automatically store it so that we can use it to check the signatures of PSMOs and eCOs. -- To view, visit https://gerrit.osmocom.org/c/onomondo-eim/+/43016?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: onomondo-eim Gerrit-Branch: master Gerrit-Change-Id: Ic2f64c254f3e07ce8dfdf43af977decf3514627b Gerrit-Change-Number: 43016 Gerrit-PatchSet: 9 Gerrit-Owner: dexter <pmaier@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier@sysmocom.de> Gerrit-Reviewer: laforge <laforge@osmocom.org>