Change in simtrace2[master]: sniff: add checksum verification for ATR and PPS
Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/9966 ) Change subject: sniff: add checksum verification for ATR and PPS .. sniff: add checksum verification for ATR and PPS a checksum error is only signaled in the USB message with the corresponding flag. Change-Id: I277868267c3199eea216ab47bdd09fb2fb944b06 --- M firmware/libcommon/include/simtrace_prot.h M firmware/libcommon/source/sniffer.c M host/simtrace2-sniff.c 3 files changed, 28 insertions(+), 3 deletions(-) Approvals: Jenkins Builder: Verified Harald Welte: Looks good to me, approved diff --git a/firmware/libcommon/include/simtrace_prot.h b/firmware/libcommon/include/simtrace_prot.h index f1f736b..58e51be 100644 --- a/firmware/libcommon/include/simtrace_prot.h +++ b/firmware/libcommon/include/simtrace_prot.h @@ -297,6 +297,7 @@ /* SIMTRACE_MSGT_SNIFF_ATR, SIMTRACE_MSGT_SNIFF_PPS, SIMTRACE_MSGT_SNIFF_TPDU flags */ #define SNIFF_DATA_FLAG_ERROR_INCOMPLETE (1<<5) #define SNIFF_DATA_FLAG_ERROR_MALFORMED (1<<6) +#define SNIFF_DATA_FLAG_ERROR_CHECKSUM (1<<7) /* SIMTRACE_MSGT_SNIFF_CHANGE */ struct sniff_change { diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index c58b047..f99c25a 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -394,6 +394,7 @@ static uint8_t atr_hist_len = 0; /* store the number of expected historical bytes */ static uint8_t y = 0; /* last mask of the upcoming TA, TB, TC, TD interface bytes */ static uint8_t i = 0; /* interface byte subgroup number */ + static uint32_t flags = 0; /* error flag */ /* sanity check */ if (ISO7816_S_IN_ATR != iso_state) { @@ -411,6 +412,7 @@ /* handle ATR byte depending on current state */ switch (atr_state) { case ATR_S_WAIT_TS: /* see ISO/IEC 7816-3:2006 section 8.1 */ + flags = 0; switch (byte) { case 0x23: /* direct convention used, but decoded using inverse convention (a parity error should also have occurred) */ case 0x30: /* inverse convention used, but decoded using direct convention (a parity error should also have occurred) */ @@ -480,8 +482,21 @@ break; } case ATR_S_WAIT_TCK: /* see ISO/IEC 7816-3:2006 section 8.2.5 */ - /* we could verify the checksum, but we are just here to sniff */ - usb_send_atr(0); /* send ATR to host software using USB */ + /* verify checksum if present */ + if (ATR_S_WAIT_TCK == atr_state) { + uint8_t ui; + uint8_t checksum = 0; + for (ui = 1; ui < atr_i; atr_i++) { + checksum ^= atr[ui]; + } + if (checksum) { + flags |= SNIFF_DATA_FLAG_ERROR_CHECKSUM; + /* We still consider the data as valid (e.g. for WT) even is the checksum is wrong. +* It is up to the reader to handle this error (e.g. by resetting) +*/ + } + } + usb_send_atr(flags); /* send ATR to host software using USB */ change_state(ISO7816_S_WAIT_TPDU); /* go to next state */ break; default: @@ -551,6 +566,7 @@ static void process_byte_pps(uint8_t byte) { uint8_t *pps_cur; /* current PPS (request or response) */ + static uint32_t flags = 0; /* error flag */ /* sanity check */ if (ISO7816_S_IN_PPS_REQ == iso_state) { @@ -565,6 +581,7 @@ /* handle PPS byte depending on current state */ switch (pps_state) { /* see ISO/IEC 7816-3:2006 section 9.2 */ case PPS_S_WAIT_PPSS: /*!< initial byte */ + flags = 0; if (0xff) { pps_cur[0] = byte; pps_state = PPS_S_WAIT_PPS0; /* go to next state */ @@ -613,8 +630,11 @@ check ^= pps_cur[4]; } check ^= pps_cur[5]; + if (check) { + flags |= SNIFF_DATA_FLAG_ERROR_CHECKSUM; + } pps_state = PPS_S_WAIT_END; - usb_send_pps(0); /* send PPS to host software using USB */ + usb_send_pps(flags); /* send PPS to host software using USB */ if (ISO7816_S_IN_PPS_REQ == iso_state) { if (0 == check) { /* checksum is valid */ change_state(ISO7816_S_WAIT_PPS_RSP); /* go to next state */ diff --git a/host/simtrace2-sniff.c b/host/simtrace2-sniff.c index efb986f..776253a 100644 --- a/host/simtrace2-sniff.c +++ b/host/simtrace2-sniff.c @@ -134,6 +134,10 @@
Change in simtrace2[master]: sniff: add checksum verification for ATR and PPS
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/9966 ) Change subject: sniff: add checksum verification for ATR and PPS .. Patch Set 6: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/9966 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I277868267c3199eea216ab47bdd09fb2fb944b06 Gerrit-Change-Number: 9966 Gerrit-PatchSet: 6 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Fri, 13 Jul 2018 05:15:25 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in simtrace2[master]: sniff: add checksum verification for ATR and PPS
Hello Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/9966 to look at the new patch set (#6). Change subject: sniff: add checksum verification for ATR and PPS .. sniff: add checksum verification for ATR and PPS a checksum error is only signaled in the USB message with the corresponding flag. Change-Id: I277868267c3199eea216ab47bdd09fb2fb944b06 --- M firmware/libcommon/include/simtrace_prot.h M firmware/libcommon/source/sniffer.c M host/simtrace2-sniff.c 3 files changed, 28 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/66/9966/6 -- To view, visit https://gerrit.osmocom.org/9966 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I277868267c3199eea216ab47bdd09fb2fb944b06 Gerrit-Change-Number: 9966 Gerrit-PatchSet: 6 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder
Change in simtrace2[master]: sniff: add checksum verification for ATR and PPS
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/9966 ) Change subject: sniff: add checksum verification for ATR and PPS .. Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/9966 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I277868267c3199eea216ab47bdd09fb2fb944b06 Gerrit-Change-Number: 9966 Gerrit-PatchSet: 3 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Wed, 11 Jul 2018 19:58:32 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in simtrace2[master]: sniff: add checksum verification for ATR and PPS
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/9966 to look at the new patch set (#3). Change subject: sniff: add checksum verification for ATR and PPS .. sniff: add checksum verification for ATR and PPS a checksum error is only signaled in the USB message with the corresponding flag. Change-Id: I277868267c3199eea216ab47bdd09fb2fb944b06 --- M firmware/libcommon/include/simtrace_prot.h M firmware/libcommon/source/sniffer.c M host/simtrace2-sniff.c 3 files changed, 28 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/66/9966/3 -- To view, visit https://gerrit.osmocom.org/9966 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I277868267c3199eea216ab47bdd09fb2fb944b06 Gerrit-Change-Number: 9966 Gerrit-PatchSet: 3 Gerrit-Owner: Kévin Redon Gerrit-Reviewer: Jenkins Builder
Change in simtrace2[master]: sniff: add checksum verification for ATR and PPS
Kévin Redon has uploaded this change for review. ( https://gerrit.osmocom.org/9966 Change subject: sniff: add checksum verification for ATR and PPS .. sniff: add checksum verification for ATR and PPS a checksum error is only signaled in the USB message with the corresponding flag. Change-Id: I277868267c3199eea216ab47bdd09fb2fb944b06 --- M firmware/libcommon/include/simtrace_prot.h M firmware/libcommon/source/sniffer.c M host/simtrace2-sniff.c 3 files changed, 28 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/66/9966/1 diff --git a/firmware/libcommon/include/simtrace_prot.h b/firmware/libcommon/include/simtrace_prot.h index f1f736b..58e51be 100644 --- a/firmware/libcommon/include/simtrace_prot.h +++ b/firmware/libcommon/include/simtrace_prot.h @@ -297,6 +297,7 @@ /* SIMTRACE_MSGT_SNIFF_ATR, SIMTRACE_MSGT_SNIFF_PPS, SIMTRACE_MSGT_SNIFF_TPDU flags */ #define SNIFF_DATA_FLAG_ERROR_INCOMPLETE (1<<5) #define SNIFF_DATA_FLAG_ERROR_MALFORMED (1<<6) +#define SNIFF_DATA_FLAG_ERROR_CHECKSUM (1<<7) /* SIMTRACE_MSGT_SNIFF_CHANGE */ struct sniff_change { diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index c58b047..f99c25a 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -394,6 +394,7 @@ static uint8_t atr_hist_len = 0; /* store the number of expected historical bytes */ static uint8_t y = 0; /* last mask of the upcoming TA, TB, TC, TD interface bytes */ static uint8_t i = 0; /* interface byte subgroup number */ + static uint32_t flags = 0; /* error flag */ /* sanity check */ if (ISO7816_S_IN_ATR != iso_state) { @@ -411,6 +412,7 @@ /* handle ATR byte depending on current state */ switch (atr_state) { case ATR_S_WAIT_TS: /* see ISO/IEC 7816-3:2006 section 8.1 */ + flags = 0; switch (byte) { case 0x23: /* direct convention used, but decoded using inverse convention (a parity error should also have occurred) */ case 0x30: /* inverse convention used, but decoded using direct convention (a parity error should also have occurred) */ @@ -480,8 +482,21 @@ break; } case ATR_S_WAIT_TCK: /* see ISO/IEC 7816-3:2006 section 8.2.5 */ - /* we could verify the checksum, but we are just here to sniff */ - usb_send_atr(0); /* send ATR to host software using USB */ + /* verify checksum if present */ + if (ATR_S_WAIT_TCK == atr_state) { + uint8_t ui; + uint8_t checksum = 0; + for (ui = 1; ui < atr_i; atr_i++) { + checksum ^= atr[ui]; + } + if (checksum) { + flags |= SNIFF_DATA_FLAG_ERROR_CHECKSUM; + /* We still consider the data as valid (e.g. for WT) even is the checksum is wrong. +* It is up to the reader to handle this error (e.g. by resetting) +*/ + } + } + usb_send_atr(flags); /* send ATR to host software using USB */ change_state(ISO7816_S_WAIT_TPDU); /* go to next state */ break; default: @@ -551,6 +566,7 @@ static void process_byte_pps(uint8_t byte) { uint8_t *pps_cur; /* current PPS (request or response) */ + static uint32_t flags = 0; /* error flag */ /* sanity check */ if (ISO7816_S_IN_PPS_REQ == iso_state) { @@ -565,6 +581,7 @@ /* handle PPS byte depending on current state */ switch (pps_state) { /* see ISO/IEC 7816-3:2006 section 9.2 */ case PPS_S_WAIT_PPSS: /*!< initial byte */ + flags = 0; if (0xff) { pps_cur[0] = byte; pps_state = PPS_S_WAIT_PPS0; /* go to next state */ @@ -613,8 +630,11 @@ check ^= pps_cur[4]; } check ^= pps_cur[5]; + if (check) { + flags |= SNIFF_DATA_FLAG_ERROR_CHECKSUM; + } pps_state = PPS_S_WAIT_END; - usb_send_pps(0); /* send PPS to host software using USB */ + usb_send_pps(flags); /* send PPS to host software using USB */ if (ISO7816_S_IN_PPS_REQ == iso_state) { if (0 == check) { /* checksum is valid */ change_state(ISO7816_S_WAIT_PPS_RSP); /* go to next state */ diff --git a/host/simtrace2-sniff.c b/host/simtrace2-sniff.c index 765ced3..64cc354 100644 --- a/host/simtrace2-sniff.c +++ b/host/simtrace2-sniff.c @@ -131,6 +131,10 @@