Skip to content

Commit

Permalink
[fix] Fake register requests are now refused whithout displaying gibb…
Browse files Browse the repository at this point in the history
…erish
  • Loading branch information
lpascal-ledger committed Oct 2, 2024
1 parent 0442270 commit 29c5a01
Show file tree
Hide file tree
Showing 17 changed files with 39 additions and 5 deletions.
1 change: 1 addition & 0 deletions include/sw_code.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@
#define SW_INCORRECT_P1P2 0x6A86
#define SW_INS_NOT_SUPPORTED 0x6D00
#define SW_CLA_NOT_SUPPORTED 0x6E00
#define SW_USER_REFUSED 0x6F01
#define SW_PROPRIETARY_INTERNAL 0x6FFF
24 changes: 24 additions & 0 deletions src/u2f_processing.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@
#include "nfc_io.h"
#include "sw_code.h"

// These are used by some services to perform fake register in order to check for user presence
// As this could be disruptive, we are going to immediately return an error on such request.
static const uint8_t bogusSize = 32;
static const uint8_t bogusAppParamValue = 0x41;
static const uint8_t bogusChallengeParamValue = 0x42;

static int u2f_get_cmd_msg_data(uint8_t *rx, uint16_t rx_length, uint8_t **data, uint32_t *le) {
uint32_t data_length;
/* Parse buffer to retrieve the data length.
Expand Down Expand Up @@ -189,6 +195,24 @@ static int u2f_handle_apdu_enroll(const uint8_t *rx, uint32_t data_length, const
return io_send_sw(SW_INCORRECT_P1P2);
}

// Hacky behavior in U2F: some browser send this gibberish 'register' command while
// waiting answers for authentication.
if (sizeof(reg_req->challenge_param) == bogusSize &&
sizeof(reg_req->application_param) == bogusSize) {
bool fake_register = true;
// checking app & challenge parameters are only 0x41s and 0x42s
for (int i = 0; i < bogusSize; i++) {
if (reg_req->challenge_param[i] != bogusChallengeParamValue ||
reg_req->application_param[i] != bogusAppParamValue) {
fake_register = false;
break;
}
}
if (fake_register) {
return io_send_sw(SW_USER_REFUSED);
}
}

// Backup ins, challenge and application parameters to be used if user accept the request
globals_get_u2f_data()->ins = FIDO_INS_REGISTER;
memmove(globals_get_u2f_data()->challenge_param,
Expand Down
6 changes: 3 additions & 3 deletions src/u2f_processing_flow.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ static int u2f_process_user_presence_confirmed(void) {
#if defined(HAVE_BAGL)

static unsigned int u2f_callback_cancel(void) {
io_send_sw(SW_PROPRIETARY_INTERNAL);
io_send_sw(SW_USER_REFUSED);
ui_idle();
return 0;
}
Expand Down Expand Up @@ -346,7 +346,7 @@ static void on_register_choice(bool confirm) {
u2f_process_user_presence_confirmed();
app_nbgl_status("Registration details\nsent", true, ui_idle);
} else {
io_send_sw(SW_PROPRIETARY_INTERNAL);
io_send_sw(SW_USER_REFUSED);
app_nbgl_status("Registration cancelled", false, ui_idle);
}
}
Expand All @@ -356,7 +356,7 @@ static void on_login_choice(bool confirm) {
u2f_process_user_presence_confirmed();
app_nbgl_status("Login request signed", true, ui_idle);
} else {
io_send_sw(SW_PROPRIETARY_INTERNAL);
io_send_sw(SW_USER_REFUSED);
app_nbgl_status("Log in cancelled", false, ui_idle);
}
}
Expand Down
1 change: 1 addition & 0 deletions tests/speculos/ctap1_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class APDU(IntEnum):

# Vendor specific status codes
SW_INTERNAL_EXCEPTION = 0X6F00,
SW_USER_REFUSED = 0x6F01,
SW_PROPRIETARY_INTERNAL = 0x6FFF,


Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion tests/speculos/u2f/test_authenticate_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def test_authenticate_user_refused(client, test_name):
check_screens="full",
compare_args=compare_args)

assert e.value.code == APDU.SW_PROPRIETARY_INTERNAL
assert e.value.code == APDU.SW_USER_REFUSED


def test_authenticate_with_reboot_ok(client):
Expand Down
10 changes: 9 additions & 1 deletion tests/speculos/u2f/test_register_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,15 @@ def test_register_user_refused(client, test_name):
check_screens="full",
compare_args=compare_args)

assert e.value.code == APDU.SW_PROPRIETARY_INTERNAL
assert e.value.code == APDU.SW_USER_REFUSED


def test_register_fake_refused(client):
# challenge parameter + application parameter
data = b'\x42' * 32 + b'\x41' * 32
with pytest.raises(ApduError) as e:
client.ctap1.send_apdu(ins=Ctap1.INS.REGISTER, data=data)
assert e.value.code == APDU.SW_USER_REFUSED


def test_register_duplicate(client):
Expand Down

0 comments on commit 29c5a01

Please sign in to comment.