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

SmartRider NFC Parser #203

Merged
merged 22 commits into from
Sep 18, 2024
Merged

SmartRider NFC Parser #203

merged 22 commits into from
Sep 18, 2024

Conversation

jaylikesbunda
Copy link
Contributor

@jaylikesbunda jaylikesbunda commented Sep 2, 2024

What's new

  • Adds support for parsing SmartRider cards, the public transport smart card used in Western Australia.
  • Implements reading and verification of SmartRider cards (Mifare Classic 1K).
  • Displays key information:
    • Balance
    • Serial number
    • Concession type
    • Auto load threshold
    • Value amount
    • Information about last ten transactions

Testing

  • Tested with multiple SmartRider cards of varying types and usage patterns
  • A sample card dump for testing is provided in the file: Smartrider.nfc.txt

Checklist for reviewers

  • Verify correct parsing of card data
  • Confirm accurate display of all key information
  • Check for Mi-Fare Classic 1K false positives
  • Uploaded the firmware with this patch to a device and verified its functionality
  • Review error handling and edge cases

Additional Notes

used data from https://github.com/SkryptKiddie/smartrider

new parser for SmartRider cards, a public transport smart card system used in Western Australia.
extracts and interprets key information from the card, including:
-Current balance
-Card serial number
-Concession type
-Purchase cost
-Details of the last two trips (including tag on/off status, cost, route, transaction number, and journey number)
- removed all logging to simplify output.
- used early returns for clearer error handling.
- optimized setups outside loops to improve memory use.
- simplified flows by removing unnecessary loops.
- placed variables closer to their use for better readability.
- cached data blocks to streamline data handling.
- added a helper function for parsing trips, reducing redundancy.
- corrected loop counter types to avoid compile-time errors.
- removed transaction (txn) and journey (jrn) numbers to declutter the trip details.
- shortened "previous trip" to "prev trip" to optimize screen space usage.
added auto load field "threshold amount / reload amount"
changed serial to display first two digits as SR0 for consistency with physical card.
Copy link

@ZProLegend007 ZProLegend007 left a comment

Choose a reason for hiding this comment

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

Exciting!

@Willy-JL Willy-JL added the feature New feature or request label Sep 4, 2024
Copy link
Member

@Willy-JL Willy-JL left a comment

Choose a reason for hiding this comment

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

need to add more checks than just "the blocks we need have been read", currently it gives false positives for any mifare classic 1k.
could check if keys match a known set of keys, or could check the layout and content of data.
important is that parse() returns true only if this card is actually a SmartRider card

- Added definitions for STANDARD_KEY_2 and STANDARD_KEY_3
- Enhanced smartrider_verify function to check for all three specific keys:
  - STANDARD_KEY_1 in Sector 0 as Key A
  - STANDARD_KEY_2 in Sector 6 as Key A
  - STANDARD_KEY_3 in Sector 6 as Key B
- Implemented read operations to verify actual key values stored on the card
- Added comparisons between read key data and expected key values
- Improved debug logging for each step of the verification process
@jaylikesbunda
Copy link
Contributor Author

unfortunately i don't have other mifare classic cards to test with but i can confirm it reads and compiles correctly still with actual verification checks

@Willy-JL
Copy link
Member

Willy-JL commented Sep 4, 2024

the checks are required in parse() too, this is what is giving false positives

- Added key verification for sectors 0 and 6
- Implemented do-while loop structure for early exit on verification failure
- Moved block readability checks inside verification process
- Added parsed flag to indicate successful parsing
- Updated return value to reflect parsing success
- Maintained existing parsing logic and output format
recieved some cuid cards today so was able to test for myself can confirm works... finally
changes made: 
-updated key assignment in smartrider_read
-simplified key verification in smartrider_parse
-improved error handling and logging
-streamlined data parsing process
-corrected key checking logic
-added checks for required block readability
-improved flow control with strategic breaks
-adjusted block data access method
- refactored `smartrider_verify` and `smartrider_read` by abstracting repeated key operations into `authenticate_and_read` function for improved code maintainability.
- optimized `smartrider_read` by introducing a loop for key setup, reducing redundancy and improving efficiency.
- streamlined error handling in `smartrider_read` by replacing do-while loop with conditional checks.
- changed standard key references to use `standard_keys` array indices
@jaylikesbunda
Copy link
Contributor Author

