miguelbalboa / rfid

Arduino RFID Library for MFRC522
The Unlicense
2.75k stars 1.43k forks source link

Hard-coded authentication key in MIFARE_SetUid #465

Closed bengineer19 closed 4 years ago

bengineer19 commented 5 years ago

Hi,

First of all I just want to say thanks for creating an awesome open source project that I've found incredibly useful!

I noticed that in MFRC522.cpp, the value of the authentication key is hard-coded: https://github.com/miguelbalboa/rfid/blob/a395a6c7b7ddfa729fe9bb998f5bb16f41438844/src/MFRC522.cpp#L1789-L1792

This appears to be misleading, since the ChangeUID example appears to set the key manually, but it doesn't look like this key is actually used? https://github.com/miguelbalboa/rfid/blob/a395a6c7b7ddfa729fe9bb998f5bb16f41438844/examples/ChangeUID/ChangeUID.ino#L45-L50

If you think this is something that could be fixed, I'd be happy to help out :) Cheers!

Rotzbua commented 5 years ago

It looks like you are right, not completely sure.

lmmeng commented 5 years ago

Maybe it is the intended behavior? It seems to me the new UID is "applied" to read data and write back to the card: // Write new UID to the data we just read, and calculate BCC byte byte bcc = 0; for (uint8_t i = 0; i < uidSize; i++) { block0_buffer[i] = newUid[i]; bcc ^= newUid[i]; } // Write BCC byte to buffer block0_buffer[uidSize] = bcc; And later on: // Write modified block 0 back to card status = MIFARE_Write((byte)0, block0_buffer, (byte)16);

bengineer19 commented 5 years ago

I assume you're referring to line ~1839 as below. It looks to me as if that bit only gets executed if we don't fail the authentication above? https://github.com/miguelbalboa/rfid/blob/557b71efa7fc3a7ce99e8f35b8ea0edcb837eef1/src/MFRC522.cpp#L1826-L1844

lmmeng commented 5 years ago

I see now what you mean. Yes, looks more like an "one shot change" (maybe it works one time with factory (not programmed) tags).

os40la commented 5 years ago

The comments in that section make it appear like it is not a bug but intentional.

Line 1776 * It assumes a default KEY A of 0xFFFFFFFFFFFF.

Rotzbua commented 4 years ago

Custom key is usable on fork version.