Add U2F over NFC support

[asaldele1]

What's new

Add U2F over NFC support to the U2F application.

---

For the reviewer

• I've uploaded the firmware with this patch to a device and verified its functionality
• I've confirmed the bug to be fixed / feature to be stable

[WillyJL]

This seems like a surprisingly small diff for what imo one would expect to be a large feature that took so long for someone to get working (first attempt was like 4 years ago). If it actually works, impressive work. Will try out later today, thanks for the PR

[WillyJL]

i cant seem to get this to work. im using https://webauthn.io on an iphone 12. what are you testing with?

[asaldele1]

I have only tested on android. It would be great if you take some logs but I can also test with an iPhone later.

[WillyJL]

@asaldele1 i might have an idea of what is going on. uint8_t ins = rx_data[1]; this seems like youre skipping the first byte in Iso14443_4aListenerEventTypeReceivedData which you assume to be the iso14443-4a PCB byte. on momentum (due to OFW PR 4242), Iso14443_4aListenerEventTypeReceivedData does not include the PCB, all PCB and iso14443-4a protocol stuff is handled by the nfc library

[WillyJL]

or maybe not, ins sounds like youre parsing apdus, as in byte 0 is cla and byte 1 is ins. still, you should be aware of the difference with Iso14443_4aListenerEventTypeReceivedData, as if you wish to support official firmware aswell for the time being (until OFW pr 4242 is merged, if ever) you would need to do extra work for it to work on official firmware

[WillyJL]

wrt that, have a look here bettse/seos_compatible#4

i made bettse's app support both momentum's and ofw's version of Iso14443_4aListenerEventTypeReceivedData

[asaldele1]

No, I assume that PCB is handled by the firmware. I think it wouldn't work on android either if i didn't. The first byte is CLA.

[asaldele1]

Yea, I know about this PR. In the app I check it like it's done in the seos app.

[asaldele1]

I mean asaldele1/u2f_over_nfc

[WillyJL]

ahh i see, very nice :)

[WillyJL]

i added a log for the full APDU:

26808 [D][U2fNfc] 60 
26809 [D][U2fNfc] Req ins=00 len=1 sel=0
26811 [W][U2fNfc] Reject APDU before SELECT: ins=00
26819 [D][U2fNfc] 90 60 00 00 00 
26821 [D][U2fNfc] Req ins=60 len=5 sel=0
26823 [W][U2fNfc] Reject APDU before SELECT: ins=60
26831 [D][U2fNfc] Disconnect
26844 [D][U2fNfc] Disconnect
27953 [D][U2fNfc] 60 
27954 [D][U2fNfc] Req ins=60 len=1 sel=0
27957 [W][U2fNfc] Reject APDU before SELECT: ins=60
27965 [D][U2fNfc] 90 60 00 00 00 
27966 [D][U2fNfc] Req ins=60 len=5 sel=0
27969 [W][U2fNfc] Reject APDU before SELECT: ins=60
27976 [D][U2fNfc] Disconnect
27989 [D][U2fNfc] Disconnect
29097 [D][U2fNfc] 60 
29098 [D][U2fNfc] Req ins=60 len=1 sel=0
29101 [W][U2fNfc] Reject APDU before SELECT: ins=60
29109 [D][U2fNfc] 90 60 00 00 00 
29110 [D][U2fNfc] Req ins=60 len=5 sel=0
29113 [W][U2fNfc] Reject APDU before SELECT: ins=60
29121 [D][U2fNfc] Disconnect
29133 [D][U2fNfc] Disconnect
30242 [D][U2fNfc] 60 
30244 [D][U2fNfc] Req ins=60 len=1 sel=0
30246 [W][U2fNfc] Reject APDU before SELECT: ins=60
30254 [D][U2fNfc] 90 60 00 00 00 
30256 [D][U2fNfc] Req ins=60 len=5 sel=0
30258 [W][U2fNfc] Reject APDU before SELECT: ins=60
30266 [D][U2fNfc] Disconnect
30278 [D][U2fNfc] Disconnect
31386 [D][U2fNfc] 60 
31387 [D][U2fNfc] Req ins=60 len=1 sel=0
31389 [W][U2fNfc] Reject APDU before SELECT: ins=60
31397 [D][U2fNfc] 90 60 00 00 00 
31399 [D][U2fNfc] Req ins=60 len=5 sel=0
31401 [W][U2fNfc] Reject APDU before SELECT: ins=60
31409 [D][U2fNfc] Disconnect
31421 [D][U2fNfc] Disconnect
32531 [D][U2fNfc] 60 
32532 [D][U2fNfc] Req ins=60 len=1 sel=0
32534 [W][U2fNfc] Reject APDU before SELECT: ins=60
32542 [D][U2fNfc] 90 60 00 00 00 
32544 [D][U2fNfc] Req ins=60 len=5 sel=0
32546 [W][U2fNfc] Reject APDU before SELECT: ins=60
32554 [D][U2fNfc] Disconnect
32566 [D][U2fNfc] Disconnect
34848 [D][U2fNfc] End

