lynxis lazus has submitted this change. (
https://gerrit.osmocom.org/c/simtrace2/+/43126?usp=email )
Change subject: firmware: card_emu: use Di in the waiting time
......................................................................
firmware: card_emu: use Di in the waiting time
ISO 7816-3 section 10.2 defines WT = WI x 960 x Fi/f seconds,
store as etu, etu = Fi / (D x f) seconds, so the Fi cancels,
but the D does not:
WT [etu] = WI x 960 x D
cemu dropped both (?!) -> WI x 960.
The old comment explains why Fi can be dropped, which is right, but
what about Di ?!
sniffer gets it right (wt_wi * 960UL * wt_d), so the two state machines
disagreed here again, by up to a factor of 64???!!?!!?
This was fixed in osmo-ccid-firmware in 066489d in 2020 but not ported
to st2.
Additionally the waiting time was only recalculated at the end of the ATR,
where D is still 1 by definition, so a PPS increasing D reprogrammed the
baud rate but left the waiting time untouched??!
etu duration shrinks with D by the same factor, wall clock WT is
independent of D, which is the whole point.
The old code decreased the waiting time by a factor of D:
after a PPS to D=8 the card emitted its NULL procedure byte at ~0.09s
instead of ~0.71s with a reader deadline of ~1.43s,
and the inactivity timeout fires 8x too early, which probably led to
unexplained wtime_exp errors.
Update wt when WI becomes known (end of ATR) and
where D changes (after the PPS response) + tests.
Change-Id: I4263176d6073029d01f9ff5b11a6311617956af6
---
M firmware/libcommon/source/card_emu.c
M firmware/test/card_emu_tests.c
2 files changed, 97 insertions(+), 11 deletions(-)
Approvals:
lynxis lazus: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/firmware/libcommon/source/card_emu.c
b/firmware/libcommon/source/card_emu.c
index bee0383..9c90850 100644
--- a/firmware/libcommon/source/card_emu.c
+++ b/firmware/libcommon/source/card_emu.c
@@ -386,6 +386,26 @@
ch->num, rc);
}
+/*! Calculate the WT from current WI and D.
+ *
+ * ISO 7816-3 10.2 defines WT = WI x 960 x Fi / f [seconds].
+ * Our waiting time is stored in units of etu = Fi / (D x f) seconds
+ * -> the Fi cancels out, but D does not.
+ *
+ * WT [etu] = WI x 960 x D
+ *
+ * D is the value from ISO 7816-3 Table 8. Only 1..9 are defined,
+ * 0 and RFU range 10..15 have no D -> use D = 1 */
+static void emu_update_wt(struct card_handle *ch)
+{
+ uint8_t d = 1;
+
+ if (ch->D_index >= 1 && ch->D_index <= 9)
+ d = iso7816_3_di_table[ch->D_index];
+
+ ch->waiting_time = ch->wi * 960 * d;
+}
+
/* Update the ISO 7816-3 TPDU receiver state */
static void card_set_state(struct card_handle *ch,
enum iso7816_3_card_state new_state)
@@ -505,11 +525,8 @@
}
}
}
- /* update waiting time (see ISO 7816-3 10.2). We can drop the Fi
- * multiplier as we store the waiting time in units of 'etu',
and
- * don't really care what the number of clock cycles or the
absolute
- * wall clock time is */
- ch->waiting_time = ch->wi * 960;
+ /* update the waiting time now that WI is known (see
emu_update_wt) */
+ emu_update_wt(ch);
/* go to next state */
card_set_state(ch, ISO_S_WAIT_TPDU);
return 0;
@@ -675,6 +692,9 @@
card_emu_uart_wait_tx_idle(ch->uart_chan);
/* update baud rate generator with F/D */
emu_update_fidi(ch);
+ /* the waiting time is expressed in etu and scales with D, so
it has
+ * to be recomputed whenever D changes */
+ emu_update_wt(ch);
/* Wait for the next TPDU */
card_set_state(ch, ISO_S_WAIT_TPDU);
set_pts_state(ch, PTS_S_WAIT_REQ_PTSS);
diff --git a/firmware/test/card_emu_tests.c b/firmware/test/card_emu_tests.c
index 1fa4a88..82939c7 100644
--- a/firmware/test/card_emu_tests.c
+++ b/firmware/test/card_emu_tests.c
@@ -396,12 +396,73 @@
0xFF ^ 0b00010000// PCK
};
-static void
-test_ppss(struct card_handle *ch)
+/* Fi/Di that is actually valid: Fi idx 9 (Fi=512) and Di idx 4 (Di=8)
+ * This tests calculating F/D ratio (512/8 = 64) and the calculation of the
+ * waiting time which scales with Di. */
+const uint8_t pps_fidi[] = {
+ 0xFF, // PPSS
+ 0b00010000, // PPS0: PPS1 present
+ 0x94, // PPS1: Fi index 9, Di index 4
+ 0xFF ^ 0b00010000 ^ 0x94// PCK
+};
+
+/* Di 8 idx Di=12 (ISO 7816-3:2006 Table 8),:
+ * the ratio be 372/12 = 31, not 372*12. */
+const uint8_t pps_di12[] = {
+ 0xFF, // PPSS
+ 0b00010000, // PPS0: PPS1 present
+ 0x18, // PPS1: Fi index 1, Di index 8
+ 0xFF ^ 0b00010000 ^ 0x18// PCK
+};
+
+/* Fi idx 5 Fi=1488 andDi=1 -> ratio 1488, needs all 11 bits of
+ * the US_FIDI.FI_DI_RATIO field. */
+const uint8_t pps_hi_ratio[] = {
+ 0xFF, // PPSS
+ 0b00010000, // PPS0: PPS1 present
+ 0x51, // PPS1: Fi index 5, Di index 1
+ 0xFF ^ 0b00010000 ^ 0x51// PCK
+};
+
+/* Get a cemu status report to check the negotiated parameters
+ * This is the only way to get the waiting time from struct card_handle. */
+static void verify_status(struct card_handle *ch, uint8_t exp_f_index, uint8_t
exp_d_index,
+ uint32_t exp_waiting_time)
{
- reader_send_bytes(ch, pps, sizeof(pps));
- get_and_verify_rctx_pps(pps, sizeof(pps));
- card_tx_verify_chars(ch, pps, sizeof(pps));
+ struct usb_buffered_ep *bep = usb_get_buf_ep(PHONE_DATAIN);
+ struct cardemu_usb_msg_status *sts;
+ struct simtrace_msg_hdr *mh;
+ struct msgb *msg;
+
+ card_emu_report_status(ch, false);
+
+ assert(bep);
+ msg = msgb_dequeue_count(&bep->queue, &bep->queue_len);
+ assert(msg);
+ mh = (struct simtrace_msg_hdr *) msg->l1h;
+ assert(mh->msg_type == SIMTRACE_MSGT_BD_CEMU_STATUS);
+ sts = (struct cardemu_usb_msg_status *) msg->l2h;
+
+ printf("status: F_index=%u D_index=%u wi=%u waiting_time=%u\n",
+ sts->F_index, sts->D_index, sts->wi, sts->waiting_time);
+
+ assert(sts->F_index == exp_f_index);
+ assert(sts->D_index == exp_d_index);
+ /* WT = WI x 960 x D in etu, see ISO 7816-3 Section 10.2 */
+ assert(sts->waiting_time == exp_waiting_time);
+
+ usb_buf_free(msg);
+}
+
+static void
+test_ppss(struct card_handle *ch, const uint8_t *req, unsigned int req_len,
+ uint8_t exp_f_index, uint8_t exp_d_index, uint32_t exp_waiting_time)
+{
+ printf("\n==> PPS exchange\n");
+ reader_send_bytes(ch, req, req_len);
+ get_and_verify_rctx_pps(req, req_len);
+ card_tx_verify_chars(ch, req, req_len);
+ verify_status(ch, exp_f_index, exp_d_index, exp_waiting_time);
}
/* READ RECORD (offset 0, 10 bytes) */
@@ -426,7 +487,12 @@
io_start_card(ch);
card_tx_verify_chars(ch, NULL, 0);
- test_ppss(ch);
+ /* WI is 10 so WT = 10 x 960 x D */
+ /* Fi/Di index 0/0 is invalid: the F/D ratio is rejected, D falls back
to 1 */
+ test_ppss(ch, pps, sizeof(pps), 0, 0, 10 * 960 * 1);
+ test_ppss(ch, pps_fidi, sizeof(pps_fidi), 9, 4, 10 * 960 * 8);
+ test_ppss(ch, pps_di12, sizeof(pps_di12), 1, 8, 10 * 960 * 12);
+ test_ppss(ch, pps_hi_ratio, sizeof(pps_hi_ratio), 5, 1, 10 * 960 * 1);
for (i = 0; i < 2; i++) {
test_tpdu_reader2card(ch, tpdu_hdr_write_rec,
tpdu_body_write_rec, sizeof(tpdu_body_write_rec));
--
To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43126?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: I4263176d6073029d01f9ff5b11a6311617956af6
Gerrit-Change-Number: 43126
Gerrit-PatchSet: 3
Gerrit-Owner: Hoernchen <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: lynxis lazus <[email protected]>
