search

flipperdevices / flipperzero-firmware

Flipper Zero firmware source code
https://flipperzero.one
GNU General Public License v3.0
12.56k stars 2.68k forks source link

[LFRFID] Added Support for Securakey Protocol #3697

Closed zinongli closed 3 months ago

zinongli commented 3 months ago

What's new

Verification

Checklist (For Reviewer)

zinongli commented 3 months ago

After further research, I learned that wiegand parity bits are, as in the name, parity bits that needs to be calculated. I will reopen this PR after adding the parity calculation in the encoder.

skotopes commented 3 months ago

@zinongli see GDB Output: https://github.com/flipperdevices/flipperzero-firmware/actions/runs/9454491882/job/26042147462?pr=3697

Your implementation is failing RFID unit tests. use ./fbt LIB_DEBUG=1 FIRMWARE_APP_SET=unit_tests flash_usb_full and then run unit_tests in device cli

skotopes commented 3 months ago

Un-draft when ready

zinongli commented 3 months ago

@skotopes I'm getting the same result with my CLI as GDB. It hits test_lfrfid_protocol_em_read_simple() and just stops moving forward. I'm very new to flipper development. Do you have any tips on where I should be looking at to debug? My assumption is me extending the protocol dictionary makes test_lfrfid_protocol_em_read_simple() unhappy. But I couldn't understand why this function should care if the changes I'm making is irrelevant to it.

skotopes commented 3 months ago

@skotopes I'm getting the same result with my CLI as GDB. It hits test_lfrfid_protocol_em_read_simple() and just stops moving forward. I'm very new to flipper development. Do you have any tips on where I should be looking at to debug? My assumption is me extending the protocol dictionary makes test_lfrfid_protocol_em_read_simple() unhappy. But I couldn't understand why this function should care if the changes I'm making is irrelevant to it.

The crash itself happens in your code. Use bt after flipper crash and then check where exactly in your code it crashes.

zinongli commented 3 months ago

@skotopes I see. Thank you. If by crash you mean unit_tests making CLI stuck, I couldn't type any commands when it is stuck. If you mean crash in like everyday use, I can't trigger any bug or crash when I flash this build and use it. Do you mind if we talk on discord? I could use some help working with this.

skotopes commented 3 months ago

@zinongli sure drop in in voice channel and tag me in chat

skotopes commented 3 months ago
skotopes commented 3 months ago

Un-draft when all fixed

zinongli commented 3 months ago

  • Key generation on flipper produces invalid card

  • 5577 write is unsuccessful

  • emulating freshly created key crashes proxmark

I think I can guess the reason for artificially generated keys to be invalid and crash pm3. The decoded data includes one byte to represent the bit-length of encoded data, and it has to be 0x1A or 0x20. If not, the generated key will not be recognized by this decoder itself as it only treats 0x1A and 0x20 bit lengths as can-be-decoded.

I'm surprised the T5577 writing failed. I have reports from another user saying that it succeeded with cloning existing fobs. Just to clarify, is it the writing failed, or is it the written T5577 failed on a reader/pm3? And are you writing an artificially generated key or an existing key? It's probable that the bit-length issue can cause encoder to not function normally on artificial keys as it fails to find correct bit-length.

I will fix the bit length issue to not allow user manually change bit length. This fix should resolve all the problems with flipper-to-flipper, flipper-to-T5577, flipper-to-pm3, flipper-written-T5577-to-pm3 pathways with artificial keys.

However, the last two checksum bytes will still need some more reverse engineering before I can make them calculated based on facility code and card number, instead of just copying from the fobs that's being read like it does now. This problem affects the artificially-generated-key-to-official-reader pathway since only the official reader checks checksum. The primary scenario of this pathway I can think of is someone trying to create a key on flipper based on pm3 reading of an existing fob. So if we can integrate the pipeline to be more compatible with pm3 outputs, it should solve most of the use cases on this pathway. I might need your help on this since I don't own a pm3. By any chance can you send me a pm3 reading of a securakey fob?

The bit length fix will be done today or tomorrow. Checksum might need more time.

skotopes commented 3 months ago

Artificially generated key write failed.

zinongli commented 3 months ago

The issues encountered with artificially generated keys should be resolved with the last commit. The bit-length issue is patched.

I currently can't create a better implementation for checksums due to lack of knowledge on its algorithm. As of the current state, there should be no scenarios that can cause anything to crash. This checksum issue wouldn't affect any usage on genuine fobs since they would have correct checksums. The only possible less than ideal scenario would be when a user artificailly generates a key and can't open a door even though the facility code and card number are correct, due to checksum failure. This issue is not resolved in PM3's repo either. So I propose we leave that part for future improvements. If, in the future, there are discoveries of the algorithm, I will update this code and open a new PR. What do you think?