[WillyJL]

i notice something interesting here. this really reminds me of how mifare desfire and mifare plus ev do their apdu's. they support both a single byte legacy mifare style command, or can wrap these commands in basic apdu format using CLA 0x90.

[asaldele1]

I think the problem is that the phone is checking the token for compatibility with Apple Pay protocols. We should be returning 'Instruction Not Supported' (6D00), but we are returning 'Conditions Not Satisfied' (6985). We need to validate the command before checking whether the applet has been selected.

[WillyJL]

i notice something interesting here. this really reminds me of how mifare desfire and mifare plus ev do their apdu's. they support both a single byte legacy mifare style command, or can wrap these commands in basic apdu format using CLA 0x90.

https://www.cardlogix.com/wp-content/uploads/MIFARE-Application-Programming-Guide-for-DESFfire_rev.e.pdf this says that INS 0x60 here means "Get File Buffer Addr." and has no additional data. not sure why, but its a real command and the format checks out, so seems like its trying to talk to a desfire / mifare plus ev card lol

[WillyJL]

still nothing sadly:

1648001 [T][U2fNfc] RX: 60 
1648002 [W][U2fNfc] No APDU in frame: 1
1648005 [T][U2fNfc] TX: 6D 00 
1648011 [T][U2fNfc] RX: 90 60 00 00 00 
1648013 [D][U2fNfc] Req ins=60 len=5 sel=0
1648016 [W][U2fNfc] Reject APDU before SELECT: ins=60
1648019 [T][U2fNfc] TX: 6D 00 
1648025 [D][U2fNfc] Disconnect
1648037 [D][U2fNfc] Disconnect

diff --git a/applications/main/u2f/u2f_nfc.c b/applications/main/u2f/u2f_nfc.c
index 404ae2d590..f4f2760c6d 100644
--- a/applications/main/u2f/u2f_nfc.c
+++ b/applications/main/u2f/u2f_nfc.c
@@ -16,10 +16,10 @@
 #define U2F_NFC_PAYLOAD_SIZE   960
 #define U2F_NFC_TX_BUFFER_SIZE 480
 
