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

Automation of card cloning, and readerthreshold stored in EEPROM #118

Merged
merged 5 commits into from
Jan 24, 2018

Conversation

robots
Copy link
Contributor

@robots robots commented Jun 15, 2017

Some hardware requires the threshold to be altered. To do this the firmware has to be recompiled. I propose to store this setting among others in eeprom.

This pull request also adds CLONE command. This command changes current config to reader, identifies card and then changes config to the identified card type with same UID.

This functionality can also be assigned to button.
Example of usage: ledgreen = CODEC_RX, ledred setting_changed, lbutton = clone, rbutton = setting_cycle, timeout = 100 (10seconds). Press button, now you have 10seconds to approach target. Green led will identify activity from card. Done. Save card by rbutton press.

Signed-off-by: Michal Demin <michaldemin@gmail.com>
Signed-off-by: Michal Demin <michaldemin@gmail.com>
This command is extended original identify command. Upon execution it
will do following:
- change current config to "reader" mode
- identify card
- if multiple or no card detected -> end
- change config to identified card (mifare classic 1k/4k or ultralight)
- set UID of the card

Signed-off-by: Michal Demin <michaldemin@gmail.com>
Signed-off-by: Michal Demin <michaldemin@gmail.com>
@j-xander
Copy link
Contributor

Nice! I was hoping someone would get to this. Can't wait to try it out!

@robots
Copy link
Contributor Author

robots commented Jun 16, 2017

I have one more in the works that would clone complete content of MF ultralight card, it will be ready in few days

@geo-rg
Copy link
Collaborator

geo-rg commented Jun 23, 2017

Nice work!
However, this is the first time the Chameleon can execute a command (in terms of a terminal command) and I am not sure what happens when pressing a button, which is configured as CLONE, 100 times. Since every command execution prints something to the Terminal, I don't know if it is possible that something like a buffer overflow will occur.

I would suggest to use a new configuration for this functionality and the CLONE command simply changes to this configuration. If any output is necessary, it could be done via the log.

What do you think?

@geo-rg geo-rg self-assigned this Jun 23, 2017
@robots
Copy link
Contributor Author

robots commented Jun 23, 2017

I have traced the execution path of the string printing. There are 2 cases to consider:

  1. usb not plugged:
    All terminal output is routed to CDC_Device_SendByte function which will do nothing when device is not connected. No problem
  2. usb plugged:
    Again we are at CDC_Device_SendByte which calls: Endpoint_IsReadAllowed that checks for space in endpoint buffer, if no space is available it waits for endpoint data to be send or timeout. So no problem either other than it will block.

https://github.com/emsec/ChameleonMini/blob/master/Firmware/LUFA/Drivers/USB/Class/Device/CDCClassDevice.c#L190

Other than this, firmware does no buffering of strings at all.
I have made sure that the CommandExecute function is save to execute from within the firmware.

Also pressing the button 100 times would restart the Reader14443A application 100times. It's not creating 100 threads in the system. ( there are no threads)

@geo-rg
Copy link
Collaborator

geo-rg commented Jun 28, 2017

As I already said, nice work! I didn't have the time yet to find out how the USB connection parts work in detail and so I just was suspicious.

Just give us some time to test the whole thing and then we'll merge it, ok? :)

@robots
Copy link
Contributor Author

robots commented Jun 28, 2017

I thought you were developer of the firmware. :-)

Anyway, i am not sure about the threshold setting being adjusted for each "setting". If you could commend on that part it would be nice.

@geo-rg
Copy link
Collaborator

geo-rg commented Jun 29, 2017

Well, I am. But when I took over the project, the USB communication already was realized and I did not have to care about it ;)

About the threshold: I think that this is quite a good idea, I could imagine for example that you can have 3 settings, each configured as reader but each one with another threshold to communicate with another type of card. Right now, we are implementing an autocalibration command, with tries to communicate with every possible threshold (with a specific step size) and then finds the best threshold to communicate. Just give us a bit of time ;)

@geo-rg
Copy link
Collaborator

geo-rg commented Jun 30, 2017

Is it intended that the CLONE command is invisible? You have put it after the last element in CommandTable in CommandLine.c which makes it invisible to the terminal.

I would suggest that the command is not invisible and I also think that it would be great to have a visible response (i.e. a LED setting which blinks if successful or so...).

@robots
Copy link
Contributor Author

robots commented Jul 3, 2017

No its not intentional. I messed up my git repo, and i had to merge few things manually. I guess it just got lost :-)
Yes, led blinks are good addition. I just used existing CODEC_RX trigger. It would not tell me that the card is supported, just that it communicated ok.

@raskai0
Copy link

raskai0 commented Jul 24, 2017

Hi,

@robots any news on this feature ?
You try this ?

I am interested to clone on the fly.

Thanks

Signed-off-by: Michal Demin <michaldemin@gmail.com>
@ghost
Copy link

ghost commented Jan 23, 2018

@robots any news about it?

@robots
Copy link
Contributor Author

robots commented Jan 23, 2018

what news are you expecting ? :-)

@ghost
Copy link

ghost commented Jan 23, 2018

Have you heard anything about they'll merge it?
I'll checkout your repo and go on testing the feature.

@geo-rg
Copy link
Collaborator

geo-rg commented Jan 24, 2018

Hi @gtpy @robots
I'll merge this as soon as I have tested it myself (maybe I'll also change few minor things), which I hope will be this week.

@ghost
Copy link

ghost commented Jan 24, 2018

@geo-rg
That would be nice, so I don't need to work more on my fork for this.
Because I don't know your few minor changes :-)
So I can work on other new features.

@geo-rg geo-rg merged commit 62ba2e0 into emsec:master Jan 24, 2018
@geo-rg
Copy link
Collaborator

geo-rg commented Jan 24, 2018

Hi @gtpy @robots @j-xander
sorry for the long time it took to merge it. However, there are a few points I want to stress:

  1. It would be great if there was the possibility to receive a visual feedback via the LED if the process is finished successfully.
  2. I merged the code, but did not yet update the "Latest" precompiled version, since I have not fully tested this updated code. It would be very nice if you could check out the latest pushed state, compile it, and test whether all these things (emulation, log, download, upload) still work correctly.
  3. What comes into my mind right now: I'm not sure whether after switching the config at the end of the clone process, this is also stored to the eeprom settings (so this config change is permanent) and whether the cloned contents (UID or UID and card content) are stored to the Flash so these data are held permanently. Otherwise, after a reboot, they would be gone.

@ghost
Copy link

ghost commented Jan 25, 2018

@geo-rg I'll compile and test the functionality and give feedback.

@geo-rg
Copy link
Collaborator

geo-rg commented Jan 25, 2018

@gtpy @robots @j-xander
I did some testing and also added the cloning being permanent. There are a few things that remain open, which I will address in an issue, but I really cannot spend more time on this thing ;)

We really appreciate your contributions, thanks! :)

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

4 participants