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.

Sign up for GitHub

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

Jump to bottom

U2F over NFC #1510

Closed
wants to merge 3 commits into from
Closed

U2F over NFC #1510

wants to merge 3 commits into from

Conversation

RvanBaarle
Copy link
Contributor

What's new

  • The ability to use U2F via NFC, instead of just over USB
  • A load of magic numbers, and some other issues (which I'll fix before I remove the draft status)

Verification

  • [ Describe how to verify changes ]

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

@hedger hedger added NFC NFC-related UI Affects UI New Feature Contains an IMPLEMENTATION of a new feature labels Aug 2, 2022
@reinerh reinerh mentioned this pull request Aug 9, 2022
Open
miili
miili suggested changes Aug 13, 2022
View reviewed changes
Copy link

@miili miili left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, learned something reading this implementation draft. Thanks for your work!

applications/u2f/u2f_nfc.c
buff_tx[0] = 0x02;
*buff_tx_len = 0;
FURI_LOG_D(TAG, "Something detected of length %d", buff_rx_len);
if(buff_rx_len >= 24) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverse logic here. Test for buff_rx_len < 24, eliminate a logical block.

applications/u2f/u2f_nfc.c
return true;
}
if(u2f_nfc->state == StateUnselected && ins == 0xa4) {
if(memcmp(&buff_rx[3], "\x04\x00\x08\xa0\x00\x00\x06\x47\x2f\x00\x01", 11) != 0) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add more insights what the byte sequences mean? Datasheets, documentation or comments.

@devsnek devsnek mentioned this pull request Sep 20, 2022
Closed
3 tasks
@gornekich
Copy link
Member

Hello everyone!
Recently we released new NFC stack in #3050 . Unfortunately, previous NFC API was not good enough to add new features like NFC over U2F. Now we finished working on new API and it is possible to make NFC over U2F application.

Thanks @RvanBaarle for PR! I will close it for now, since we are not planning to support NFC over U2F in our firmware. However, if you want to continue working on it, you can write application and submit it to https://github.com/flipperdevices/flipper-application-catalog repository. If you have any questions, feel free to contact me on Discord.

@gornekich gornekich closed this Nov 15, 2023
@ChristianTacke
Copy link

Well, if the core "U2F over USB" application is part of the main flipper OS (like it currently is), then really that same application should also handle the U2F over NFC.

  • It should use the same keys for both protocols. Or can an external U2F-over-NFC app use the same keys again in a proper in supported way?
  • Sharing the U2F code would be really nice too. Same question: Could an external U2F-over-NFC app reuse code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miili miili miili requested changes

@skotopes skotopes Awaiting requested review from skotopes skotopes will be requested when the pull request is marked ready for review skotopes is a code owner

@DrZlo13 DrZlo13 Awaiting requested review from DrZlo13 DrZlo13 will be requested when the pull request is marked ready for review DrZlo13 is a code owner

@hedger hedger Awaiting requested review from hedger hedger will be requested when the pull request is marked ready for review hedger is a code owner

@gsurkov gsurkov Awaiting requested review from gsurkov gsurkov will be requested when the pull request is marked ready for review gsurkov is a code owner

Assignees
No one assigned
Labels
New Feature Contains an IMPLEMENTATION of a new feature NFC NFC-related UI Affects UI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@RvanBaarle @gornekich @ChristianTacke @miili @hedger