Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NFC presence timeout in Android 13+ #52

Open
wants to merge 1 commit into
base: community
Choose a base branch
from

Conversation

StarGate01
Copy link

This PR fixes a breaking bug which I encountered on my smartphone (Samsung Galaxy S10+, running Android 13, specifically LineageOS 20-20240402-microG-beyond2lte). The bug breaks the communication with the id card while the secure element in the card is still computing key transformations, because Android times out the NFC connection. This leads to a complete failure of functionality.

The core issue is that the Android API NFCAdapter::enableReaderMode [1] apparently had a subtle implementation change regarding the tag presence detection, sometime before or during Android 13 [2].

Specifically, the mechanism how the Android OS detects the presence of a tag has been modified. Because different NFC chipsets and Android versions handle function differently, there is no real standard how presence detection should work [2]. Especially on Broadcom chips, these seem to be issues with how the enableReaderMode handles presence checks [4].

The EXTRA_READER_PRESENCE_CHECK_DELAY configuration parameter [6] (which is passed via the Bundle configuration option in enableReaderMode) controls the timeout after which the Android OS considers a tag as lost. By default, this is 125 (presumably milliseconds) [5]. This explains why I was observing OS-induced reconnection attempts in my logfiles, as a GlobalPlatform key transformation might take longer than 125 milliseconds. Apparently Android uses the timeout value set via IsoDep::setTimeout only for active communication, not for the automatic OS-level presence checks.

The value of 1000 for EXTRA_READER_PRESENCE_CHECK_DELAY translates to presumably one second. I am unsure if it would be wise to set this any higher, I fear the Android tag detection system might hang if the timeout is too long. One second works fine for my phone - you might want to test this on your side as well.

This appears to be a general issue with the app and Android 13+ on select models, regardless of LineageOS or any other Android distribution.

[1] https://developer.android.com/reference/android/nfc/NfcAdapter#enableReaderMode(android.app.Activity,%20android.nfc.NfcAdapter.ReaderCallback,%20int,%20android.os.Bundle)

[2] https://community.st.com/t5/st25-nfc-rfid-tags-and-readers/what-is-the-difference-between-android-11-and-android-13-for-nfc/td-p/121846

[3] https://github.com/frankmorgner/vsmartcard/blob/master/remote-reader/app/src/main/java/com/vsmartcard/remotesmartcardreader/app/MainActivity.java#L163

[4] https://community.nxp.com/t5/NFC/Tranceive-method-throws-a-tag-lost-exception-only-for-specific/m-p/665270

[5] https://android.googlesource.com/platform/packages/apps/Nfc/+/master/src/com/android/nfc/NfcService.java#235

[6] https://developer.android.com/reference/android/nfc/NfcAdapter#EXTRA_READER_PRESENCE_CHECK_DELAY

@CLAassistant
Copy link

CLAassistant commented May 15, 2024

CLA assistant check
All committers have signed the CLA.

@misery
Copy link
Contributor

misery commented May 27, 2024

Thank you very much for this. We already investigate your solution internally.

@SmallLars
Copy link
Contributor

SmallLars commented Jun 10, 2024

Thank you very much again. Your idea helped us to understand a bug we had with the Fairphone 3 a lot better.

When the Fairphone 3 got an update from Android 11 (8901.4.A...) to Android 13 (6.A...) some id cards no longer worked because of the problem you noticed. It looks like that only some older id cards are affected. LineageOS has exactly the same behavior on the Fairphone 3.

After a lot of communication with the Fairphone 3 Team (Thank you very much again) Android 13 started to work again with Version 6.A.028... The Problem was a change of the NFC package. For Android 11 they used "NQNfcNci" (from NXP/Qualcomm). But this package has no official support for Android 13 so they replaced it with "NfcNci" (from AOSP). After porting NQNfcNci to Android 13 by themself everything works fine again. So it looks like a bug in "NfcNci" and maybe LineageOS should go the same way and use "NQNfcNci". We can also confirm that a presence check delay of 1000ms helps to get old id cards working again with 6.A.025... which is using "NfcNci".

