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

[remote-reader] Fix ATR for 14443 Type B cards #251

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

mildsunrise
Copy link
Contributor

Hi, thanks for the project! It has been very useful so far :)

For Type B cards, the Android app is currently calling IsoDep#getHiLayerResponse() to get the data to use as historical bytes in the generated wrapped ATR. According to the documentation, this method returns "a subset of the data in the ATTRIB response".

This is not compliant to the PC/SC spec; in the section defining what the wrapped ATR should be, it reads:

PC/SC part 3 section 3.1.3.2.3

For 14443 Type B smartcards, the historical data should be constructed from two fields of the ATQB (the application data and the protocol info) and a nibble from ATTRIB. This pull request implements this correctly by grabbing the necessary info from NfcB.

@frankmorgner
Copy link
Owner

Thank you. Could it be that the maximum frame size is miscalculated? ISO 14443-3:
grafik
It looks like a maximum frame size code of, e.g., 0 should be mapped to a maximum frame size of 16 bytes.

@mildsunrise
Copy link
Contributor Author

you are completely right, I forgot to to translate the maximum frame size code into actual size. done.

another thing I forgot to mention is that this code has the edge case where it can't differentiate between MBLI=0 and MBLI=1. right now it's returning the 2nd thing, maybe it would be better to be conservative and return MBLI=0...

@frankmorgner
Copy link
Owner

I don't have experience on which value to choose as best guess, so I'd leave this up to you.

@mildsunrise
Copy link
Contributor Author

I also don't have much experience... I'll ask around

@frankmorgner
Copy link
Owner

Hi! Any updates on this?

@frankmorgner frankmorgner merged commit 44825be into frankmorgner:master Oct 30, 2024
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.

2 participants