Skip to content

Commit

Permalink
Ensure only one APDU is processed at the same time across all media
Browse files Browse the repository at this point in the history
When two apdus are received in parallel:
* The first one is not impacted by the second one
* The second one receives an error response SWO_IOL_STA_02 (0x1302)
  • Loading branch information
yrichard-ledger committed Oct 16, 2023
1 parent ef27df1 commit dedfb45
Show file tree
Hide file tree
Showing 8 changed files with 164 additions and 27 deletions.
7 changes: 7 additions & 0 deletions include/os_io_usb.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ io_usb_hid_receive_status_t io_usb_hid_receive(io_send_t sndfct,
unsigned short l,
apdu_buffer_t *apdu_buffer);

/**
* Read next HID transport packet, keep track of received APDU but discard content.
* Return true when an apdu is fully received (and discarded)
* To be called typically upon USB OUT event while an other apdu is being processed
*/
bool io_usb_hid_discard(unsigned char *buffer, unsigned short l);

/**
* Mark the last chunk transmitted as sent.
* To be called typically upon USB IN ACK event
Expand Down
72 changes: 61 additions & 11 deletions lib_blewbxx_impl/src/ledger_ble.c
Original file line number Diff line number Diff line change
Expand Up @@ -452,10 +452,11 @@ static void init_mngr(uint16_t opcode, const uint8_t *buffer, uint16_t length)
ledger_ble_data.hci_cmd_opcode = 0xfc0f;
aci_hal_set_tx_power_level(1, // High power (ignored)
#ifdef TARGET_STAX
0x19); // 0 dBm
#else // !TARGET_STAX
0x07); // -14.1 dBm
#endif // !TARGET_STAX
0x19 // 0 dBm
#else // !TARGET_STAX
0x07 // -14.1 dBm
#endif // !TARGET_STAX
);
break;

case BLE_INIT_STEP_CONFIGURE_ADVERTISING:
Expand Down Expand Up @@ -510,7 +511,18 @@ static void hci_evt_cmd_complete(const uint8_t *buffer, uint16_t length)
if (ledger_ble_data.transfer_mode_enable) {
if ((ledger_protocol_data.rx_apdu_length)
&& (ledger_protocol_data.rx_apdu_status == APDU_STATUS_COMPLETE)) {
copy_apdu_to_app(false);
if (G_io_app.apdu_state == APDU_IDLE) {
copy_apdu_to_app(false);
}
else {
// Discard received APDU and respond with error
ledger_protocol_data.rx_apdu_length = 0;
ledger_protocol_data.rx_apdu_status = APDU_STATUS_WAITING;
uint8_t resp[2];
U2BE_ENCODE(resp, 0, SWO_IOL_STA_02);
LEDGER_PROTOCOL_tx(resp, 2);
notify_chunk();
}
}
}
else {
Expand All @@ -521,7 +533,9 @@ static void hci_evt_cmd_complete(const uint8_t *buffer, uint16_t length)
if (!ledger_protocol_data.tx_apdu_buffer) {
ledger_protocol_data.tx_chunk_length = 0;
G_io_app.ble_xfer_timeout = 0;
G_io_app.apdu_state = APDU_IDLE;
if (G_io_app.apdu_state == APDU_BLE) {
G_io_app.apdu_state = APDU_IDLE;
}
if ((!ledger_ble_data.connection_updated)
&& (ledger_ble_data.connection.conn_interval > BLE_SLAVE_CONN_INTERVAL_MIN)) {
ledger_ble_data.connection_updated = 1;
Expand Down Expand Up @@ -872,7 +886,18 @@ static void attribute_modified(const uint8_t *buffer, uint16_t length)
LOG_BLE("Transfer failed 0x%04x\n", U2BE(ledger_ble_data.resp, 0));
G_io_app.transfer_mode = 0;
check_transfer_mode(G_io_app.transfer_mode);
copy_apdu_to_app(true);
if (G_io_app.apdu_state == APDU_IDLE) {
copy_apdu_to_app(true);
}
else {
// Discard received APDU and respond with error
ledger_protocol_data.rx_apdu_length = 0;
ledger_protocol_data.rx_apdu_status = APDU_STATUS_WAITING;
uint8_t resp[2];
U2BE_ENCODE(resp, 0, SWO_IOL_STA_02);
LEDGER_PROTOCOL_tx(resp, 2);
notify_chunk();
}
}
else if (ledger_ble_data.resp_length) {
LEDGER_PROTOCOL_tx(ledger_ble_data.resp, ledger_ble_data.resp_length);
Expand All @@ -881,13 +906,25 @@ static void attribute_modified(const uint8_t *buffer, uint16_t length)
}
}
else {
copy_apdu_to_app(true);
// Nominal case for apdu reception
if (G_io_app.apdu_state == APDU_IDLE) {
copy_apdu_to_app(true);
}
else {
// Discard received APDU and respond with error
ledger_protocol_data.rx_apdu_length = 0;
ledger_protocol_data.rx_apdu_status = APDU_STATUS_WAITING;
uint8_t resp[2];
U2BE_ENCODE(resp, 0, SWO_IOL_STA_02);
LEDGER_PROTOCOL_tx(resp, 2);
notify_chunk();
}
}
}
else if (ledger_protocol_data.tx_chunk_length >= 2) {
// apdu not complete yet
G_io_app.ble_xfer_timeout = 2000;
notify_chunk();
G_io_app.apdu_state = APDU_BLE;
}
}
else {
Expand Down Expand Up @@ -917,7 +954,18 @@ static void write_permit_request(const uint8_t *buffer, uint16_t length)
data_length,
&buffer[3]);
if (ledger_protocol_data.rx_apdu_status == APDU_STATUS_COMPLETE) {
copy_apdu_to_app(true);
if (G_io_app.apdu_state == APDU_IDLE) {
copy_apdu_to_app(true);
}
else {
// Discard received APDU and respond with error
ledger_protocol_data.rx_apdu_length = 0;
ledger_protocol_data.rx_apdu_status = APDU_STATUS_WAITING;
uint8_t resp[2];
U2BE_ENCODE(resp, 0, SWO_IOL_STA_02);
LEDGER_PROTOCOL_tx(resp, 2);
notify_chunk();
}
}
}
else {
Expand Down Expand Up @@ -1040,7 +1088,9 @@ void LEDGER_BLE_set_recv_buffer(uint8_t *buffer, uint16_t buffer_length)
void LEDGER_BLE_send(const uint8_t *packet, uint16_t packet_length)
{
if ((ledger_ble_data.transfer_mode_enable != 0) && (packet_length == 2)) {
G_io_app.apdu_state = APDU_IDLE;
if (G_io_app.apdu_state == APDU_BLE) {
G_io_app.apdu_state = APDU_IDLE;
}
ledger_ble_data.resp_length = 2;
ledger_ble_data.resp[0] = packet[0];
ledger_ble_data.resp[1] = packet[1];
Expand Down
6 changes: 3 additions & 3 deletions lib_standard_app/io.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ WEAK uint8_t io_event(uint8_t channel)
#endif // HAVE_BAGL
break;
case SEPROXYHAL_TAG_STATUS_EVENT:
if (G_io_apdu_media == IO_APDU_MEDIA_USB_HID && //
!(U4BE(G_io_seproxyhal_spi_buffer, 3) & //
SEPROXYHAL_TAG_STATUS_EVENT_FLAG_USB_POWERED)) {
if (G_io_apdu_media == IO_APDU_MEDIA_USB_HID
&& !(U4BE(G_io_seproxyhal_spi_buffer, 3)
& SEPROXYHAL_TAG_STATUS_EVENT_FLAG_USB_POWERED)) {
THROW(EXCEPTION_IO_RESET);
}
__attribute__((fallthrough));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ void CCID_BulkMessage_In (USBD_HandleTypeDef *pdev,
// not timeout compliant // USBD_LL_PrepareReceive(pdev, CCID_BULK_OUT_EP, CCID_BULK_EPOUT_SIZE);

// mark transfer as completed
G_io_app.apdu_state = APDU_IDLE;
if (G_io_app.apdu_state == APDU_USB_CCID){
G_io_app.apdu_state = APDU_IDLE;
}
}

// if remaining length is < EPIN_SIZE: send packet and prepare to receive a new command
Expand Down
20 changes: 18 additions & 2 deletions lib_stusb_impl/usbd_impl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ uint8_t USBD_HID_DataOut_impl(USBD_HandleTypeDef *pdev,

#ifndef HAVE_USB_HIDKBD
// avoid troubles when an apdu has not been replied yet
if (G_io_app.apdu_media == IO_APDU_MEDIA_NONE) {
if (G_io_app.apdu_state == APDU_IDLE) {
// add to the hid transport
switch (io_usb_hid_receive(io_usb_send_apdu_data,
buffer,
Expand All @@ -1052,6 +1052,14 @@ uint8_t USBD_HID_DataOut_impl(USBD_HandleTypeDef *pdev,
break;
}
}
else {
if (io_usb_hid_discard(buffer, io_seproxyhal_get_ep_rx_size(HID_EPOUT_ADDR))) {
// Discarded new APDU, send error response
unsigned char resp_apdu[2] = {};
U2BE_ENCODE(resp_apdu, 0, SWO_IOL_STA_02);
io_usb_hid_send(io_usb_send_apdu_data, 2, resp_apdu);
}
}
#endif // HAVE_USB_HIDKBD
break;
}
Expand Down Expand Up @@ -1116,7 +1124,7 @@ uint8_t USBD_WEBUSB_DataOut(USBD_HandleTypeDef *pdev,
USBD_LL_PrepareReceive(pdev, WEBUSB_EPOUT_ADDR, WEBUSB_EPOUT_SIZE);

// avoid troubles when an apdu has not been replied yet
if (G_io_app.apdu_media == IO_APDU_MEDIA_NONE) {
if (G_io_app.apdu_state == APDU_IDLE) {
// add to the hid transport
switch (io_usb_hid_receive(io_usb_send_apdu_data_ep0x83,
buffer,
Expand All @@ -1132,6 +1140,14 @@ uint8_t USBD_WEBUSB_DataOut(USBD_HandleTypeDef *pdev,
break;
}
}
else {
if (io_usb_hid_discard(buffer, io_seproxyhal_get_ep_rx_size(WEBUSB_EPOUT_ADDR))) {
// Discarded new APDU, send error response
unsigned char resp_apdu[2] = {};
U2BE_ENCODE(resp_apdu, 0, SWO_IOL_STA_02);
io_usb_hid_send(io_usb_send_apdu_data_ep0x83, 2, resp_apdu);
}
}
break;
}

Expand Down
4 changes: 3 additions & 1 deletion lib_u2f/src/u2f_transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ void u2f_transport_sent(u2f_service_t *service, u2f_transport_media_t media)
u2f_transport_reset(service);
// we sent the whole response (even if we haven't yet received the ack for the last sent usb
// in packet)
G_io_app.apdu_state = APDU_IDLE;
if (G_io_app.apdu_state == APDU_U2F) {
G_io_app.apdu_state = APDU_IDLE;
}
}
}

Expand Down
35 changes: 27 additions & 8 deletions src/os_io_seproxyhal.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,19 +241,37 @@ void io_seproxyhal_handle_capdu_event(void)
// copy apdu to apdu buffer
memcpy(G_io_apdu_buffer, G_io_seproxyhal_spi_buffer + 3, G_io_app.apdu_length);
}
else {
// Refuse APDU, send error reply
G_io_seproxyhal_spi_buffer[0] = SEPROXYHAL_TAG_RAPDU;
G_io_seproxyhal_spi_buffer[1] = 0;
G_io_seproxyhal_spi_buffer[2] = 2;
U2BE_ENCODE(G_io_seproxyhal_spi_buffer, 3, SWO_IOL_STA_02);
io_seproxyhal_spi_send(G_io_seproxyhal_spi_buffer, 5);
}
}

#ifdef HAVE_NFC
void io_seproxyhal_handle_nfc_recv_event(void)
{
size_t max = MIN(sizeof(G_io_apdu_buffer), sizeof(G_io_seproxyhal_spi_buffer) - 3);
size_t size = U2BE(G_io_seproxyhal_spi_buffer, 1);
if (G_io_app.apdu_state == APDU_IDLE) {
size_t max = MIN(sizeof(G_io_apdu_buffer), sizeof(G_io_seproxyhal_spi_buffer) - 3);
size_t size = U2BE(G_io_seproxyhal_spi_buffer, 1);

G_io_app.apdu_media = IO_APDU_MEDIA_NFC;
G_io_app.apdu_state = APDU_NFC;
G_io_app.apdu_length = MIN(size, max);
G_io_app.apdu_media = IO_APDU_MEDIA_NFC;
G_io_app.apdu_state = APDU_NFC;
G_io_app.apdu_length = MIN(size, max);

memcpy(G_io_apdu_buffer, &G_io_seproxyhal_spi_buffer[3], G_io_app.apdu_length);
memcpy(G_io_apdu_buffer, &G_io_seproxyhal_spi_buffer[3], G_io_app.apdu_length);
}
else {
// Refuse APDU, send error reply
G_io_seproxyhal_spi_buffer[0] = SEPROXYHAL_TAG_NFC_RAPDU;
G_io_seproxyhal_spi_buffer[1] = 0;
G_io_seproxyhal_spi_buffer[2] = 2;
U2BE_ENCODE(G_io_seproxyhal_spi_buffer, 3, SWO_IOL_STA_02);
io_seproxyhal_spi_send(G_io_seproxyhal_spi_buffer, 5);
}
}
#endif
unsigned int io_seproxyhal_handle_event(void)
Expand Down Expand Up @@ -1364,7 +1382,7 @@ unsigned short io_exchange(unsigned char channel, unsigned short tx_len)
io_seproxyhal_spi_send(G_io_seproxyhal_spi_buffer, 3);
io_seproxyhal_spi_send(G_io_apdu_buffer, tx_len);

// isngle packet reply, mark immediate idle
// single packet reply, mark immediate idle
G_io_app.apdu_state = APDU_IDLE;
G_io_app.apdu_media = IO_APDU_MEDIA_NONE;
goto break_send;
Expand All @@ -1381,8 +1399,9 @@ unsigned short io_exchange(unsigned char channel, unsigned short tx_len)
io_seproxyhal_spi_send(G_io_seproxyhal_spi_buffer, 3);
io_seproxyhal_spi_send(G_io_apdu_buffer, tx_len);

// isngle packet reply, mark immediate idle
// single packet reply, mark immediate idle
G_io_app.apdu_state = APDU_IDLE;
G_io_app.apdu_media = IO_APDU_MEDIA_NONE;
// finished, no chunking
goto break_send;

Expand Down
43 changes: 42 additions & 1 deletion src/os_io_usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,45 @@ io_usb_hid_receive_status_t io_usb_hid_receive(io_send_t sndfct,
return IO_USB_APDU_RESET;
}

bool io_usb_hid_discard(unsigned char *buffer, unsigned short l)
{
static unsigned int seq_number = 0;
static unsigned int remaining_length = 0;

if (buffer[2] != 0x05) {
return false;
}

// ensure sequence idx is 0 for the first chunk
if ((unsigned int) U2BE(buffer, 3) != (unsigned int) seq_number) {
// ignore packet
seq_number = 0;
remaining_length = 0;
return false;
}

// cid, tag, seq
l -= 2 + 1 + 2;

if (seq_number == 0) {
// total apdu size to receive
remaining_length = U2BE(buffer, 5);
l -= 2;
}
if (l > remaining_length) {
l = remaining_length;
}
remaining_length -= l;
seq_number++;

// Full apdu received, reset sequence number for next time
if (remaining_length == 0) {
seq_number = 0;
return true;
}
return false;
}

void io_usb_hid_init(void)
{
G_io_usb_hid_sequence_number = 0;
Expand Down Expand Up @@ -293,7 +332,9 @@ void io_usb_hid_sent(io_send_t sndfct)
io_usb_hid_init();

// we sent the whole response
G_io_app.apdu_state = APDU_IDLE;
if (G_io_app.apdu_state == APDU_USB_HID || G_io_app.apdu_state == APDU_USB_WEBUSB) {
G_io_app.apdu_state = APDU_IDLE;
}
}
}

Expand Down

0 comments on commit dedfb45

Please sign in to comment.