-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[LFRFID] Added Support for Securakey Protocol #3697
Conversation
Support both 26- and 32-bit format Solves issue #2619
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. |
@zinongli see GDB Output: https://github.com/flipperdevices/flipperzero-firmware/actions/runs/9454491882/job/26042147462?pr=3697 Your implementation is failing RFID unit tests. |
Un-draft when ready |
@skotopes I'm getting the same result with my CLI as GDB. It hits |
The crash itself happens in your code. Use |
@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. |
@zinongli sure drop in in voice channel and tag me in chat |
|
Un-draft when all fixed |
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. |
Artificially generated key write failed. |
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? |
What's new
Verification
Checklist (For Reviewer)