Skip to content

NFC: Add support for Gallagher access control (MIFARE Classic only) #3306

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

Merged
merged 14 commits into from
Feb 9, 2024

Conversation

nickmooney
Copy link
Contributor

@nickmooney nickmooney commented Dec 22, 2023

What's new

Based on the reverse-engineering work presented by megabug in this repository and this Kawaiicon talk, this pull request implements the ability for Flipper Zero to parse Gallagher cardholder credentials from MIFARE Classic cards.

Note that this implementation is relatively limited: it will only work if the Gallagher installation is using the default site keys, or if the MIFARE Classic dictionary on the Flipper contains the site-specific keys. Additionally, only one cardholder credential (the default credential, held in sector 15) is currently supported. Emulation of this type of Gallagher credential is already supported by the Flipper's MIFARE Classic emulation functionality.

This is my first contribution to the Flipper Zero ecosystem; I'm open to feedback.

Verification

If you don't have access to a Gallagher MIFARE Classic, you can write/emulate the following blocks:

60: A3A3A31EA3A3A3F85C5C5CE15C5C5C07
61: 7777772E6361726461782E636F6D2020
62: 00000000000000000000000000000000
63: 160A91D29A9C787788C1B7BF0C13066E

This encodes a Gallagher credential with region code 1, facility code 2, card number 3, and issue level 4. The keys in the last block are the default Gallagher site keys -- the first is already present in the Flipper MIFARE Classic dictionary.

The tag should parse like shown:

Screenshot 2023-12-22 at 15 57 37

Note that the facility / region codes are displayed combined (i.e. an alphabet letter A-P for the region, followed by the numeric facility). I'm open to displaying this data in other formats.

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

@nickmooney nickmooney changed the title Add support for Gallagher access control (MIFARE Classic only) NFC: Add support for Gallagher access control (MIFARE Classic only) Dec 22, 2023
@hedger hedger added the NFC NFC-related label Dec 24, 2023
Copy link
Member

@gornekich gornekich left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

gallagher_util shouldn't be part of firmware in NFC library. The perfect solution would be to put this lib in NFC application helpers and expose this API for plugins. We don't have this yet, but an example in applications/examples/example_plugins_advanced/ may give you tips how that can be done.

I suggest two solutions. You can add NFC application API for plugins with gallagher_util wrapped as it's done in example_plugins_advanced. Or you can temporary put gallagher_util inside gallagher plugin and we will rework that in future.

Copy link
Member

@gornekich gornekich left a comment

Choose a reason for hiding this comment

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

Let me know what you choose or if you have any questions.

@nickmooney
Copy link
Contributor Author

Thanks, @gornekich! I'll take a crack at the plugins approach, but it might be a bit after the holidays til I get around to it.

Copy link
Member

@gornekich gornekich left a comment

Choose a reason for hiding this comment

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

I think you are on the right way.
One more time: gallagher_util shouldn't be in lib/nfc, it should be in NFC application. And NFC app should expose API from gallagher_util, so that gallagher plugin could resolve symbols using NFC app API

* It is also exposed to plugins to allow them to use the application's API.
*/
#include <stdint.h>
#include <nfc/helpers/gallagher_util.h>
Copy link
Member

Choose a reason for hiding this comment

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

The gallagher_util should become part of NFC app. It shouldn't be in lib/nfc.

@gornekich gornekich marked this pull request as draft January 12, 2024 08:34
@nickmooney
Copy link
Contributor Author

Thanks - I've got it near working locally. This WIP push wasn't supposed to update the PR, I forgot GitHub did that.

At the moment I've got the functionality exposed as part of the plugins API and housed within the NFC app. All I've got left to do is expose the constants defined in the utils headers to the plugins. Any suggestions? Currently getting complaints about the symbols being unresolved since they're no longer linked directly.

@nickmooney
Copy link
Contributor Author

I'll drop a comment on this PR when it's ready to be looked at as well

@gornekich
Copy link
Member

Hard to say without looking at code. You can push code and may be I will come up with a solution :)

@nickmooney
Copy link
Contributor Author

