Attention is currently required from: fixeria, neels.
laforge has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/40208?usp=email )
Change subject: personalization: implement UppAudit and BatchAudit ......................................................................
Patch Set 11:
(1 comment)
File pySim/esim/saip/batch.py:
https://gerrit.osmocom.org/c/pysim/+/40208/comment/312edcb2_f467d0a3?usp=ema... : PS11, Line 143: {'001010000000023'}
Ok, I wasn't aware of the possibility... […]
To me, the "single USIM NAA and single ISIM NAA" is the common case, and whatever we do should support this. This most definitely includes the sysmo-esim-mgr, and I would so far have assumed that if the IMSI is personalized there, it will be personalized in the ADF.USIM and automagically, if an ADF.ISIM/EF.IMSI exists, also there. If not, it is quite simply broken.
The "multiple USIM and/or multiple ISIM NAAs in a single profile" is a to me seemingly esoteric option in the standard, but there must be valid use cases as real-world eUICCs like I think the sysmoEUICC1 actually do support it. The EUICCs actually indicate the support of this optional feature to the SM-DP+ during the mutual authentication phase, so the SM-DP+ can ensure to provide compatible profiles.
I still believe our code should be safe. So if we do not support multiple USIM NAAs or multiple ISIM NAAs from the audit code (or any other part of the code), it should safely abort and refuse to proceed with such a PESequence as input data. The way to do this in python is raising an exception. This way a user [of the API / module] cannot end up with an invalid/non-applicable uadit result, or with something that's only half personalized. It will have zero implications on sysmo-esim-mgr as long as we are smart enough not to give it input PEsequences that us features which our code doesn't support. And if we ever forget that constraint, it will fail and we will notice it.
I'm not very happy about your "but this is used in production so let's merge something that hasn't really been discussed during it's design" argument. To me it just illustrates once more that having dragged along this way past due code review is causing downstream problems. Review of this code, including any of its possible constraints, should have happened as the first step *before* sysmo-esim-mgr is written or at least has gone public. But that's not something we can change now or need to debat here. Simply indicating that I'm a bit allergic to that kind of argument.
Regarding your 'morge complex idea': Yes, that is how I would think is the way to go. And if we want to avoid breaking applications later on, we could make the current code already generate the output in this format. Even if it only supports a single ISMI in ADF.USIM and optionally one in ADF.ISIM, it could generate a format that later on can be augmented with more data as our pySim.esim.saip module becomes more mature. Up to you if you think it makes sense to change that now and avoid future breakage in the returned data format.