When we try to understand the situation it also makes sense: When there is a transaction with an explicit transmit timeout of 1 or 2 seconds, the presence check should not be interesting because the system should know there is probably a busy card.

Nevertheless, we tried your idea on a Samsung S20 with Samsung Android. Sadly there is at least one reproducible negative effect, where removal of the id card is no longer detected after the third contact. Also the (now expected) delay of the removed card detection does not feel very well. So based on the current situation we would not like to accept your change.

@StarGate01
Copy link
Author

some id cards

Very interesting observation. I suppose the ID cards use progressively newer chipsets (eg. the NXP P71D321) as they are issued each year, and older chipsets might be to slow to perform the key transformations in the default timeout window.

NQNfcNci vs. NfcNci

Thank you for the detailed explanation! This actually helps a lot. I will try to report the suggestion of using NQNfcNci in other builds of LineageOS to the LineageOS team once I figure out their communication channels. Are there any plans by the Fairphone team to upstream and share their ported implementation? Is there a preferred way to contact them?

reproducible negative effects

I see your reasoning concerning the detrimental side effects. I did some testing on my side as well, The timeout can be reduced to 500 ms without loss of functionality on my device, which might help with the observed latency / sluggishness. However, I do not observe the card not working after the third contact, it works every time for me - not sure what is going on there, but I agree that this is not what you would want to deliver to your clients as a default experience.

Maybe you want to consider a simple "compatibility switch" option in the app settings with a short explanation? My ID card is still valid for more than a year, it might take even longer until citizens have received faster cards. That option could then increase the timeout to 500 - 1000 ms, but would be disabled by default. You could even check for communication errors during the APDU transfer and suggest enabling this option to the user.

Thank you for the transparency and research!

@StarGate01
Copy link
Author

Issue report in the LineageOS project: https://gitlab.com/LineageOS/issues/android/-/issues/7268

@StarGate01
Copy link
Author

Backlink into related Fairphone support thread: https://forum.fairphone.com/t/nfc-ausweis-app-fehler-nach-pin-eingabe/105244/61?u=stargate01

@z3ntu
Copy link

z3ntu commented Jun 11, 2024

@StarGate01

Are there any plans by the Fairphone team to upstream and share their ported implementation? Is there a preferred way to contact them?

The sources for our NQNfcNci repository can be found here: https://gerrit-public.fairphone.software/plugins/gitiles/platform/vendor/nxp/opensource/packages/apps/Nfc/+log/refs/tags/rel/13/fp3/6.A.028.1 (right now only this tag and the release branch are pushed there, hopefully int/13/fp3 branch gets pushed soon but the contents are the same anyways)

There's also a few other relevant repositories (like platform/vendor/nxp/opensource/halimpl and platform/vendor/nxp/opensource/external/libnfc-nci) that are part of the Nfc stack but that's the most important one iirc.

@SmallLars
Copy link
Contributor

Is there a preferred way to contact them?

We see you already found the right way and got feedback.

Maybe you want to consider a simple "compatibility switch" option in the app settings with a short explanation? My ID card is still valid for more than a year, it might take even longer until citizens have received faster cards. That option could then increase the timeout to 500 - 1000 ms, but would be disabled by default. You could even check for communication errors during the APDU transfer and suggest enabling this option to the user.

We had a discussion @ option like this before some years when we had a lot of problems with extended length. Sadly the cost-benefit ratio is to low and we need to set other priories. I am in the same situation, my id card is still valid for two years.

@SmallLars
Copy link
Contributor

To complete the technical situation and maybe help to find the bug we would like to add a hint:

Maybe the EXTRA_READER_PRESENCE_CHECK_DELAY is not a problem while the card is busy. It looks like the way, "NfcNci" implements the presence check, destroys the secure messaging channel. The relevant part of the authentication with a german id card works as follows:

  1. ...
  2. Communication with the eID-Server
  3. Establishing a PACE channel with the PIN (secure messaging)
  4. Communication with the eID-Server
  5. Communication with the card <- Bug with NfcNci. Answer is 0x6988
  6. ...

