Change in simtrace2[master]: sniff: add checksum verification for ATR and PPS

2018-07-12 Thread Harald Welte
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

2018-07-12 Thread Harald Welte
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

2018-07-11 Thread Kévin Redon
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

2018-07-11 Thread Harald Welte
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

2018-07-11 Thread Kévin Redon
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

2018-07-11 Thread Kévin Redon
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 @@