laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/37113?usp=email )
Change subject: libosmosim: class_tables: Resolve conflicting CLA=8x INS=F2 definitions
......................................................................
libosmosim: class_tables: Resolve conflicting CLA=8x INS=F2 definitions
In their infinite wisdom, GlobalPlatform re-defined the CLA 8x / INS F2 command
alreay specified by ETSI TS 102 221. This wouldn't be as bads if they
had the same "Case". However, ETSI has case 2 while GP has case 4.
Lucikly, the P1 coding of ETSI [so far] states all the four upper bits
must be 0, while GP always has one of those bits set.
Before this patch, it is possible that a Modem/Phone will send an 8xF2
command and intends it as a GlobalPlatform command (with Lc > 0 and
command data portion), while this code assumes it is an ETSI UICC
command with Lc=0 and hence no command data portion. This will make
communication break when using simtrace2 'cardem'.
Change-Id: I8dd317ef8f942542e412b18c834a0467c51291c3
Related: SYS#6865
Related: https://lists.osmocom.org/hyperkitty/list/simtrace@lists.osmocom.org/thread…
---
M src/sim/class_tables.c
M tests/sim/sim_test.c
M tests/sim/sim_test.ok
3 files changed, 53 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/13/37113/1
diff --git a/src/sim/class_tables.c b/src/sim/class_tables.c
index 9c51387..1cda9a8 100644
--- a/src/sim/class_tables.c
+++ b/src/sim/class_tables.c
@@ -187,6 +187,16 @@
default:
return 3;
}
+ break;
+ case 0xF2:
+ /* in their infinite wisdom, GlobalPlatform re-defined the CLA 8x / INS F2 command, so one can
+ * take a guess if it's GlobalPlatform or ETSI. Lucikly, the P1 coding of ETSI [so far]
+ * states all the four upper bits must be 0, while GP always has one of those bits set */
+ if (p1 & 0xF0)
+ return 4; /* GlobalPlatform v2.2 11.4.2 */
+ else
+ return 2; /* ETSI TS 102 221 V16.2.0 11.1.2 */
+ break;
}
return 0;
}
@@ -217,7 +227,7 @@
[0xE2] = 0x80, /* STORE DATA */
[0xCA] = 4, /* GET DATA */
[0xCB] = 4, /* GET DATA */
- [0xF2] = 4, /* GET STATUS */
+ [0xF2] = 0x80, /* GET STATUS */
[0xE6] = 4, /* INSTALL */
[0xE8] = 4, /* LOAD */
[0xD8] = 4, /* PUT KEY */
@@ -245,6 +255,12 @@
.helper = uicc046_cla_ins_helper,
.ins_tbl = uicc_ins_tbl_046,
}, {
+ /* must be before uicc_ins_tbl_8ce below with same CLA+mask */
+ .cla = 0x80,
+ .cla_mask = 0xF0,
+ .helper = gp_cla_ins_helper,
+ .ins_tbl = gp_ins_tbl_8ce,
+ }, {
.cla = 0x80,
.cla_mask = 0xF0,
.ins_tbl = uicc_ins_tbl_8ce,
@@ -257,11 +273,6 @@
.cla_mask = 0xF0,
.ins_tbl = uicc_ins_tbl_8ce,
}, {
- .cla = 0x80,
- .cla_mask = 0xF0,
- .helper = gp_cla_ins_helper,
- .ins_tbl = gp_ins_tbl_8ce,
- }, {
.cla = 0xC0,
.cla_mask = 0xF0,
.helper = gp_cla_ins_helper,
@@ -307,6 +318,12 @@
.helper = uicc046_cla_ins_helper,
.ins_tbl = uicc_ins_tbl_046,
}, {
+ /* must be before uicc_ins_tbl_8ce below with same CLA+mask */
+ .cla = 0x80,
+ .cla_mask = 0xF0,
+ .helper = gp_cla_ins_helper,
+ .ins_tbl = gp_ins_tbl_8ce,
+ }, {
.cla = 0x80,
.cla_mask = 0xF0,
.ins_tbl = uicc_ins_tbl_8ce,
diff --git a/tests/sim/sim_test.c b/tests/sim/sim_test.c
index 2e2eec5..9a52af4 100644
--- a/tests/sim/sim_test.c
+++ b/tests/sim/sim_test.c
@@ -27,6 +27,8 @@
const uint8_t uicc_tprof_wrong_class[] = { 0x00, 0x10, 0x00, 0x00, 0x02, 0x01, 0x02 };
const uint8_t uicc_read[] = { 0x00, 0xB0, 0x00, 0x00, 0x10 };
const uint8_t uicc_upd[] = { 0x00, 0xD6, 0x00, 0x00, 0x02, 0x01, 0x02 };
+const uint8_t uicc_get_status[] = { 0x80, 0xf2, 0x00, 0x02, 0x10 };
+const uint8_t euicc_m2m_get_status[] = { 0x81, 0xf2, 0x40, 0x02, 0x02, 0x4f, 0x00 };
#define APDU_CASE_ASSERT(x, y) \
do { \
@@ -45,6 +47,8 @@
APDU_CASE_ASSERT(uicc_tprof_wrong_class, 0);
APDU_CASE_ASSERT(uicc_read, 2);
APDU_CASE_ASSERT(uicc_upd, 3);
+ APDU_CASE_ASSERT(uicc_get_status, 2);
+ APDU_CASE_ASSERT(euicc_m2m_get_status, 4);
}
int main(int argc, char **argv)
diff --git a/tests/sim/sim_test.ok b/tests/sim/sim_test.ok
index 7d3f986..cac59ba 100644
--- a/tests/sim/sim_test.ok
+++ b/tests/sim/sim_test.ok
@@ -4,3 +4,5 @@
Testing uicc_tprof_wrong_class
Testing uicc_read
Testing uicc_upd
+Testing uicc_get_status
+Testing euicc_m2m_get_status
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37113?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: rel-1.9.1
Gerrit-Change-Id: I8dd317ef8f942542e412b18c834a0467c51291c3
Gerrit-Change-Number: 37113
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newchange
laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/37114?usp=email )
Change subject: libosmosim: class_tables: Fix GlobalPlatform CLA=8x INS=CA/CB GET DATA
......................................................................
libosmosim: class_tables: Fix GlobalPlatform CLA=8x INS=CA/CB GET DATA
in their infinite wisdom, GlobalPlatform made GET DATA a command that can be either APDU
case 2 or case 4. As the specify Le must be 0x00, we can conclude that P3 == 0x00 must be
Le, while P3 != 0x00 must be Lc and hence case 4 */
Change-Id: Ic8a17921f5a42d227791f1de39f90b4967c2e1b6
Related: SYS#6865
---
M src/sim/class_tables.c
M tests/sim/sim_test.c
M tests/sim/sim_test.ok
3 files changed, 33 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/14/37114/1
diff --git a/src/sim/class_tables.c b/src/sim/class_tables.c
index 1cda9a8..3217446 100644
--- a/src/sim/class_tables.c
+++ b/src/sim/class_tables.c
@@ -178,6 +178,7 @@
{
uint8_t ins = hdr[1];
uint8_t p1 = hdr[2];
+ uint8_t p3 = hdr[4];
switch (ins) {
case 0xE2: /* STORE DATA */
@@ -197,6 +198,16 @@
else
return 2; /* ETSI TS 102 221 V16.2.0 11.1.2 */
break;
+ case 0xCA:
+ case 0xCB:
+ /* in their infinite wisdom, GlobalPlatform made GET DATA a command that can be either APDU
+ * case 2 or case 4. As the specify Le must be 0x00, we can conclude that P3 == 0x00 must be
+ * Le, while P3 != 0x00 must be Lc and hence case 4 */
+ if (p3 == 0x00)
+ return 2;
+ else
+ return 4;
+ break;
}
return 0;
}
@@ -225,8 +236,8 @@
static const uint8_t gp_ins_tbl_8ce[256] = {
[0xE4] = 4, /* DELETE */
[0xE2] = 0x80, /* STORE DATA */
- [0xCA] = 4, /* GET DATA */
- [0xCB] = 4, /* GET DATA */
+ [0xCA] = 0x80, /* GET DATA */
+ [0xCB] = 0x80, /* GET DATA */
[0xF2] = 0x80, /* GET STATUS */
[0xE6] = 4, /* INSTALL */
[0xE8] = 4, /* LOAD */
diff --git a/tests/sim/sim_test.c b/tests/sim/sim_test.c
index 9a52af4..ab5d2be 100644
--- a/tests/sim/sim_test.c
+++ b/tests/sim/sim_test.c
@@ -29,6 +29,8 @@
const uint8_t uicc_upd[] = { 0x00, 0xD6, 0x00, 0x00, 0x02, 0x01, 0x02 };
const uint8_t uicc_get_status[] = { 0x80, 0xf2, 0x00, 0x02, 0x10 };
const uint8_t euicc_m2m_get_status[] = { 0x81, 0xf2, 0x40, 0x02, 0x02, 0x4f, 0x00 };
+const uint8_t gp_get_data2[] = { 0x81, 0xCA, 0x00, 0x5A, 0x00 };
+const uint8_t gp_get_data4[] = { 0x81, 0xCA, 0x00, 0x5A, 0x12 };
#define APDU_CASE_ASSERT(x, y) \
do { \
@@ -49,6 +51,8 @@
APDU_CASE_ASSERT(uicc_upd, 3);
APDU_CASE_ASSERT(uicc_get_status, 2);
APDU_CASE_ASSERT(euicc_m2m_get_status, 4);
+ APDU_CASE_ASSERT(gp_get_data2, 2);
+ APDU_CASE_ASSERT(gp_get_data4, 4);
}
int main(int argc, char **argv)
diff --git a/tests/sim/sim_test.ok b/tests/sim/sim_test.ok
index cac59ba..3abfb71 100644
--- a/tests/sim/sim_test.ok
+++ b/tests/sim/sim_test.ok
@@ -6,3 +6,5 @@
Testing uicc_upd
Testing uicc_get_status
Testing euicc_m2m_get_status
+Testing gp_get_data2
+Testing gp_get_data4
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37114?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: rel-1.9.1
Gerrit-Change-Id: Ic8a17921f5a42d227791f1de39f90b4967c2e1b6
Gerrit-Change-Number: 37114
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newchange
Attention is currently required from: osmith.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36892?usp=email )
Change subject: add umts_cell_id_test.c
......................................................................
Patch Set 4:
(1 comment)
File tests/umts_cell_id/umts_cell_id_test.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/36892/comment/bccda4de_bbf3d44c
PS4, Line 79: /* TODO? There is no bounds checking on RAC and SAC.
> do you want to keep this?
yes, it is a different "problem" in the current umts_cell_id implementation. I noticed while writing the test that we have no bounds checking there, but that's for a separate patch, and possibly not even a problem...
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36892?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ibbb61a2c53a11dea794f451d3074bc9ba50862fe
Gerrit-Change-Number: 36892
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 06 Jun 2024 16:25:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: comment