Here you go. This is still a WIP - but I'm open to any solutions. It "should work" in the sense that we're now passing a composite resolver all the way through to flipper_application_alloc with the nfc_app_api_gallagher_deobfuscate_and_parse_credential function exposed, but we're still getting crashes. Also, as mentioned before, I'm not sure how to expose the constants - I think I'm on the right track with API_VARIABLE, but at the moment I'm defining them directly in the API interface file just so I can at least get the function resolution stuff working (and then I'll sort the rest out).

Notes:

  • We haven't changed nfc_supported_cards_load_context_free to free the resolver yet, there's a TODO comment about it
  • We've duplicated the code to initialize the composite resolver three times, which is ugly and can be fixed after figuring out other issues.

@nickmooney
Copy link
Contributor Author

You think me leaving furi_assert(false) in there could have caused any issues?

Just kidding. Give me a bit more to sort the rest out, then I'll comment again when it's ready for review.

@skotopes
Copy link
Contributor

Btw furi_assert(false) == furi_crash()

@nickmooney
Copy link
Contributor Author

Btw furi_assert(false) == furi_crash()

Yeap -- I had completely forgot that I left this in while debugging another issue. I thought my symbol resolver was broken but in fact it was working perfectly, and hitting this line.

@nickmooney
Copy link
Contributor Author

Okay. Things are working.

I'd like some feedback on the following:

  • Currently, I'm passing the ElfApiResolver via the LoadContext -- however, the resolver should exist for the lifetime of the plugin, not just during load -- and so it doesn't seem appropriate to free the composite resolver when we free the load context. Should the resolver perhaps live somewhere else? flipper_application_free does not free its API interface when torn down...
  • How can I get the linker to see these symbols? Currently things are working but we still get the following:
fbt: warning: build/f7-firmware-D/.extapps/gallagher_parser.fal: app may not be runnable. Symbols not resolved using firmware's API: {'nfc_app_api_gallagher_deobfuscate_and_parse_credential', 'NFC_APP_API_GALLAGHER_CARDAX_ASCII'}

Thanks!

@nickmooney
Copy link
Contributor Author

Hey @gornekich, just pinging you on this. It's fully functional except for the compiler warning that I need some input to resolve. Do I need to change the status on this PR? No rush, just want to make sure I was clear. Thanks!

@gornekich
Copy link
Member

@nickmooney thanks! I will review the code in a few days

@gornekich gornekich marked this pull request as ready for review February 8, 2024 17:59
@gornekich gornekich requested a review from gsurkov as a code owner February 8, 2024 17:59
@gornekich
Copy link
Member

gornekich commented Feb 8, 2024

@nickmooney I am sorry for pushing to your branch. I think it's better than making a big patch.

First of all, thanks for your work. Actually you made everything right, and I made small changes.

Answering your questions:

  1. Allocating composite api resolver for 3 times is not necessary. The instance doesn't requires too much memory and it's better to allocate this instance along with NfcSupportedCards, since it doesn't change during nfc app execution.
  2. The warning logs during plugins load were because composite api resolver searches symbols in last loaded api interface. So when composite api resolver couldn't find symbols from firmware_api interface in nfc_api_interface, we got warnings in logs. I changed log level for this message to Trace. The reason behind api resolver searches symbols in last added api interface is that it's possible to override some symbols from firmware api in application api.
  3. Unfortunately we can't avoid fbt warning when launching app for now. The warning shows that there are some symbols that fbt can't find in firmware's API and it can't know about nfc api in advance. We will think about this warning more.

I made some changes in your code:

  1. Allocate and free composite api resolver during allocation of NfcSupportedCards. This api resolver doesn't change during nfc application work and we can do it. This also fixes memory leak.
  2. Add api folder in nfc application and move all related files there. Also move gallagher_util there, since it's not used in nfc application. In my opinion introducing nfc_app_api is not necessary. It's just a wrapper of gallagher_util symbols and it's OK to add directly symbols from gallagher_util to nfc api interface.

Please, let me know if my changes work for you. If everything OK, we will merge your PR

@nickmooney
Copy link
Contributor Author

Thanks so much for the review. All the changes you've made seem reasonable and good to me! I'm happy with this PR in its current state.

@skotopes skotopes merged commit f6eb79e into flipperdevices:dev Feb 9, 2024
@hlask1
Copy link

hlask1 commented Feb 16, 2024

Hello friends, I am new to flipper devices , and I am wondering if it is appropriate to seek some advice here. Please advise if I should have posted this message somewhere else.

I installed the newest firmware 0.98.3, and tried to read a gallagher NFC card. The card can be recognised by flipper, but not as a gallagher card.

I am wondering if I need to install some library or dependencies manually before I can use firmware 0.98.3 to emulate gallagher card?

Thank you for your help

@skotopes
Copy link
Contributor

skotopes commented Feb 16, 2024

@doomwastaken @nickmooney @hedger I think I saw some abnormalities in this specific plugin linking

fbt: warning: build/f7-firmware-D/.extapps/gallagher_parser.fal: app may not be runnable. Symbols not resolved using firmware's API: {'gallagher_deobfuscate_and_parse_credential', 'GALLAGHER_CARDAX_ASCII'}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NFC NFC-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants