-
Notifications
You must be signed in to change notification settings - Fork 27
Some steps towards PACE support #37
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
base: main
Are you sure you want to change the base?
Conversation
…in passy. Log selected method in state machine
bettse
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the kind words!
This is a big PR, and it's not just adding some ASN1 stuff for PACE, but also making quite a few small changes to the way the app already works. I've added comments and I think there is material for a number of smaller PRs, and those could have further context on them (as mentioned in some of my comments).
I also believe that EC/ECDH support via mbedtls support in the flipper zero api is probably the biggest huddle for PACE support, so I probably won't merge PACE specific stuff until that is available (but this PR has plenty of non-PACE specific stuff we could still add). Have you started conversation with the flipper devs about enabling that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we need a whole script to comment out one call that won't change again (unless I'm missing something)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When asn1c is run again, it will overwrite the modifications made to lib/asn1/constr_SET_OF.c. I thought it would be useful to include a script to run effortlessly after running asn1c to disable unused, troubling code.
The issue being that qsort is used here, but it's not available in Flipper firmware.
|
Thanks for your extensive review and comments. I'll try to split the changes into smaller PRs to enable more focussed discussion on the changes. To get a better understanding before I'll do that, I'll have a look at your comments and reply here first. I was under the impression that EC/ECDH is supported in MbedTLS and thus available in Flipper firmware. Do you know where or how to check for availability of functions in the firmware? |
Not fully accurate. EC/ECDH is support by MbedTLS, but not all MbedTLS is exposed as part of the flipper SDK see the minus sign? that means it's not exposed |
Do you know if, as a temporal or definitive solution, it would be possible to include disabled functions by linking them statically in passy? |
I think you basically mean to include mbedtls into Passy. This is far from ideal: I'd only want to explore that if there was serious pushback from the flipper devs about including the EC/ECDH methods, or significant delay |
|
I saw activity in the firmware repo, so I opened an issue about ECDH (flipperdevices/flipperzero-firmware#4281) which I believe would allow PACE. |
Dear Eric,
Thanks for your efforts on Passy. I have been working on reading eMRTDs with Flipper too, before there were applications and before the big change of NFC reading. Your project has inspired me to work on reading eMRTDs with Flipper Zero again.
I am working on some steps towards PACE support and would like your feedback on my progress and direction. All current features should be stable, so mering this PR should not cause any disruptions.