-#define U2F_SW_CONDITIONS_NOT_SATISFIED_1 0x69
-#define U2F_SW_CONDITIONS_NOT_SATISFIED_2 0x85
-#define U2F_SW_NO_ERROR_1                 0x90
-#define U2F_SW_NO_ERROR_2                 0x00
+#define U2F_SW_INSTRUCTION_NOT_SUPPORTED_1 0x6D
+#define U2F_SW_INSTRUCTION_NOT_SUPPORTED_2 0x00
+#define U2F_SW_NO_ERROR_1                  0x90
+#define U2F_SW_NO_ERROR_2                  0x00
 
 struct U2fNfc {
     Nfc* nfc;
@@ -52,6 +52,12 @@ static bool u2f_nfc_transmit_apdu(
     bit_buffer_reset(tx_buffer);
 
     bit_buffer_append_bytes(tx_buffer, apdu, apdu_len);
+    FuriString* str = furi_string_alloc();
+    for(size_t i = 0; i < bit_buffer_get_size_bytes(tx_buffer); i++) {
+        furi_string_cat_printf(str, "%02X ", bit_buffer_get_byte(tx_buffer, i));
+    }
+    FURI_LOG_T(TAG, "TX: %s", furi_string_get_cstr(str));
+    furi_string_free(str);
 
     Iso14443_4aError error = iso14443_4a_listener_send_block(iso14443_listener, tx_buffer);
     if(error != Iso14443_4aErrorNone) {
@@ -131,8 +137,22 @@ NfcCommand u2f_nfc_worker_listener_callback(NfcGenericEvent event, void* context
         const uint8_t* rx_data = bit_buffer_get_data(rx_buffer);
         uint16_t rx_size = bit_buffer_get_size_bytes(rx_buffer);
 
-        if(rx_size == 0) {
+        FuriString* str = furi_string_alloc();
+        for(size_t i = 0; i < rx_size; i++) {
+            furi_string_cat_printf(str, "%02X ", rx_data[i]);
+        }
+        FURI_LOG_T(TAG, "RX: %s", furi_string_get_cstr(str));
+        furi_string_free(str);
+
+        if(rx_size < 5) {
             FURI_LOG_W(TAG, "No APDU in frame: %u", rx_size);
+            const uint8_t reject_sw[2] = {
+                U2F_SW_INSTRUCTION_NOT_SUPPORTED_1,
+                U2F_SW_INSTRUCTION_NOT_SUPPORTED_2,
+            };
+            if(!u2f_nfc_transmit_apdu(u2f_nfc, iso14443_listener, reject_sw, sizeof(reject_sw))) {
+                break;
+            }
             break;
         }
 
@@ -152,8 +172,8 @@ NfcCommand u2f_nfc_worker_listener_callback(NfcGenericEvent event, void* context
 
         if(!u2f_nfc->applet_selected && ins != U2F_APDU_SELECT) {
             const uint8_t reject_sw[2] = {
-                U2F_SW_CONDITIONS_NOT_SATISFIED_1,
-                U2F_SW_CONDITIONS_NOT_SATISFIED_2,
+                U2F_SW_INSTRUCTION_NOT_SUPPORTED_1,
+                U2F_SW_INSTRUCTION_NOT_SUPPORTED_2,
             };
             FURI_LOG_W(TAG, "Reject APDU before SELECT: ins=%02x", ins);
             if(!u2f_nfc_transmit_apdu(u2f_nfc, iso14443_listener, reject_sw, sizeof(reject_sw))) {

i have to go, but if you have more ideas ill try them whenever i have time

[asaldele1]

Ok, thanks for testing. I'll try to fix it tomorrow.

[asaldele1]

Also I'll try to change ATS values in u2f.nfc.

[aaronjamt]

Just tested with an Android device (Google Pixel 6, fully updated on Android 16) with https://webauthn.io, and it just crashes the Flipper when trying to register. My phone does says that it was successful, but it is not able to authenticate afterwards.

Flipper log:

15468 [D][U2fNfc] Disconnect
15470 [D][U2fNfc] Disconnect
15472 [D][U2fNfc] Disconnect
15474 [D][U2fNfc] Disconnect
15508 [D][U2fNfc] Disconnect
15514 [D][U2fNfc] Disconnect
15544 [D][U2fNfc] Req ins=a4 len=13 sel=0
15546 [D][U2fNfc] Applet select failed
15552 [D][U2fNfc] Req ins=a4 len=12 sel=0
15554 [D][U2fNfc] Applet select failed
15560 [D][U2fNfc] Disconnect
15566 [D][U2fNfc] Disconnect
15605 [D][U2fNfc] Disconnect
15613 [D][U2fNfc] Disconnect

[CRASH][NfcWorker] furi_check failed

It is crashing in the NfcWorker, so not sure if that's truly an issue with this PR or something else. Here's the stack trace for both the U2F app and NfcWorker:

[WillyJL]

That looks like it would've worked, but some bit buffer somewhere is too small so it failed copying the response

[asaldele1]

@aaronjamt does it crash every time? I can't really reproduce it on Android 16 but I remember something simillar happend to me before

[aaronjamt]

@aaronjamt does it crash every time? I can't really reproduce it on Android 16 but I remember something simillar happend to me before

Yeah, I tried it a few times (both the "Register" and "Authenticate" buttons on that website) and it crashed every time.

[xMasterX]

@asaldele1
Please also check review I left for you at flipperdevices/flipper-application-catalog#1022

I also was testing with ios device