Small "bug" fix

Makefile

@@ -152,7 +152,6 @@ DEFINES += RK_SIZE=6144
 endif
 
 DEFINES += HAVE_DEBUG_THROWS
-
 #DEFINES  += HAVE_CBOR_DEBUG
 
 

include/globals.h

@@ -39,7 +39,7 @@ static const uint8_t FIDO_AID[FIDO_AID_SIZE] = {0xA0, 0x00, 0x00, 0x06, 0x47, 0x
 #define OFFSET_P2  3
 
 #define FIDO_CLA               0x00
-#define FIDO_INS_ENROLL        0x01
+#define FIDO_INS_REGISTER      0x01
 #define FIDO_INS_SIGN          0x02
 #define FIDO_INS_GET_VERSION   0x03
 #define FIDO_INS_CTAP2_PROXY   0x10

include/nfc_io.h

@@ -23,7 +23,7 @@ bool nfc_io_is_response_pending(void);
 
 /*
  * Sends a previously prepared response through NFC, then (if successful) displays a status screen
- * (usgin app_nbgl_status). Depending on `display_infos`, this screen will contain additional
+ * (using `app_nbgl_status`). Depending on `display_infos`, this screen will contain additional
  * information such as the relying party name and/or the user credential.
  *
  * @param display_infos If the displayed status screen should contain RP/user information or not.

include/sw_code.h

@@ -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

src/u2f_processing.c

@@ -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.
@@ -189,8 +195,26 @@ 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_ENROLL;
+    globals_get_u2f_data()->ins = FIDO_INS_REGISTER;
     memmove(globals_get_u2f_data()->challenge_param,
             reg_req->challenge_param,
             sizeof(reg_req->challenge_param));
@@ -356,7 +380,7 @@ int u2f_handle_apdu(uint8_t *rx, int rx_length) {
 
     if (rx[OFFSET_CLA] == FIDO_CLA) {
         switch (rx[OFFSET_INS]) {
-            case FIDO_INS_ENROLL:
+            case FIDO_INS_REGISTER:
                 PRINTF("enroll\n");
                 return u2f_handle_apdu_enroll(rx, data_length, data);
 

src/u2f_processing_flow.c

@@ -235,7 +235,7 @@ static int u2f_process_user_presence_confirmed(void) {
     uint16_t length = 0;
 
     switch (globals_get_u2f_data()->ins) {
-        case FIDO_INS_ENROLL:
+        case FIDO_INS_REGISTER:
             sw = u2f_prepare_enroll_response(responseBuffer, &length);
             break;
 
@@ -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;
 }
@@ -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);
     }
 }
@@ -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);
     }
 }

tests/speculos/ctap1_client.py

@@ -26,6 +26,7 @@ class APDU(IntEnum):
 
     # Vendor specific status codes
     SW_INTERNAL_EXCEPTION = 0X6F00,
+    SW_USER_REFUSED = 0x6F01,
     SW_PROPRIETARY_INTERNAL = 0x6FFF,
 
 

tests/speculos/u2f/test_authenticate_cmd.py

@@ -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):

tests/speculos/u2f/test_register_cmd.py

@@ -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):