nfc: add basic Mifare DESFire support #1024

kevinwallace · 2022-03-14T23:12:48Z

What's new

Detection of DESFire cards based on ATQA/SAK
Reading of all non-protected card data
UI to browse apps/files in card data
Fixes to TextBox wrap behavior
Adds bool type to flipper_format
Saving and loading DESFire tag data

Verification

Open NFC app and scan a DESFire card. Here is an example of me scanning an ORCA (Seattle public transit) card and briefly browsing its data.

flipper_desfire_demo_480p.mov

Checklist (do not modify)

PR has description of feature/bug or link to Confluence/Jira task
Description contains actions to verify feature/bugfix
I've built this code, uploaded it to the device and verified feature/bugfix

* Wrap width is 120 pixels, not 140. (140 is larger than the screen!) * Glyph width already includes spacing; don't add 1 additional px * When starting a new line, include wrapped glyph width in new line_width. * Call canvas_set_font before text_box_insert_endline so that glyph width is calculated using correct font. Previous approach worked somewhat well using default TextBoxFontText but this version is more robust, particularly when using TextBoxFontHex.

/project/applications/desktop/scenes/desktop_scene_main.c:51:13: error: 'desktop_switch_to_app' defined but not used [-Werror=unused-function] 51 | static void desktop_switch_to_app(Desktop* desktop, const FlipperApplication* flipper_app) { | ^~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make[1]: *** [/project/make/rules.mk:70: .obj/f7//project/applications/desktop/scenes/desktop_scene_main.o] Error 1 (Discovered when turning off all the other APP_*s to free up enough space to use APP_UNIT_TESTS)

Built with: make APP_UNIT_TESTS=1 SRV_DESKTOP=0 APP_ARCHIVE=0 APP_GPIO=0 APP_IBUTTON=0 APP_INFRARED=0 APP_LF_RFID=0 APP_NFC=0 APP_SUBGHZ=0 APP_ABOUT=0 APP_PASSPORT=0 APP_MUSIC_PLAYER=0 APP_SNAKE_GAME=0 APP_ACCESSOR=0 APP_BLINK=0 APP_INFRARED_MONITOR=0 APP_KEYPAD_TEST=0 APP_SD_TEST=0 APP_VIBRO_TEST=0 APP_USB_TEST=0 APP_DISPLAY_TEST=0 APP_BLE_HID=0 APP_USB_MOUSE=0 APP_BAD_USB=0 APP_U2F=0 APP_UART_ECHO=0 and passed `unit_tests` via cli

kevinwallace · 2022-03-16T03:30:18Z

Updated to include saving/loading of DESFire tag data. Let me know if the flipper_format bool change is unwanted -- I'm happy to drop that commit and rewrite to use hex for those fields instead.

skotopes · 2022-03-16T04:19:25Z

Hi @kevinwallace

Great amazing job, you are awesome! )

Adding bool is ok. @gornekich will review PR and provide feedback.

All of the same information is present in NfcSceneReadMifareDesfireSuccess, except for ATQA/SAK, which is implicitly known for all saved DESFire cards (0x4403, 0x20).

kevinwallace · 2022-03-16T23:41:03Z

UI has changed a bit since the original video, here's the current state of things

desfire_ui_714915e.mov

(and thank you for Flipper -- this has been such a fun device to hack on! :)

gornekich

Hello @kevinwallace !
Thank you very much for your amazing work!
So far, I looked through your code and left a few comments. I need more time to dive into your DESFire implementation more carefully, and make detailed review.
I noticed a memory leakage after I open DESFire card and then exit NFC application. You can see this from our log console (LoaderSrv logs) or executing 'free' CLI command before and after launching NFC app and opening DESFire card. Memory leakage is a blocking issue for merging your pull request.
I suggest you fix memory issues while I will make detailed review in next couple of days.

gornekich · 2022-03-17T18:50:16Z

applications/nfc/nfc.c

@@ -163,6 +163,8 @@ int32_t nfc_app(void* p) {
    if((*args != '\0') && nfc_device_load(nfc->dev, p)) {
        if(nfc->dev->format == NfcDeviceSaveFormatMifareUl) {
            scene_manager_next_scene(nfc->scene_manager, NfcSceneEmulateMifareUl);
+        } else if(nfc->dev->format == NfcDeviceSaveFormatMifareDesfire) {
+            scene_manager_next_scene(nfc->scene_manager, NfcSceneReadMifareDesfireSuccess);


Now we start nfc application with parameters if we want to launch NFC app from Archive and start UID or Mifare Ultralight emulation immediately. In your case, we will see information about selected DESFire card. I am not sure that we want to break user's expectation here, so I would delete this for now

Done! 8b85ff3

gornekich · 2022-03-17T19:37:17Z

applications/nfc/nfc_worker.c

+                    continue;
+                }
+                if(!mf_df_parse_read_data_response(rx_buff, rx_len, file)) {
+                    FURI_LOG_W(TAG, "Bad response reading file %d\r\n", file->id);


No need in \r\n here

Oops! This was originally a printf. Done. ff1fcbf

gornekich · 2022-03-17T19:49:16Z

applications/nfc/scenes/nfc_scene_mifare_desfire_app.c

+    MifareDesfireApplication* app = nfc_scene_mifare_desfire_app_get_app(nfc);
+    if(!app) {
+        FURI_LOG_E(TAG, "Bad state. No app selected?");
+        return;


There is a risk of undesired behavior. If you return here, view dispatcher will display and deliver events to the view from previous scene and execute on_event() handler of MifareDesfireApp scene. I suggest switching to a Popup view with error explanation.

If this codepath ever runs, it's a bug in the MifareDesfireData scene. But just in case I made a nice popup anyway. :) f5dbd6f

kevinwallace · 2022-03-17T21:15:22Z

Thank you for the comments!

A few hours ago, I noticed a memory leak while reviewing this PR's diff and fixed it in 56ab13e. I think this was the same leak you saw -- I'm now checking with the free CLI command and Free heap size is remaining at the same size whenever I return to the desktop from the NFC app. Could you let me know if you're still seeing this, and if there's a particular set of interactions to reproduce it?

gornekich

Perfect! The leakage I mentioned is gone. Also, thanks for following my suggestions!

Another issue I found is that if I go to saved DESFire card -> Emulate UID -> Back -> Info, there is no info about saved card and after I exit NFC ~3kB of heap leak.
Current UID emulation supports displaying reader commands on the screen. The reason behind this is that we give users an opportunity to research what kind of reader in front of them. From technical point of view, we pass commands received in nfc worker to emulate uid scene through NfcReaderRequestData structure in NfcDeviceData, which shares memory with MifareDesfireData. That's where after UID emulation we loose DESFire data and can't free allocated memory.
The quickest fix here is to disable UID emulation for DESFire cards, just like it's done for Bank Cards. However, if you want to keep opportunity for UID emulation, you should fix NfcDeviceData structure so that NfcReaderRequestData and MifareDesfireData don't share same memory.

Let me know how you want to fix UID emulation issue and when it's done, we will merge your brilliant PR :)

gornekich · 2022-03-18T11:54:48Z

applications/nfc/scenes/nfc_scene_saved_menu.c

@@ -62,7 +74,11 @@ bool nfc_scene_saved_menu_on_event(void* context, SceneManagerEvent event) {
            scene_manager_next_scene(nfc->scene_manager, NfcSceneDelete);
            consumed = true;
        } else if(event.event == SubmenuIndexInfo) {
-            scene_manager_next_scene(nfc->scene_manager, NfcSceneDeviceInfo);
+            if(nfc->dev->format == NfcDeviceSaveFormatMifareDesfire) {
+                scene_manager_next_scene(nfc->scene_manager, NfcSceneReadMifareDesfireSuccess);


I think it's better to show DESFire cards Info in NfcSceneDeviceInfo, or make another NfcSceneMifareDesfireInfo scene. If I go to NfcSceneReadMifareDesfireSuccess scene from Saved cards, I don't expect to Name and save card that I already opened

Agreed that offering "Name and save" for an already-saved card is inappropriate. I've added support for DESFire cards to NfcSceneDeviceInfo and now go there instead of NfcSceneReadMifareDesfireSuccess when reading a saved card. The "Data" button goes to NfcSceneMifareDesfireData, and NfcSceneMifareDesfireMenu is now only accessible when scanning a new card.

bf64326

I also tried another approach, keeping NfcSceneReadMifareDesfireSuccess in both cases, and showing the 'More' button selectively based on scene history. It's in my desfire_wip branch (kevinwallace@26ef6e0, kevinwallace@a42c938) if you'd like to take a look, but after trying both, I prefer the approach in this PR.

I like current approach with adding DESFire to NfcSceneDeviceInfo scene

This makes it safe to emulate DESFire/EMV without clobbering card data.

kevinwallace · 2022-03-18T18:30:56Z

Good catch on the broken UID emulation behavior due to memory sharing -- I didn't look closely at the UID emulation code earlier to understand this was going on. It seems useful to still be able to emulate UID only, so I've moved NfcReaderRequestData out of the union so that it no longer shares memory with DESFire data. This means it's possible to safely emulate UID of EMV cards from the saved card menu, too, so I've also re-enabled it for those.

fcc1bfa

I'm also fine disabling it and leaving NfcReaderRequestData in the union if you prefer that approach -- let me know and I'll revise.

This can happen when a file doesn't allow unauthenticated reads (see the call to mf_df_parse_read_data_response in nfc_worker.c).

gornekich · 2022-03-20T16:42:05Z

Thanks for fixing UID emulation. Now everything looks good!

gornekich · 2022-03-23T13:47:47Z

Thanks for contribution!

Kevin Wallace added 2 commits March 14, 2022 15:35

Add basic Mifare DESFire reading, file/app browser

d4228fb

kevinwallace requested review from skotopes, DrZlo13 and gornekich as code owners March 14, 2022 23:12

Kevin Wallace added 2 commits March 15, 2022 16:44

kevinwallace force-pushed the desfire branch from b174e0b to 99ca048 Compare March 16, 2022 00:42

Add ability to save and load DESFire card data

f8474be

kevinwallace force-pushed the desfire branch from 99ca048 to f8474be Compare March 16, 2022 03:26

skotopes assigned gornekich Mar 16, 2022

Skip over NfcSceneDeviceInfo when viewing saved DESFire info

714915e

All of the same information is present in NfcSceneReadMifareDesfireSuccess, except for ATQA/SAK, which is implicitly known for all saved DESFire cards (0x4403, 0x20).

skotopes and others added 2 commits March 17, 2022 16:56

Merge branch 'dev' into desfire

f3fb08a

mf_df_clear: don't leak master key settings key versions

56ab13e

gornekich requested changes Mar 17, 2022

View reviewed changes

Kevin Wallace added 2 commits March 17, 2022 13:59

When opening a DESFire card from Archive, retain UID emulation behavior

8b85ff3

rm unnecessary \r\n

ff1fcbf

show Popup instead of leaving view in bad state

f5dbd6f

kevinwallace force-pushed the desfire branch from a935853 to f5dbd6f Compare March 17, 2022 22:21

gornekich self-requested a review March 18, 2022 10:51

gornekich reviewed Mar 18, 2022

View reviewed changes

Kevin Wallace added 2 commits March 18, 2022 10:10

Move NfcReaderRequestData out of union

fcc1bfa

This makes it safe to emulate DESFire/EMV without clobbering card data.

Display saved DESFire cards via NfcSceneDeviceInfo

bf64326

kevinwallace requested a review from gornekich March 18, 2022 18:31

Display and save file metadata even when contents are missing

9c071a6

This can happen when a file doesn't allow unauthenticated reads (see the call to mf_df_parse_read_data_response in nfc_worker.c).

Merge branch 'dev' into desfire

6286b9d

gornekich and others added 2 commits March 23, 2022 14:56

Merge branch 'dev' into desfire

c84228f

Merge branch 'dev' into desfire

e2d253c

gornekich approved these changes Mar 23, 2022

View reviewed changes

skotopes approved these changes Mar 23, 2022

View reviewed changes

skotopes merged commit 3857cd7 into flipperdevices:dev Mar 23, 2022

kevinwallace deleted the desfire branch March 23, 2022 21:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nfc: add basic Mifare DESFire support #1024

nfc: add basic Mifare DESFire support #1024

kevinwallace commented Mar 14, 2022 •

edited

kevinwallace commented Mar 16, 2022

skotopes commented Mar 16, 2022

kevinwallace commented Mar 16, 2022

gornekich left a comment •

edited

gornekich Mar 17, 2022

kevinwallace Mar 17, 2022

gornekich Mar 17, 2022

kevinwallace Mar 17, 2022

gornekich Mar 17, 2022

kevinwallace Mar 17, 2022 •

edited

kevinwallace commented Mar 17, 2022

gornekich left a comment •

edited

gornekich Mar 18, 2022

kevinwallace Mar 18, 2022

gornekich Mar 20, 2022

kevinwallace commented Mar 18, 2022

gornekich commented Mar 20, 2022

gornekich commented Mar 23, 2022

nfc: add basic Mifare DESFire support #1024

nfc: add basic Mifare DESFire support #1024

Conversation

kevinwallace commented Mar 14, 2022 • edited

What's new

Verification

Checklist (do not modify)

kevinwallace commented Mar 16, 2022

skotopes commented Mar 16, 2022

kevinwallace commented Mar 16, 2022

gornekich left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinwallace Mar 17, 2022 • edited

Choose a reason for hiding this comment

kevinwallace commented Mar 17, 2022

gornekich left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinwallace commented Mar 18, 2022

gornekich commented Mar 20, 2022

gornekich commented Mar 23, 2022

kevinwallace commented Mar 14, 2022 •

edited

gornekich left a comment •

edited

kevinwallace Mar 17, 2022 •

edited

gornekich left a comment •

edited