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

NFC - Machine Readable Travel Documents #1866

Closed
wants to merge 61 commits into from
Closed

Conversation

qistoph
Copy link
Contributor

@qistoph qistoph commented Oct 11, 2022

Maybe not quite a finished PR yet. I'm looking for some feedback to make sure I'm going in the right direction.

Especially with the new FAP's, I can see you might prefer this to be an external application (FAP). On the other hand, maybe you're looking to support more and more NFC cards with the base firmware.

Please let me know your thoughts and any feedback on the code.

What's new

  • Read some basic info from Machine Readable Travel Documents (spec. ICAO9303)
  • Support Basic Access Authentication

Verification

  • Start NFC reader
  • Offer an MRTD to the reader
  • Perform the Auth step

TODO

  • PACE authentication
  • Indicate used authentication (BAC / PACE)
  • Card Access Number access
  • Active Authentication
  • Passive Authentication (maybe even against ICAO Master List)
  • Dump (selected) files (for external usage)
  • Save (as much info as possible)
  • Emulate (as much as possible) a saved MRTD
  • Support NFC-B MRTDs
  • Move helper files to helper folder
  • Determine size of files from header, instead of reading till end
  • Remove some testing/debug statements and logging
  • Show information about LDSv2 apps
  • Save/Load authentication parameters

Checklist (For Reviewer)

  • 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

The TextInput would not allow another key press if the template that is
going to be overwritten is over the over the buffer size.

This change clears any default text, before checking the length of
entered input.
- Check digit
- KMRZ
This reverts commit 3458ded128a3c13ddd18484c2e1be2945e951a28.
@craftbyte
Copy link

When testing with a Slovenian passport, I get a MemManage restart.

@qistoph
Copy link
Contributor Author

qistoph commented Nov 8, 2022

When testing with a Slovenian passport, I get a MemManage restart.

@craftbyte, would you be able to share some logging and debugging details with me? You can reach me at [my username]@gmail.com

@ivorsmorenburg
Copy link

<3

@soter19
Copy link

soter19 commented Nov 11, 2022

Just to inform, Brazilian passports work flawlessly, I've tested 5 different and all worked as expected, awesome job @qistoph !

@ezhevita
Copy link
Contributor

ezhevita commented Nov 20, 2022

While I was testing with Russian international passport, no data from EF_DG1 were extracted
I will look into it and maybe send you a patch to fix it

@ezhevita
Copy link
Contributor

Log fragment:

66841725 [D][Mrtd] Read and parse COM (011E)
66841728 [D][Mrtd] Send select EF: COM (0x011E)
DES add: ***
DES buf: ***
DES add: ***
DES buf: ***
MAC: ***
DES add: ***
DES buf: ***
66841772 [D][Mrtd] Selected COM
66841774 [D][Mrtd] Read binary, offset: 0
DES add: ***
DES buf: ***
DES buf: ***
MAC: ***
66841808 [I][Mrtd] APDU answer is not 0x9000, but 0x6B00
66841810 [E][Mrtd] Failed to read
66841812 [E][Mrtd] Could not read COM
66841814 [D][Mrtd] Read and parse DG1 (0101)
66841817 [D][Mrtd] Send select EF: DG1 (0x0101)
DES add: ***
DES buf: ***
DES add: ***
DES buf: ***
MAC: ***
66841846 [I][Mrtd] APDU answer is not 0x9000, but 0x6988
66841848 [I][Mrtd] 'secure messaging data objects are incorrect'
66841851 [E][Mrtd] Failed select EF 0x0101
66841853 [E][Mrtd] Could not select DG1
66841876 [D][NFC] Read passport, auth: 1, success: 1

@skotopes
Copy link
Member

Hi ;-) Looks like it make sense to move this feature into separate application, doesn't it?

I'll switch this PR to Draft, un-draft it when ready.

@skotopes skotopes marked this pull request as draft December 10, 2022 20:16
@Semptum
Copy link

Semptum commented Dec 10, 2022

Hi ;-) Looks like it make sense to move this feature into separate application, doesn't it?

I'll switch this PR to Draft, un-draft it when ready.

Maybe decoding yes but I'd argue it makes sense to at least detect mrtds in stock firmware.

@skotopes
Copy link
Member

@gornekich what do you think?

@Llama-Master
Copy link

wow good job

@pawisoon
Copy link

Hey! Fantastic job! Need any help?

@qistoph
Copy link
Contributor Author