recieved some cuid cards today so was able to test for myself can confirm works... finally
sorry bout the runaround, let me know if anything else needs changing

@RogueMaster
Copy link

Hi @jaylikesbunda , seems like you have an extra file here. Please correct the right version of the file in the right location and re-test it is working as intended.

@jaylikesbunda
Copy link
Contributor Author

jaylikesbunda commented Sep 6, 2024

Hi @jaylikesbunda , seems like you have an extra file here. Please correct the right version of the file in the right location and re-test it is working as intended.

i can't believe i missed that thank you. everything is still working just uploaded the formatting commit incorrectly

if i make any more commits after this point they will be fully tested and just because im bored

jaylikesbunda and others added 5 commits September 8, 2024 14:31
-removed last trip/prev trip wording and replaced with "Trip History" header
-added date in front of each transaction 
-only shows transaction cost if it's higher than 0
-changed tag on/tag off to +/- to save room
-added 8 more transactions to Trip History
-verified still working and formatted with fbt
-added bounds checking for all block accesses to prevent out-of-range memory access
-implemented improved error handling with an error_occurred flag
-introduced a maximum iteration count for date calculation to prevent infinite loops
-used snprintf with size limits for all string operations to avoid buffer overflows
-added validation for trip count to ensure it doesn't exceed the maximum allowed
-implemented checks to skip unread or out-of-range blocks during trip parsing
-added safeguards against corrupted or invalid timestamp data
- replaced do-while loop with direct error checks in smartrider_parse
- optimized key verification using direct memcmp in smartrider_verify
- introduced inline functions for common operations (e.g., set_key, read_le16)
- replaced bubble sort with insertion sort for trip data
- simplified date calculation using a lookup table for days in month
- used uint_fast8_t for loop counters to allow compiler optimization
- added __attribute__((hot)) to key functions for aggressive optimization
- removed redundant variable declarations and function calls
- optimized memory usage with static const arrays for required blocks
- simplified error handling in smartrider_read and authenticate_and_read
- used __builtin_memcpy and __builtin_memcmp for potential compiler optimizations
- tested and formatted with fbt
-renamed "Trip History" to "Tag On/Off History"
-fixed date calculation to account for leap years
-misc changes
Copy link
Member

@Willy-JL Willy-JL left a comment

Choose a reason for hiding this comment

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

LGTM, nice work :D

@Willy-JL Willy-JL merged commit 65df2e4 into Next-Flip:dev Sep 18, 2024
2 checks passed
@ZProLegend007
Copy link

ZProLegend007 commented Sep 19, 2024

I updated to the latest commit and tested on my own smartrider, plus another to double check and it didn't read any different than it used to. Just tries and fails to unlock with classic dictionary and presents it as a plain mifare card 😕🧐

Persistent after a reinstall

@Willy-JL
Copy link
Member

because you did not grab the keys. you need the keys. you can use nested and mfkey to crack them most likely. or install #207 which has noproto's mifare classic improvements, and managed to get all keys on its own

@jaylikesbunda
Copy link
Contributor Author

I updated to the latest commit and tested on my own smartrider, plus another to double check and it didn't read any different than it used to. Just tries and fails to unlock with classic dictionary and presents it as a plain mifare card 😕🧐

Persistent after a reinstall

what willy said, you need to be able to read the whole card so you gotta crack the keys and make sure it says
Keys Found 32/32
Sectors Read 16/16
maybe someday will be able to figure out how the unique keys are generated so can unlock on the fly

@Willy-JL
Copy link
Member

Willy-JL commented Sep 19, 2024

KDF (key derivation function) is the term :D yes would be cool, but with noproto's MFC improvements on the way it might not be needed after all xD

@ZProLegend007
Copy link

Ahh I see, thank you very much for the elaboration. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants