lynxis lazus has submitted this change. (
https://gerrit.osmocom.org/c/simtrace2/+/43185?usp=email )
Change subject: Add osmo_apdu_segment_in2() to ensure correct parsing of GP GET
DATA
......................................................................
Add osmo_apdu_segment_in2() to ensure correct parsing of GP GET DATA
osmo_apdu_segment_in() doesn't have the context of the previous APDU,
for nearly all calls this is fine, except for GP GET DATA, which can be
a case 2 or case 4 APDU.
GP GET DATA defines the Le field as 0x00 which is used by osmo_apdu_segment_in
to detect if it is a case 2 or case 4.
However a card can respond with the SW 6cXX, which means the previous
command must be resend with a Le field XX, which got misinterpreted by
the osmo_apdu_segment_in() as a case 4 resulting in a timeout waiting for
more data.
Modem <-> Card
81cadf2000 ->
<- 6c0b
81cadf200b ->
-- Simtrace waits for more data because it wrongly decodes it as APDU case 4
instead of case 2.
Related: SYS#8147
Change-Id: Ie238662e7e6a10dd4283ede0c8d8d73bc375c247
---
M TODO-RELEASE
M host/Makefile.am
M host/configure.ac
M host/include/osmocom/simtrace2/apdu_dispatch.h
M host/lib/apdu_dispatch.c
A host/tests/Makefile.am
A host/tests/apdu_dispatch/Makefile.am
A host/tests/apdu_dispatch/apdu_dispatch_test.c
A host/tests/apdu_dispatch/apdu_dispatch_test.ok
A host/tests/testsuite.at
10 files changed, 197 insertions(+), 1 deletion(-)
Approvals:
dexter: Looks good to me, approved; Verified
Jenkins Builder: Verified
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 0ed7189..5922210 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -7,3 +7,4 @@
# If any interfaces have been added since the last public release: c:r:a + 1.
# If any interfaces have been removed or changed since the last public
release: c:r:0.
#library what description / commit summary line
+libosmo-simtrace2 added osmo_apdu_segment_in2()
diff --git a/host/Makefile.am b/host/Makefile.am
index 338a46b..465a4e0 100644
--- a/host/Makefile.am
+++ b/host/Makefile.am
@@ -1,7 +1,7 @@
AUTOMAKE_OPTIONS = foreign dist-bzip2 1.6
AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include
-SUBDIRS = include lib src contrib #tests examples doc
+SUBDIRS = include lib src contrib tests # examples doc
EXTRA_DIST = .version
diff --git a/host/configure.ac b/host/configure.ac
index 83a2b23..84b4147 100644
--- a/host/configure.ac
+++ b/host/configure.ac
@@ -100,4 +100,6 @@
src/Makefile
lib/Makefile
contrib/Makefile
+ tests/Makefile
+ tests/apdu_dispatch/Makefile
Makefile)
diff --git a/host/include/osmocom/simtrace2/apdu_dispatch.h
b/host/include/osmocom/simtrace2/apdu_dispatch.h
index 62ff762..f3c0ada 100644
--- a/host/include/osmocom/simtrace2/apdu_dispatch.h
+++ b/host/include/osmocom/simtrace2/apdu_dispatch.h
@@ -45,3 +45,6 @@
int osmo_apdu_segment_in(struct osmo_apdu_context *ac, const uint8_t *apdu_buf,
unsigned int apdu_len, bool new_apdu);
+
+int osmo_apdu_segment_in2(struct osmo_apdu_context *ac, struct
osmo_apdu_context *prev_ac,
+ const uint8_t *apdu_buf, unsigned int apdu_len, bool
new_apdu);
diff --git a/host/lib/apdu_dispatch.c b/host/lib/apdu_dispatch.c
index f2fa64e..8233507 100644
--- a/host/lib/apdu_dispatch.c
+++ b/host/lib/apdu_dispatch.c
@@ -78,14 +78,52 @@
int osmo_apdu_segment_in(struct osmo_apdu_context *ac, const uint8_t *apdu_buf,
unsigned int apdu_len, bool new_apdu)
{
+ return osmo_apdu_segment_in2(ac, NULL, apdu_buf, apdu_len, new_apdu);
+}
+
+/*! \brief input function for APDU segmentation
+ * \param ac APDU context across successive calls
+ * \param prev_ac Previous APDU context across successive calls
+ * \param[in] apdu_buf APDU input data buffer
+ * \param[in] apdu_len Length of apdu_buf
+ * \param[in] new_apdu Is this the beginning of a new APDU?
+ *
+ * The function returns APDU_ACT_TX_CAPDU_TO_CARD once there is
+ * sufficient data of the APDU received to transmit the command-APDU to
+ * the actual card.
+ *
+ * The function returns APDU_ACT_RX_MORE_CAPDU_FROM_READER when there
+ * is more data to be received from the card reader (GSM Phone).
+ */
+int osmo_apdu_segment_in2(struct osmo_apdu_context *ac, struct
osmo_apdu_context *prev_ac,
+ const uint8_t *apdu_buf, unsigned int apdu_len,
+ bool new_apdu)
+{
int rc = 0;
if (new_apdu) {
+ if (prev_ac)
+ memcpy(prev_ac, ac, sizeof(*ac));
/* initialize the apdu context structure */
memset(ac, 0, sizeof(*ac));
/* copy APDU header over */
memcpy(&ac->hdr, apdu_buf, sizeof(ac->hdr));
ac->apdu_case =
osim_determine_apdu_case(&osim_uicc_sim_cic_profile, apdu_buf);
+
+ /* Corner case when card returns SW 6CXX (resend previous
command with Le = XX).
+ * Especial GP APDU defines Le to be 0x00, except if card
returned SW 6cXX,
+ * Le must be set to XX. osim_determine_apdu_case() might
return the wrong
+ * APDU case because osim_determine_apdu_case() uses the Le
field to determine if the APDU
+ */
+ if (prev_ac && prev_ac->sw[0] == 0x6c && /* check if Le was
invalid and must be set to sw[1] */
+ (prev_ac->apdu_case >= 1 && prev_ac->apdu_case < 4) && /*
handling only valid for apdu case 1..3 */
+ ac->hdr.cla == prev_ac->hdr.cla && /* check if CLA, INS,
P1, P2 are equal */
+ ac->hdr.ins == prev_ac->hdr.ins &&
+ ac->hdr.p1 == prev_ac->hdr.p1 &&
+ ac->hdr.p2 == prev_ac->hdr.p2) {
+ ac->apdu_case = prev_ac->apdu_case;
+ }
+
switch (ac->apdu_case) {
case 1: /* P3 == 0, No Lc/Le */
ac->le.tot = ac->lc.tot = 0;
diff --git a/host/tests/Makefile.am b/host/tests/Makefile.am
new file mode 100644
index 0000000..e58a286
--- /dev/null
+++ b/host/tests/Makefile.am
@@ -0,0 +1,40 @@
+SUBDIRS = apdu_dispatch
+
+# The `:;' works around a Bash 3.2 bug when the output is not writeable.
+$(srcdir)/package.m4: $(top_srcdir)/configure.ac
+ :;{ \
+ echo '# Signature of the current package.' && \
+ echo 'm4_define([AT_PACKAGE_NAME],' && \
+ echo ' [$(PACKAGE_NAME)])' && \
+ echo 'm4_define([AT_PACKAGE_TARNAME],' && \
+ echo ' [$(PACKAGE_TARNAME)])' && \
+ echo 'm4_define([AT_PACKAGE_VERSION],' && \
+ echo ' [$(PACKAGE_VERSION)])' && \
+ echo 'm4_define([AT_PACKAGE_STRING],' && \
+ echo ' [$(PACKAGE_STRING)])' && \
+ echo 'm4_define([AT_PACKAGE_BUGREPORT],' && \
+ echo ' [$(PACKAGE_BUGREPORT)])'; \
+ echo 'm4_define([AT_PACKAGE_URL],' && \
+ echo ' [$(PACKAGE_URL)])'; \
+ } >'$(srcdir)/package.m4'
+
+EXTRA_DIST = testsuite.at $(srcdir)/package.m4 $(TESTSUITE)
+TESTSUITE = $(srcdir)/testsuite
+DISTCLEANFILES = atconfig
+
+check-local: atconfig $(TESTSUITE)
+ $(SHELL) '$(TESTSUITE)' $(TESTSUITEFLAGS)
+
+installcheck-local: atconfig $(TESTSUITE)
+ $(SHELL) '$(TESTSUITE)' AUTOTEST_PATH='$(bindir)' \
+ $(TESTSUITEFLAGS)
+
+clean-local:
+ test ! -f '$(TESTSUITE)' || \
+ $(SHELL) '$(TESTSUITE)' --clean
+
+AUTOM4TE = $(SHELL) $(top_srcdir)/missing --run autom4te
+AUTOTEST = $(AUTOM4TE) --language=autotest
+$(TESTSUITE): $(srcdir)/testsuite.at $(srcdir)/package.m4
+ $(AUTOTEST) -I '$(srcdir)' -o [email protected] [email protected]
+ mv [email protected] $@
diff --git a/host/tests/apdu_dispatch/Makefile.am
b/host/tests/apdu_dispatch/Makefile.am
new file mode 100644
index 0000000..06a07b5
--- /dev/null
+++ b/host/tests/apdu_dispatch/Makefile.am
@@ -0,0 +1,13 @@
+AM_LDFLAGS = -no-install
+AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include
+AM_CFLAGS=-Wall $(LIBOSMONETIF_CFLAGS) $(LIBOSMOCORE_CFLAGS)
$(LIBOSMOVTY_CFLAGS)
+LDADD = $(top_builddir)/lib/.libs/libosmo-simtrace2.la \
+ $(LIBOSMOCORE_LIBS) $(LIBOSMOGSM_LIBS) $(LIBOSMOVTY_LIBS)
$(LIBOSMONETIF_LIBS) $(LIBSCTP_LIBS)
+
+EXTRA_DIST = \
+ apdu_dispatch_test.ok \
+ $(NULL)
+
+check_PROGRAMS = apdu_dispatch_test
+
+apdu_dispatch_test_SOURCES = apdu_dispatch_test.c
diff --git a/host/tests/apdu_dispatch/apdu_dispatch_test.c
b/host/tests/apdu_dispatch/apdu_dispatch_test.c
new file mode 100644
index 0000000..280736f
--- /dev/null
+++ b/host/tests/apdu_dispatch/apdu_dispatch_test.c
@@ -0,0 +1,83 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+
+#include <osmocom/simtrace2/apdu_dispatch.h>
+
+const uint8_t get_data_c2_ca[] = { 0x81, 0xCA, 0x00, 0x5A, 0x00 };
+const uint8_t get_data_c2_cb[] = { 0x81, 0xCB, 0x00, 0x5A, 0x00 };
+
+/* teset data for 6cXX test */
+const uint8_t get_data_c2_ca_le_23[] = { 0x81, 0xCA, 0x00, 0x5A, 0x23 };
+const uint8_t get_data_c2_cb_le_23[] = { 0x81, 0xCb, 0x00, 0x5A, 0x23 };
+
+#define APDU_SEGMENT_IN(apdu, exp_rc) \
+ do { \
+ memset(&ac, 0, sizeof(ac)); \
+ printf("Testing " #apdu "\n"); \
+ int rc = osmo_apdu_segment_in(&ac, apdu, ARRAY_SIZE(apdu),
true); \
+ if (rc != exp_rc) \
+ printf("%d (actual) != %d (expected)\n", rc, exp_rc);\
+ OSMO_ASSERT(rc == exp_rc); \
+ } while (0)
+
+#define APDU_SEGMENT_IN2(apdu, exp_rc) \
+ do { \
+ memset(&ac, 0, sizeof(ac)); \
+ memset(&prev_ac, 0, sizeof(prev_ac)); \
+ printf("Testing " #apdu "\n"); \
+ int rc = osmo_apdu_segment_in2(&ac, &prev_ac, apdu,
ARRAY_SIZE(apdu), true); \
+ if (rc != exp_rc) \
+ printf("%d (actual) != %d (expected)\n", rc, exp_rc);\
+ OSMO_ASSERT(rc == exp_rc); \
+ } while (0)
+
+void test_apdu_dispatch_simple(void)
+{
+ struct osmo_apdu_context ac, prev_ac;
+ APDU_SEGMENT_IN(get_data_c2_ca, APDU_ACT_TX_CAPDU_TO_CARD);
+ APDU_SEGMENT_IN(get_data_c2_cb, APDU_ACT_TX_CAPDU_TO_CARD);
+
+ APDU_SEGMENT_IN2(get_data_c2_ca, APDU_ACT_TX_CAPDU_TO_CARD);
+ APDU_SEGMENT_IN2(get_data_c2_cb, APDU_ACT_TX_CAPDU_TO_CARD);
+}
+
+void test_apdu_dispatch_context(void)
+{
+ struct osmo_apdu_context ac, prev_ac;
+ int rc;
+
+ printf("Testing GET DATA / 0xCA with SW 6Cxx\n");
+ memset(&ac, 0, sizeof(ac));
+ memset(&prev_ac, 0, sizeof(prev_ac));
+ rc = osmo_apdu_segment_in2(&ac, &prev_ac, get_data_c2_ca,
ARRAY_SIZE(get_data_c2_ca), 1);
+ OSMO_ASSERT(rc == APDU_ACT_TX_CAPDU_TO_CARD);
+ OSMO_ASSERT(ac.apdu_case == 2)
+ ac.sw[0] = 0x6c;
+ ac.sw[1] = 0x23;
+ rc = osmo_apdu_segment_in2(&ac, &prev_ac, get_data_c2_ca_le_23,
ARRAY_SIZE(get_data_c2_ca_le_23), 1);
+ OSMO_ASSERT(rc == APDU_ACT_TX_CAPDU_TO_CARD);
+ OSMO_ASSERT(ac.apdu_case == 2)
+
+ printf("Testing GET DATA / 0xCB with SW 6Cxx\n");
+ memset(&ac, 0, sizeof(ac));
+ memset(&prev_ac, 0, sizeof(prev_ac));
+ rc = osmo_apdu_segment_in2(&ac, &prev_ac, get_data_c2_cb,
ARRAY_SIZE(get_data_c2_cb), 1);
+ OSMO_ASSERT(rc == APDU_ACT_TX_CAPDU_TO_CARD);
+ OSMO_ASSERT(ac.apdu_case == 2)
+ ac.sw[0] = 0x6c;
+ ac.sw[1] = 0x23;
+ rc = osmo_apdu_segment_in2(&ac, &prev_ac, get_data_c2_cb_le_23,
ARRAY_SIZE(get_data_c2_cb_le_23), 1);
+ OSMO_ASSERT(rc == APDU_ACT_TX_CAPDU_TO_CARD);
+ OSMO_ASSERT(ac.apdu_case == 2)
+}
+
+int main(int argc, char **argv)
+{
+ test_apdu_dispatch_simple();
+ test_apdu_dispatch_context();
+
+ printf("All tests passed.\n");
+ return 0;
+}
diff --git a/host/tests/apdu_dispatch/apdu_dispatch_test.ok
b/host/tests/apdu_dispatch/apdu_dispatch_test.ok
new file mode 100644
index 0000000..108450f
--- /dev/null
+++ b/host/tests/apdu_dispatch/apdu_dispatch_test.ok
@@ -0,0 +1,7 @@
+Testing get_data_c2_ca
+Testing get_data_c2_cb
+Testing get_data_c2_ca
+Testing get_data_c2_cb
+Testing GET DATA / 0xCA with SW 6Cxx
+Testing GET DATA / 0xCB with SW 6Cxx
+All tests passed.
diff --git a/host/tests/testsuite.at b/host/tests/testsuite.at
new file mode 100644
index 0000000..22fcdc4
--- /dev/null
+++ b/host/tests/testsuite.at
@@ -0,0 +1,9 @@
+AT_INIT
+AT_BANNER([Regression tests.])
+
+AT_SETUP([apdu_dispatch])
+AT_KEYWORDS([apdu_dispatch])
+cat $abs_srcdir/apdu_dispatch/apdu_dispatch_test.ok > expout
+AT_CHECK([$abs_top_builddir/tests/apdu_dispatch/apdu_dispatch_test], [],
[expout], [ignore])
+AT_CLEANUP
+
--
To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43185?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: simtrace2
Gerrit-Branch: master
Gerrit-Change-Id: Ie238662e7e6a10dd4283ede0c8d8d73bc375c247
Gerrit-Change-Number: 43185
Gerrit-PatchSet: 10
Gerrit-Owner: lynxis lazus <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: lynxis lazus <[email protected]>
