Project

General

Profile

Actions

Bug #5830

closed

pySim-prog.py --mnclen=3 buggy / broken

Added by roh over 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
12/14/2022
Due date:
% Done:

100%

Spec Reference:

Description

trying to reprogramm a few sja2 i run into --mnclen not working at all for me.

when trying to use it was adamant that "--mnclen=2" is not a valid option and i should select 2 or 3.... something is buggy here.
then i patched the default from 2 to 3 to use the code and it tried writing a 2 digit mnc.

was this feature tested in batch mode?

my command line looked similar to this:

./pySim-prog.py --mnclen=3 --read-iccid --read-csv=simcards_adm.csv -S csv -p 0 --batch --dry-run

after patching mnclen to default to 3 i tried

./pySim-prog.py --mnc=900 --read-iccid --read-csv=simcards_adm.csv -S csv -p 0 --batch --dry-run

but this also prints out 2 digit mnc in the generated output.

Actions #1

Updated by roh over 1 year ago

  • Subject changed from pySim-prog.py to pySim-prog.py --mnclen=3 buggy / broken
Actions #2

Updated by laforge over 1 year ago

  • Assignee set to dexter
  • Priority changed from Normal to Urgent
Actions #3

Updated by dexter over 1 year ago

As far as I can see right now multiple things seem to be broken.

What I need would be the following:

  • A commandline (I assume the above are exactly what you have tried)
  • A CSV file sample (I think 3 records are enough) to test it. You do not have to do any modifications, I will then just copy+paste the data from my test card)

(In general log output from your system would be very helpful but since you showed my what goes wrong on your machine I know what output I should expect.)

Actions #4

Updated by dexter over 1 year ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 90

I have fixed the problem now. The --mnclen parameter can now be used again, since our CSV files do not have an mnclen fields the --mnclen parameter also works in CSV mode now. I also checked the dry run and cleaned up the code there a bit but thats not mandatory to get the problem fixed.

https://gerrit.osmocom.org/c/pysim/+/30632
https://gerrit.osmocom.org/c/pysim/+/30634

Actions #5

Updated by dexter over 1 year ago

  • Status changed from Resolved to In Progress

The current status can be found on pySim branch: pmaier/mnclen

Actions #6

Updated by dexter over 1 year ago

  • Status changed from In Progress to Resolved

The patches from pmaier/mnclen are all merged now.

Actions #7

Updated by roh over 1 year ago

thanks for fixing this.

i got a little odd warning from python on debian11:

/home/osmocom/test/pysim/./pySim-prog.py:602: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if opts.mnclen is "2":
/home/osmocom/test/pysim/./pySim-prog.py:604: SyntaxWarning: "is" with a literal. Did you mean "=="?
  elif opts.mnclen is "3":

not sure if this is the correct fix, but it seems to work:

diff --git a/pySim-prog.py b/pySim-prog.py
index b3919ee..448f2a0 100755
--- a/pySim-prog.py
+++ b/pySim-prog.py
@@ -599,9 +599,9 @@ def read_params_csv(opts, imsi=None, iccid=None):
         # commandline options we can use that info, otherwise we guess that
         # the length is 2, which is also the most common case.
         if opts.mnclen != "auto":
-            if opts.mnclen is "2":
+            if opts.mnclen == "2":
                 row['mnc'] = row.get('mnc', mnc_from_imsi(row.get('imsi'), False))
-            elif opts.mnclen is "3":
+            elif opts.mnclen == "3":
                 row['mnc'] = row.get('mnc', mnc_from_imsi(row.get('imsi'), True))
             else:
                 raise ValueError("invalid parameter --mnclen, must be 2 or 3 or auto")

provisioning seems to work fine.

Actions #8

Updated by roh over 1 year ago

  • Status changed from Resolved to In Progress

this seems not fixed completely yet.

i have tried and the sim still has:

MF/ADF.USIM/EF.AD still features a "mnc_len": 2 when read via pysimshell after programming a 3 digit mnc

the phone can register when forced but it assumes it is roaming.

Actions #9

Updated by dexter about 1 year ago

I have checked this back with current master and --mnclen 3 and --mnclen 3, with pySim-shell I can confirm that EF.AD changes accordingly. We should try to reproduce the problem on the card programming setup.

Actions #10

Updated by dexter about 1 year ago

I think I found the reason why it still works on my machine. The cards I am using are programmed with a different profile. Apparently the EF.AD in ADF.USIM is now an individual file and not a link to the EF.AD in DF.GSM. I am actually surprised that this is the case. A solution could be to select both files and program them both with the same content.

