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

Improve loclass logic for readers doing keyrolling. #50

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

nvx
Copy link
Contributor

@nvx nvx commented Sep 24, 2023

Improve loclass logic for readers doing keyrolling. Writing to the loclass log is also now deferred until the CSN changes for speed.

What's new

Removes the forced anticoll between MAC collections except for when the CSN changes. This speeds up the collection process and should be more reliable with readers doing keyrolling.

Verification

  • Perform loclass and ensure it is still able to recover keys from a legacy elite keyed reader.

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

picopass/picopass_worker.c Outdated Show resolved Hide resolved
bettse
bettse previously approved these changes Sep 24, 2023
Copy link
Collaborator

@bettse bettse left a comment

Choose a reason for hiding this comment

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

I don't claim to know all the details of loclass, but overall the codes makes sense.

@bettse
Copy link
Collaborator

bettse commented Sep 24, 2023

I used the nfc-iclass project to make a "KDE" keyroll config card, which I used on a Rev A reader. I ran the new code against it, downloaded the file, and was able to use https://loclass.ericbetts.dev/ to calculate the key I'd configured 😁.

Writing to the loclass log is now deferred until the CSN changes for speed.
@nvx nvx force-pushed the picopass_improve_loclass_keyroll branch from 7e09c15 to 98ab7f2 Compare September 25, 2023 01:33
@nvx
Copy link
Contributor Author

nvx commented Sep 25, 2023

Fixed that bug with the numbering in the loclass log being wrong and also made the code a little more defensive in the emulation loop to stop when finished rather than relying on the UI to stop the emulation.

@bettse
Copy link
Collaborator

bettse commented Sep 25, 2023

Pulled down the new code and used it again with my keyrolled reader. So fast! Log looks good and works with loclass.ericbetts.dev. I'm going to use my new found poweres to approve try the "Approve and run" in the PR.

@bettse
Copy link
Collaborator

bettse commented Sep 25, 2023

Looks like I can approve, but merging is blocked 👍

@skotopes
Copy link
Member

@bettse looks like you are missing in codeowners. Can you make PR with it?

@skotopes skotopes merged commit 25a57e2 into flipperdevices:dev Sep 25, 2023
@bettse bettse mentioned this pull request Sep 25, 2023
3 tasks
@skotopes
Copy link
Member

Also don't forget changelog + catalog update

@bettse
Copy link
Collaborator

bettse commented Sep 25, 2023

We've also got #47 open, so we could do the bump after that

@skotopes
Copy link
Member

okok

@nvx nvx deleted the picopass_improve_loclass_keyroll branch September 25, 2023 01:56
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.

3 participants