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

Mifare classic refactor #121

Merged
merged 21 commits into from
Aug 17, 2019

Conversation

securechicken
Copy link
Collaborator

As I had quite a hard time understanding state of play in MifareClassic.c, this is a refactor that aims to be more maintainable, more readable, and more easy to build on.
This also fixes indeterminist or wrong handling of HALT command/state and WUPA/REQA in some cases (because HALT was not anticipated, or flags were not initialised consistently to check for REQA).

I tried to keep the commits as atomic as possible, so that it can be reviewed commit by commit to understand what have been done in which order.

Here is a list of what have been done:

  • remove LED and Random headers as not used (and cleaning LED.h spacing BTW),
  • clean ISO14443-3A from not quite readable kung-fu, and making WUPA/REQA handling more modular to enable use without duplicates in MifareClassic,
  • change 7bitsUID flag to a proper bool, and factor the way we init 4K and 1K settings,
  • factor CRC checks in AUTHED_IDLE and CHINESE_IDLE state,
  • ensure main process returns with a value in all cases, with a return value instead of returning in-place,
  • factor, optimize and fix REQA/WUPA handling, thanks to previous refactoring in ISO14443-3A,
  • factor block0 write check for WRITE and TRANSFER in AUTHED_IDLE,
  • factor changes to HALT state, fixing possible HALT in AUTHED_IDLE state, and fixing associated FromHalt bool init,
  • make NAK codes compliant with latest MF1S50YYX_V1 datasheet,
  • move access control related constants where they belong, set constants instead of in-code values, and make it clear that access control is not implemented yet,
  • ensure determinist state init, and various small code readability, compression, and commenting change.

I tested it with arbitrary reads and search/detect on SCL, PM3 and MCT readers with success. I cannot test with real applications for now, but will next week.

ShinHub added 21 commits August 17, 2019 17:10
…constant instead of in-code value, making clear access control is not implemented yet
…te, and fixing associated FromHalt bool init
@iceman1001 iceman1001 merged commit 28bb0eb into iceman1001:master Aug 17, 2019
@iceman1001
Copy link
Owner

excellent!

@securechicken securechicken deleted the mifare_classic_refactor branch September 28, 2019 16:50
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