When the response from the server in 4. takes to long (> 125 milliseconds), the presence check becomes active and destroys the PACE channel. In general a PACE channel should be destroyed when a command was received not matching the current secure messaging channel. With a fast server this is not a problem. Please consider these statements as speculation based on some circumstantial evidence.

@StarGate01
Copy link
Author

StarGate01 commented Jun 12, 2024

Thanks for the information, this matches my observation in another place.

I was trying to manage applets on some smartcards via GlobalPlatform, using the Fidesmo app. The same bug occurred there, specifically NfcNci would reset the connection during the two-way handshake of the GlobalPlatform secure channel authentication: INITIALIZE UPDATE is received and processed successfully, but takes to long on the card, and after a connection reset the next EXTERNAL AUTHENTICATE command fails with 6985 (The Get Challenge command has not been sent before the command). This indicates that the transient authentication keys were lost in the card reset in between the two parts of the handshake, similar to the bug in the PACE process, but independent of any server.

Looking at the log files, it appears that NfcNci actually power-cycles the NFC reader field once it detects a tag as unresponsive / lost, causing smartcards to reset.

@snoweuph
Copy link

snoweuph commented Jun 19, 2024

Is this Bug, why I always get? :

Error Code: Workflow_Card_Removed
Reason:
Did_Authentucate_Eac2_Card_Command_Failed

  • Card_Return_Code: COMMAND_FAILED

And no, I didnt remove the Card. (im also running lineage 13)
And Im holding it as still as posible. Also i did set my Original pin on this device (with an Older Lineage Version), so it cant be Hardware related (NFC works just fine for all other things, like YubiKeys)

Btw, why are Issues disabled?
also is: https://flathub.org/apps/de.bund.ausweisapp.ausweisapp2 offical? if yes, then please Verify it

@StarGate01
Copy link
Author

@snoweuph Yes, the app error you describe (see the attached screenshot) is caused by the connection terminating while the card is performing key computations, due to NfcNci timing out the presence detection.

signal-2024-06-20-095326

@StarGate01
Copy link
Author

StarGate01 commented Jun 20, 2024

I found even more apps struggling with this issue (specifically Nect Wallet). This prompted me to finish up and publish an LSPosed module, which injects the NFC configuration fix into any selected apps at runtime. It is open source at https://github.com/StarGate01/NFCPresenceFix , downloads are available as well. I hope this helps affected LineageOS users.

@snoweuph
Copy link

@snoweuph Yes, the app error you describe (see the attached screenshot) is caused by the connection terminating while the card is performing key computations, due to NfcNci timing out the presence detection.

signal-2024-06-20-095326

yea, exact Issue I got, thanks for your Work 👍

@snoweuph
Copy link

I found even more apps struggling with this issue (specifically Nect Wallet). This prompted me to finish up and publish an LSPosed module, which injects the NFC configuration fix into any selected apps at runtime. It is open source at https://github.com/StarGate01/NFCPresenceFix , downloads are available as well. I hope this helps affected LineageOS users.

Thats awesome ,I gonna give it a test rn

@rmnscnce
Copy link

rmnscnce commented Aug 23, 2024

I also published my NfcNci Patience module after encountering this issue on several apps. The difference here compared to @StarGate01's module is that this module directly hooks into the "Nfc Service" (com.android.nfc package) instead to ensure that the module can still take effect on apps that otherwise refuse LSPosed code injection (e.g. a lot of banking apps)

@StarGate01
Copy link
Author

@rmnscnce Great work! Your module works flawlessly! It is a lot better than mine, as it does not clash with Shamiko due to injecting directly into Android instead of the individual apps. Kudos!

@rmnscnce
Copy link

Thanks 😄 but my module would have never existed without your initial bug report, so kudos to you too for noticing (and reporting) the primary cause of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants