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

DESFire support for more readers and ISO7816 wrapped commands #314

Merged
merged 18 commits into from
Jan 28, 2022

Conversation

maxieds
Copy link
Contributor

@maxieds maxieds commented Jan 22, 2022

This pull request is a collaborative effort with much help testing from @lvandenb and @colinoflynn. It resolves issues #302 and #313. The equipment purchased with the suggestions in #310 was a great help in resolving the former two issues. All three issues can be closed when this pull request is merged. These source modifications to the DESFire emulation support have been tested and shown to work on the Omnikey 5022 and ACR 122 NFC readers.


Support for @maxieds to work on this project was funded by Georgia Tech via the university's COVID relief funding.

* "../LUFA/Common/Endianness.h" to avoid compilation issues without
* all of the overhead of pre-processing the LUFA setup on all sources:
*/
#if 0

Choose a reason for hiding this comment

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

Should this be removed from the PR? If might be used in the future makes sense but I see it's added but then #ifdef'd out.

@maxieds
Copy link
Contributor Author

maxieds commented Jan 23, 2022

@colinoflynn
Good catch. It's removed in the last appended commit.

@maxieds
Copy link
Contributor Author

maxieds commented Jan 23, 2022

I should point out a note about the modifications to the logging code for the moderators that will merge the PR. I had some reservations about the timing of transfers being off with the DESFire code when logging was done on the order of every 100ms (as before). I changed it to happen approximately every 600ms instead. The rationale for this change was that any extra time spent in the firmware writing to the logging buffer / FRAM / doing serial USB exchanges would be outside of the window of any single operation with the DESFire tags where the time delay could throw things off.

@maxieds
Copy link
Contributor Author

maxieds commented Jan 23, 2022

N.b., this PR makes the math whiz in me happy to consider the numbering as a forecast of good things to come with this project and DESFire support. Like the lucky numbers on a fortune cookie wrapper (in unicode): 𝛑 𝜋 𝝅 𝞹 π ℼ ϖ 兀 ...

@colinoflynn
Copy link

Thanks for all the fixes - I'll try to rebase my "mifare plus test code" on this after too so can look at if it makes sense pulling some of that over. I'm sure I'm not the only person with some use for limited mifare plus work.

@maxieds
Copy link
Contributor Author

maxieds commented Jan 23, 2022

@colinoflynn
Perhaps the work with Mifare Plus support you have been working on would be better to put in a separate PR? The code submitted with this PR extends some basic functionality that was missing in #287. That is, the Chameleon will actually now be functional with readers and PCDs "in the wild". Your extensions are somewhat different in nature. Is that ok for a plan?

@colinoflynn
Copy link

@maxieds Yes sorry totally! I just meant now that this "base" is there I might be encouraged to write my code in a less hacky way ;-) As before I wasn't sure if I was breaking the DESFire code etc... now with some of the major bugs fixed I'll feel better structuring my code nicely.

@fptrs
Copy link
Collaborator

fptrs commented Jan 27, 2022

Hi all,
thank you for your great work so far. I noticed that you did not run make style to format the code. When I do so some multi line comments get broken leading the compilation process to fail. Also it would be nice if you could quickly add DESFire log codes with the correct decoder to the python chamlog script here .

@maxieds
Copy link
Contributor Author

maxieds commented Jan 27, 2022

@fptrs
Just in time. I was headed to sleep soon. I modified the ChamTool logging codes, though some of the names/descs are longer than the others. Feel free to modify. The changes to the ISO7816 SW1/SW2 returns were a start at fixing (not done yet) what @lvandenb noted in this thread. A quick check with the two Omnikey and ACR readers I tested with shows that we still get recognition as a DESFire tag when running in CONFIG=MF_DESFIRE mode on the Chameleon. I didn't do detailed testing with the modifications to the ISO7816 status code changes.

@maxieds
Copy link
Contributor Author

maxieds commented Jan 27, 2022

@fptrs
I fixed the issue you mentioned in the last post where make style breaks the compilation process. The culprit was a single line // commented multi-line macro in one of the headers. The avr-gcc compiler treats this as non-existent code, but as you noticed it breaks guidelines on good form. :)

Any chance this will get merged sometime soon?

@fptrs fptrs merged commit 88ff788 into emsec:master Jan 28, 2022
@colinoflynn
Copy link

Thanks @maxieds et al, great to see the DESFire support there which is a nice base for many other features!

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.

None yet

3 participants