From c8458e24776c4f86fbd04e956697a28deb30ce5b Mon Sep 17 00:00:00 2001 From: Vadim Yanitskiy Date: Fri, 12 Mar 2021 00:34:10 +0100 Subject: [PATCH] pySim/utils.py: fix 3-digit MNC encoding in enc_plmn() The bug that was attempted to be fixed in [1] actually was in the encoding API - pySim.utils.enc_plmn(). According to 3GPP TS 31.102, which points to TS 24.008, the three-digit (E)HPLMN shall be encoded as shown below (ASCII-art interpretation): 0 1 2 3 4 5 6 7 +---+---+---+---+---+---+---+---+ | MCC Digit 2 | MCC Digit 1 | +---+---+---+---+---+---+---+---+ | MNC Digit 3 | MCC Digit 3 | +---+---+---+---+---+---+---+---+ | MNC Digit 2 | MNC Digit 1 | +---+---+---+---+---+---+---+---+ while pySim.utils.enc_plmn() would produce the following: 0 1 2 3 4 5 6 7 +---+---+---+---+---+---+---+---+ | MCC Digit 2 | MCC Digit 1 | +---+---+---+---+---+---+---+---+ | MNC Digit 1 | MCC Digit 3 | +---+---+---+---+---+---+---+---+ | MNC Digit 3 | MNC Digit 2 | +---+---+---+---+---+---+---+---+ Initially the _decoding_ API was correct, but then got changed in [1] to follow buggy pySim's encoding API. As a result, a (E)HPLMN programmed with pySim-prog.py would look correct if verified by pySim-read.py, but the actual file content would be wrong. This situation shows that our 'program-read-match' build verification approach alone is insignificant. The lack of unit test coverage, at least for the core parts of the project, makes it possible to have symmetrical bugs in both encoding and decoding API parts unnoticed. This problem was found while trying to enable dead unit tests in [3]. Change [1] that introduced a symmetrical bug is reverted in [2]. Change-Id: Ic7612502e1bb0d280133dabbcb5cb146fc6997e5 Related: [1] I799469206f87e930d8888367890babcb8ebe23a9 Related: [2] If6bf5383988ad442e275efc7c5a159327d104879 Related: [3] I4d4facfabc75187acd5238ff4d0f26022bd58f82 --- pySim/utils.py | 4 ++-- tests/test_utils.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/pySim/utils.py b/pySim/utils.py index b49a437f..d75a000c 100644 --- a/pySim/utils.py +++ b/pySim/utils.py @@ -109,8 +109,8 @@ def enc_iccid(iccid): def enc_plmn(mcc, mnc): """Converts integer MCC/MNC into 3 bytes for EF""" if len(mnc) == 2: - mnc = "F%s" % mnc - return swap_nibbles("%s%s" % (mcc, mnc)) + mnc += "F" # pad to 3 digits if needed + return (mcc[1] + mcc[0]) + (mnc[2] + mcc[2]) + (mnc[1] + mnc[0]) def dec_spn(ef): byte1 = int(ef[0:2]) diff --git a/tests/test_utils.py b/tests/test_utils.py index 44fe631a..0df205ae 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -29,6 +29,16 @@ class DecTestCase(unittest.TestCase): def testDecMNCfromPLMN_unused(self): self.assertEqual(utils.dec_mnc_from_plmn("00f0ff"), 4095) + def test_enc_plmn(self): + with self.subTest("2-digit MCC"): + self.assertEqual(utils.enc_plmn("001", "01F"), "00F110") + self.assertEqual(utils.enc_plmn("001", "01"), "00F110") + self.assertEqual(utils.enc_plmn("295", "10"), "92F501") + + with self.subTest("3-digit MCC"): + self.assertEqual(utils.enc_plmn("001", "001"), "001100") + self.assertEqual(utils.enc_plmn("302", "361"), "031263") + def testDecAct_noneSet(self): self.assertEqual(utils.dec_act("0000"), [])