RfidResearchGroup / proxmark3

Iceman Fork - Proxmark3
http://www.icedev.se
GNU General Public License v3.0
4k stars 1.05k forks source link

USB serial number is not unique #1904

Closed henrygab closed 1 year ago

henrygab commented 1 year ago

User Impact

The invalid USB descriptors force Windows to workaround the device error. This workaround results in ~5 seconds enumeration delay. That enumeration delay caused other, extremely difficult-to-triage failures.

As a concrete example, this bug prevented me from attaching two Proxmark3 devices to WSL2 via USBIPD. Root caused via the following thread: https://github.com/dorssel/usbipd-win/issues/452#issuecomment-1365481288.

Because it's so difficult to triage, I recommend fixes at the end

Describe the bug The firmware uses a hard-coded serial number. This causes problems when connecting more than one Proxmark3 device to a single computer. It should not expose a serial number in the USB descriptors, unless the serial number is unique for that VID/PID.

To Reproduce Steps to reproduce the behavior:

  1. Own two Proxmark3 devices (use RDV4 for simpler repro steps .. no Makefile.platform required)
  2. clone/sync/build the latest iceman fork's release
  3. Plug one in ... flash the firmware
  4. Unplug the first, plug the second in ... flash the firmware
  5. On Windows machine, plug in the first device (notice very fast discovery / exposure of device)
  6. On Windows machine, plug in the second device

Expected behavior Enumeration and exposure of COM port occurs quickly.

Actual behavior USB devices should NOT have identical VID + PID + ManufacturerString + ProductString + Serial Number. This breaks the implied uniqueness of having an exposed serial number.

Windows appears to have a workaround, where it totally prevents reading the serial number of 2nd (and later) devices that are plugged in ... but the workaround happens to add ~5 second delay to enumeration.

Other operating systems may not fail as gracefully, or exhibit unusual or unexpected side-effects. It's just a bad idea....

Desktop (please complete the following information):

Additional context

It is better to exclude the serial number, forcing the operating system to dynamically assign the device a unique ID, than to pretend that the device has (and exposes) its own unique ID.

As it is, Windows will dynamically assign the device ID to the 2nd (and later) devices. The first device attached will show its serial number. But, that depends on the order the devices appear to the OS, and thus is not reliable.

Suggested Fix

Choose one of the following:

  1. IFF a device-specific serial number exists, ARM code can dynamically (at PM3 boot) use that for the serial number used in the USB device descriptor.
  2. Remove the serial number from the USB device descriptor.
merlokk commented 1 year ago

image as i remembered it implemented

iceman1001 commented 1 year ago

I think after looking in the datasheet, that ID code register looks like this

image

henrygab commented 1 year ago
moot - but click to see old thoughts

I also did not find a unique ID available within the processor itself, nor withing the FPGA. There are a few options, each has positives and negatives: ### Option 1 On RDV4, use SIM module to generate a statistically-unique ID. This only resolves the serial number for RDV4, so another solution is needed for other models. However, it's a solid solution for RDV4 devices. ### Option 2 Reserve a predetermined location in the flash to store the device serial number. Exclude this location during flashing. At boot, if this flash location stores non-zero data, use that data for the serial number. Preferably, have a makefile option to randomize (or set to all-zero) this location in flash. ### Option 3 Combine options 1 & 2 ... if the device is RDV4, it uses the SIM module for uniqueness. Otherwise, it falls back to using the reserved area in the flash. Fewer steps for RDV4 owners. ### Option 4 Disable the serial number. Devices do not need to expose one, and not exposing one is more closely aligned with the purpose of this field. Normally, if a device did have a real serial number, this would downgrade some functionality, as Windows and other OS could track settings per-device. This may not matter for Proxmark3, and frankly, is actually **_broken_** today because of the false indication of a unique serial number. Settings initially start with one device, but then magically migrate to a different device, based only on the order in which they are plugged in. Not good. ### Thoughts? The above four options are just ones that I currently have thought of. I'd love to hear thoughts, suggestions, etc., because USB descriptors (and their effects) likely vary widely across platforms....

henrygab commented 1 year ago
moot - but click to see old thoughts

I know all four options are technically possible. I could very likely get something "good enough" for option 1, and likely could correctly provide a solution for Option 4. I suppose there's also **_option 5_** ... combination of unique serial from SIM for RDV4, and others having no serial number reported? However, while I know it's possible to exclude flash ranges from programming, and there exist ways to read raw flash, I do not have the confidence that I could make such changes reliably. Therefore, either option #2 or #3 would require additional help.

henrygab commented 1 year ago

As an aside, I think the "Chip ID Register" defines the type of chip for purposes of JTAG / Debugging, and is not a unique per-device identifier. The name sure sounded promising, however!

iceman1001 commented 1 year ago

Usually people only have one proxmark3 connected. Having multiple is a more rare problem.

since it would need to be a function that works for all possible proxmark3 hardware platforms, option 1 is out. When it comes to option 2, changing every time you flash the device might generate lots of new enumerations and if you hard code a value in the firmware file it the same problem as now. option 3 is just a mix not really solves anything. option 4 might be of interest but that would ruin my little Easter egg.

Its easy to solve for a rdv4 but not for all other platforms. What we would love to have is a unique id from either the fpga or the arm chip, and use that when enumerating the serial number.

henrygab commented 1 year ago
moot - but click to see old thoughts