qistoph commented Jan 30, 2023

Thanks @pawisoon!

I was really hoping to have this included in the firmware. The suggestion to convert into an app is a bit of a disappointment.

If anyone is willing to help me get that started, I'm willing to reconsider working on it.

@skynet01
Copy link

@qistoph is there a limitation to what travel passports can be read? When I try to authenticate US passport flipper crashes with furi_check error.

@pawisoon
Copy link

@qistoph is there a limitation to what travel passports can be read? When I try to authenticate US passport flipper crashes with furi_check error.

You need to do BAC/PACE access control - based on DoE, DoB and document number in order to access Chip data. You can read everything whats present on data page + high resolution photo + sometimes some optional data is present (see ICAO 9303).

The fingerprints are not readable by regular users in any passports, need a cert that only country officials have access to. Most agencies won't even do this themselves due to the complexity and cost of reading them from the chip.

@pawisoon
Copy link

pawisoon commented Mar 11, 2023

@qistoph is there a limitation to what travel passports can be read? When I try to authenticate US passport flipper crashes with furi_check error.

Sorry to reply again. Have you checked your DoE, DoB and document number correctness? Im not sure but maybe you are holding some newest model of US passport(post 2020) that migth be PACE only. You could verify this with a passport reader e.g. https://apps.apple.com/us/app/readid-me/id1463949991
After you read NFC using the above app, in result screen, go to security tab and see value of "Type of access control". You can also see what features chip supports above it.

@skynet01
Copy link

I tried different access types (BAC and Any). If I mistype any of the fields on purpose I get the can't authenticate error. My passport is before 2020. I tried it with 2 different passports that are both US and have the same issues on both of them.

@pawisoon
Copy link

pawisoon commented Mar 11, 2023

I tried different access types (BAC and Any). If I mistype any of the fields on purpose I get the can't authenticate error. My passport is before 2020. I tried it with 2 different passports that are both US and have the same issues on both of them.

Maybe its the implementation issue specific to US passport. Is the passport reader app I linked working with your document? Making sure NFC chip is intact and working.

@skynet01
Copy link

Yeah the app you linked works great. One passport is from 2015 and another from 2019 both are read correctly by the app.

@pawisoon
Copy link

@mranostay
Copy link

mranostay commented Mar 12, 2023

@skynet01 Having the same issue with a US passport. And yes that ReadID application reads my passport data correctly. I do notice the buffer[100] in mrtd_read_parse_file is probably too small for some of the tag data on the COM page on the US passport.

@qistoph Seems for EF.COM page is the length is 265 for a US passport. Which seems to be a lot more data than other passports I've tried.. Has the multibyte BER length check been tested?

8608050 [D][Mrtd] Selected COM
8608052 [D][Mrtd] Read binary, offset: 0
8608055 [T][Mrtd] Send APDU, lc: 0, le: 0
...
DATA LENGTH: 283
BER length of 2 bytes
byte 0: 01
byte 1: 09
DO87.Tag: 87
DO87.Length: 265
... crashes here in mrtd_bac_decrypt_verify_sm (can confirm via backtrace on debugger board) ...

EDIT: Fixed the issues and here is the PR with the changes qistoph#1

@skynet01
Copy link

@mranostay I can confirm that your fix solved the problem for me and passports read correctly now :)

@hacdias
Copy link

hacdias commented Apr 1, 2023

Are there any plans on this being worked on / merged / made an app?

@qistoph
Copy link
Contributor Author

qistoph commented Apr 1, 2023

I'm not turning it into an app. If someone else is and needs some MRTD help, feel free to ask me.

I was really hoping it to be part of the firmware, but that doesn't seem to be happening

@hacdias
Copy link

hacdias commented Apr 2, 2023

@qistoph thanks for the reply. If this won't be allowed as part of the official firmware, would you consider moving this PR to a different firmware?

@skotopes I see you made this PR a draft. Any reason why it should be an app instead?

@skotopes
Copy link
Member

skotopes commented Apr 2, 2023

@hacdias not going to merge it in this form, this should be standalone application.

Also we are currently refactoring NFC, so I highly advice to wait till we finish with it.

@SolarSciencePup
Copy link

How is it going?

@skotopes
Copy link
Member

@SolarSciencePup I'd say at this point it can be closed. We really'd like to see it as external app.

@skotopes skotopes closed this Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature Contains an IMPLEMENTATION of a new feature NFC NFC-related UI Affects UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.