Actions #11

Updated by dexter about 1 year ago

I have now fixed the problem. EF.AD is now also programmed under ADF.USIM:
https://gerrit.osmocom.org/c/pysim/+/31004 cards: also program EF.AD under ADF.USIM [NEW]

I still wonder why EF.AD is no longer linked to a single file. pySim-prog.py relies on the assumption that when the files in DF.GSM are written that then the corresponding files in ADF.USIM are updated as well. There are a few other files where it works the same way. I wonder if those need attention now. I could check it with an experiment, but then I need one of those new cards I can freely experiment with.

Here is a list of the potentially affected files:
EF.IMSI (seems to be ok)
EF.PLMNsel
EF.PLMNwAcT
EF.OPLMNwAcT
EF.HPLMNwAcT
EF.SMSP

Actions #12

Updated by roh about 1 year ago

seems to work, the phone is happy now.

Actions #13

Updated by dexter about 1 year ago

roh thanks for trying it out.

sysmo-isim-tool_sja2 had the same problem. I have fixed that now as well. I am a bit unsure if we should program all linked files in all locations. Is there any opinion about that? Are we expecting spec changes that require us to unlink files all too soon?

Actions #14

Updated by laforge about 1 year ago

On Thu, Jan 19, 2023 at 11:33:30AM +0000, dexter wrote:

sysmo-isim-tool_sja2 had the same problem. I have fixed that now as well. I am a bit unsure if we should program all linked files in all locations. Is there any opinion about that? Are we expecting spec changes that require us to unlink files all too soon?

you could always check the FCP if they are linked or not. That's probably the best approach as it will always work, no matter of the detailed card profile.

Actions #15

Updated by dexter about 1 year ago

I wasn't aware that the link state is in the FCP. That allows us to detect if we should write the second location or leave it as it is. This might be helpful in more complex situations where we have to add an IE to a file that isn't present in the other for example. For the EF.AD, things should be fine now. We just read a the file and only modify the MNCLEN field and then we write it back. So we won't mix up things even when we write the second (linked) location.

My question is more about, do we want to keep everything as it is or do we want to write all files in all locations now in case we have to change the profile again?

Actions #16

Updated by laforge about 1 year ago

I think introducing and using some generic function that checks if the file exists a second time and is not linked would be useful. The beat approach is probably to store the (path of) the sister file in the CardEF class. So this way some generic code can

  • determine if the spec allows for a "sister" file somewhere else (and where!)
  • determine if the current card has that sister file
  • determine if the current card links the two files or not

But let's please hold back on this until the unit test patches are merged to avoid any conflicts

Actions #17

Updated by laforge about 1 year ago

laforge wrote in #note-16:

But let's please hold back on this until the unit test patches are merged to avoid any conflicts

This has been done quite some time ago, yet this ticket is "urgent" and "in progress" without any updates?

Actions #18

Updated by dexter about 1 year ago

  • Status changed from In Progress to Stalled
  • Priority changed from Urgent to Normal

I have reset the priority to normal now. The urgent priority was referring to the problem we had when programming the mnclen, this is fixed and no longer a problem. However, we are currently still looking for a general approach for the linked file problem.

Having a list with all sister files in the CardEF class is necessary I think. Each file that has sisters would have such a list as property. Anything else probably needs to be found out by actively probing the files and looking at the FCP. Due to the card profile variations we can never be sure which file is linked and which is not linked.

We could have the following methods:
get_sisters() -> gets a list with the pathes to all sisters (passive, just looking the sisters list
get_links() -> probe all sisters, check the FCP if they are links, then return a list with the linked sisters
get_phys_sisters() -> use get_links(), subtract the result from local list, remainder are the physical sisters
is_link() -> check own FCP to see if we are working on a link
get_phys_file() -> return path to the corresponding physical file

All this would be great to discover the card file system but I am not sure if this is helpful in practice.

However, when I think about what is really needed is a way to ask a file if it is linked or not. This can be told by the FCP. During the card provisioning we would check if a file is just a link. Then we would not touch it since we always program all files we will meet the related physical file at some point anyway. When the file is a physical file, then we would always program it and we can be sure that the content is also reflected in the linked files we may have skipped in the process. This way would also be safe from deviations in the file contents due to evolving specs.

Actions #19

Updated by dexter about 1 year ago

  • Status changed from Stalled to Resolved
  • % Done changed from 90 to 100

I think we can close this now since the actual problem is solved. I have created a new ticket for the problem with the linked files we stumbled upon while fixing this. See also: #5976

Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)