You are correct as to #1, #3, #4, and the surprise. I do not see a unique ID available from the ARM chip, and I have no experience (yet) with the FPGA. Does the FPGA have a unique ID? If so, you are correct that would be a best option. For option #2, I did not explain the option well. Allow me to clarify some parts: 1. During build, the serial number is zero. 2. The location of that variable is hard-coded to a specific flash memory address 3. Because #2, during compilation, the address does not change between builds. 4. Because #2, the final binary has same all-zero data for that flash memory address 5. HOWEVER, flashing of the device is modified slightly to retain the existing data.... 6. This could be by excluding the address range where the serial is stored during flashing. 7. This could be by reserving a data partition at the end of the flash. 8. This could be by a distinct step that saves old data, allows flash, restores old data. Therefore, the serial number should **_not_** be constantly changing with each build, although a full flash erase might (if the SN is not saved beforehand) cause a change. I didn't see the FPGA datasheet in the depot. I'll have to take a look for that (another day... too late tonight!)

henrygab commented 1 year ago

I could not find a serial number accessible programmatically from the XILINX Spartan-II datasheet, and it doesn't appear to have an OTP area, either. Therefore, I don't think the FPGA will be a source of a unique number.

henrygab commented 1 year ago
moot - but click to see old thoughts

Ok. How about this low-effort solution: 1. Add a command under `hw` that adds user-specified serial number a. input is hex, between 4 and 32 hex characters b. unused characters are 0x00 2. Put into a 32-byte(*) struct a. Magic number at the start b. 32 hex characters == 16 bytes in the middle c. Hash of 16 bytes at end (or CRC, or other check) 3. Write that 32-byte struct to the end of the on-chip flash 4. During PM3 boot... a. Read the last 32-bytes of on-chip flash b. If magic number && hash/crc match... c. then use those 16-bytes for a longer serial number By appending the 16-bytes (converted to text, with 0x00 --> space), @iceman1001 can keep the nice easter egg. Sound reasonable? (*) Or one page of the flash (e.g. 1k, 2k, or 4k, if that's the erase block size). Or put it into space reserved for the bootloader ... especially as bootloader isn't written often ... etc.

henrygab commented 1 year ago

If we accept that the solution will only change behavior for Proxmark3 devices that have flash enabled, we can use the flash chip's unique ID.

It's a compile-time options, which simplifies the logic of creating the string descriptors. And the worst-case scenario is that the flash chip doesn't have a unique number, in which case it's no less unique than what's currently done.

Would you consider a PR which enables unique serial numbers only when compiled with WITH_FLASH defined?

Relevant pointers to code of interest

The USB Serial Number string descriptor: https://github.com/RfidResearchGroup/proxmark3/blob/bfd2fc2a568148696c4ea803bfa3301663e9b8ee/common_arm/usb_cdc.c#L369-L373 `Flash_UniqueID()` function will write a 128-bit value that the flash chip indicates is a unique identifier.

Flash_UniqueID(uint8_t *uid)

https://github.com/RfidResearchGroup/proxmark3/blob/bfd2fc2a568148696c4ea803bfa3301663e9b8ee/armsrc/flashmem.c#L254-L273

Strawman code, called at first boot, before enabling USB

```c #ifndef WITH_FLASH static const char StrSerialNumber[] = { 14, // Length 0x03, // Type is string 'i', 0, 'c', 0, 'e', 0, 'm', 0, 'a', 0, 'n', 0 }; #else // !defined(WITH_FLASH) static char StrSerialNumber[] = { 14, // Length is initially identical to non-unique version ... gets updated if unique serial is available 0x03, // Type is string 'i', 0, 'c', 0, 'e', 0, 'm', 0, 'a', 0, 'n', 0, '_', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, 'x', 0, }; void usb_update_serial_from_flash(void) { static bool configured = false; if (configured) { return; } configured = true; // run this only once per boot... uint8_t flash_uniqueID[8] = {0}; if (!FlashInit()) { return; } Flash_CheckBusy(BUSY_TIMEOUT); Flash_UniqueID(&(flash_uniqueID[0])); FlashStop(); // TODO: update StrSerialNumber with 16x hex characters StrSerialNumber[0] = 48; // enables the unique ID } #endif ```

iceman1001 commented 1 year ago

The flashchip has unique ID, we can start with adding this functionality for devices with flash memory..

The call to flash unique id fct will need to be preceeded with flash init call to set it up...

iceman1001 commented 1 year ago

If I remember this correct, there where a thing... there the serial number had to be of a certain length

Found it, https://github.com/RfidResearchGroup/proxmark3/commit/ff16cbb4dba558909fc72494b1e351ac73ea5626#commitcomment-30418488

It must not be a tuple of 8... which 48 is...

May I suggest we add an extra _ underscore?

henrygab commented 1 year ago

May I suggest we add an extra _ underscore?

Absolutely! I'll write up a PR. I am already successfully using this myself; chip reports as Winbond / 0x13.

henrygab commented 1 year ago

OK, the PR is ready for additional hardware testing. I was confident enough to load it onto my PM3 Easy devices, without having a JTAG adapter on hand for that 0.05" header. I do not have an RDV4 at the moment, and have no access to the iCopyX. So, if you have those around, and willing to test on those hardware, it'd be much appreciated.

I'm going to verify, but I think this may also bring stability for COM port assignment, which will be awesome.

iceman1001 commented 1 year ago

it will be a stepup for the people who uses two or more devices :)

iceman1001 commented 1 year ago

Alrighty then,

I have now tested with 4 devices connected to the same usb hub on Win11. Three RDV4 w unique serial and one generic device which has serial "iceman".

Findings,

All on all, I would say its a good addition.

Lets see if I can find my macbook and test

henrygab commented 1 year ago

Awesome! I had hoped/expected each of these things, and saw most of them during my own testing. I am especially happy with the reduced enumeration time (it caused other issues).