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

Allow non-root access on Android devices #565

Merged
merged 6 commits into from Feb 17, 2018

Conversation

Projects
None yet
6 participants
@Proxcloud

Proxcloud commented Feb 6, 2018

Add the BOS USB descriptor. This tells the OS the device supports extended commands, on Android this means the OS allows apps to interact with the device using the Android USB Host API. i.e. apps can communicate with the PM3 without needing root.

@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 6, 2018

Member

hm, I think I played with this last august, turned out to be bad with osx, linux, win10, win8.1, mingw ....
I couldn't get all os:s to play nice with it. Have you verifed device is enumerable and functional on those OS?

Member

iceman1001 commented Feb 6, 2018

hm, I think I played with this last august, turned out to be bad with osx, linux, win10, win8.1, mingw ....
I couldn't get all os:s to play nice with it. Have you verifed device is enumerable and functional on those OS?

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 7, 2018

It works on Android, Linux, and OSX, but I have not tested in Windows. I'll do some testing and report back.

Proxcloud commented Feb 7, 2018

It works on Android, Linux, and OSX, but I have not tested in Windows. I'll do some testing and report back.

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 7, 2018

Tested working (device is enumerated and client can connect & run commands):
Ubuntu 14.04 (native) - still need the device blacklist rules
Android 6.0.1 (native)
Windows 8.1 (virtual machine) - Proxgator environment. (side note: I had to manually install the driver INF file to get both clean master and BOS firmware to work. I also had to disable driver signature checking to force it to load)
Windows 10.0.16299 (virtual machine) - Proxgator environment.
(as an interesting side note, the clean master firmware fails to correctly load the driver on this Windows system. It is possibly because the system doesn't have Internet access, so it can't download drivers. However, with this added BOS descriptor it successfully loads the driver)

Unknown:
Windows 7 I have not been able to test it on Windows 7 because the Proxgator fails to build/run. I'm not sure if Win7 is supported?

OSX I need to test again on Mac as the version I tested before used a slightly different descriptor. I'll post again when I can find a Mac to test it with

You also said it needed to be tested on mingw. Proxgator environment is mingw, with which it works on the two versions of Windows I tested, but is there another type mingw which needs testing?

Proxcloud commented Feb 7, 2018

Tested working (device is enumerated and client can connect & run commands):
Ubuntu 14.04 (native) - still need the device blacklist rules
Android 6.0.1 (native)
Windows 8.1 (virtual machine) - Proxgator environment. (side note: I had to manually install the driver INF file to get both clean master and BOS firmware to work. I also had to disable driver signature checking to force it to load)
Windows 10.0.16299 (virtual machine) - Proxgator environment.
(as an interesting side note, the clean master firmware fails to correctly load the driver on this Windows system. It is possibly because the system doesn't have Internet access, so it can't download drivers. However, with this added BOS descriptor it successfully loads the driver)

Unknown:
Windows 7 I have not been able to test it on Windows 7 because the Proxgator fails to build/run. I'm not sure if Win7 is supported?

OSX I need to test again on Mac as the version I tested before used a slightly different descriptor. I'll post again when I can find a Mac to test it with

You also said it needed to be tested on mingw. Proxgator environment is mingw, with which it works on the two versions of Windows I tested, but is there another type mingw which needs testing?

@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 7, 2018

Member

Current official mingw is Gator9600's , which needs to be supported.

Win10 doesn't need to install driver, it will correctly identify the pm3 given vid/pid but on older OS versions its not the same story. But I got ppl reporting in from WinXp and upwards. The October brickin' incident is what I call what was the effects when iceman fork changed the usb-enumeration...

Official PM3 don't have to experience the same, the testing needs to be thorough before merge.

Member

iceman1001 commented Feb 7, 2018

Current official mingw is Gator9600's , which needs to be supported.

Win10 doesn't need to install driver, it will correctly identify the pm3 given vid/pid but on older OS versions its not the same story. But I got ppl reporting in from WinXp and upwards. The October brickin' incident is what I call what was the effects when iceman fork changed the usb-enumeration...

Official PM3 don't have to experience the same, the testing needs to be thorough before merge.

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 7, 2018

Cool, just so I have this straight, it needs to be tested and work on the following:

  • Gator9600's mingw environment running on:
    • WinXP
    • Win7
    • Win8.1
    • Win10
  • Linux
  • OSX
  • Android

Correct?
If I'm missing any systems, please let me know so I can test on those too.
So far I have tested it works on: Win8.1, Win10, Linux, Android. I'll try to test the remaining systems tomorrow.

Proxcloud commented Feb 7, 2018

Cool, just so I have this straight, it needs to be tested and work on the following:

  • Gator9600's mingw environment running on:
    • WinXP
    • Win7
    • Win8.1
    • Win10
  • Linux
  • OSX
  • Android

Correct?
If I'm missing any systems, please let me know so I can test on those too.
So far I have tested it works on: Win8.1, Win10, Linux, Android. I'll try to test the remaining systems tomorrow.

@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 7, 2018

Member

Correct, it seems the following linux distros are used

  • ubuntu
  • kali
  • archlinux
Member

iceman1001 commented Feb 7, 2018

Correct, it seems the following linux distros are used

  • ubuntu
  • kali
  • archlinux

@Proxcloud Proxcloud changed the title from Add BOS USB descriptor. This allows non-root access on Android devices to Allow non-root access on Android devices Feb 8, 2018

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 8, 2018

I was thinking about this issue last night, and had this nagging feeling something didn't make sense. Because the firmware used to work on Android, and then it didn't. I only recently noticed because I've been working on code from nearly a year ago, and my code works fine on Android. I thought it was because of the BOS descriptor I'd added. But that didn't explain why it used to work. So I went looking for what caused it to stop working, and found it was the change @pwpiwi made last April to the Manufacturer descriptor. Commit that breaks non-root Android access

So I have removed the BOS descriptor as it's not required and increase the chances of something breaking. Now the pull request is just two lines, and is just a revert of @pwpiwi's changes.

Proxcloud commented Feb 8, 2018

I was thinking about this issue last night, and had this nagging feeling something didn't make sense. Because the firmware used to work on Android, and then it didn't. I only recently noticed because I've been working on code from nearly a year ago, and my code works fine on Android. I thought it was because of the BOS descriptor I'd added. But that didn't explain why it used to work. So I went looking for what caused it to stop working, and found it was the change @pwpiwi made last April to the Manufacturer descriptor. Commit that breaks non-root Android access

So I have removed the BOS descriptor as it's not required and increase the chances of something breaking. Now the pull request is just two lines, and is just a revert of @pwpiwi's changes.

@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 8, 2018

Member

the problematic vid/pid... both sets of vid/pid is connected with PM3.
the old one, which is associated with HID
the new one, which is asscociated with CDC
then the swap last april, from new set to old set, where we now don't have a way of correctly identifying HID/CDC anymore. This is the major reason for all issues/threads with subjects similar to "I read the wiki, and I got a HID firmware on.. my device doesn't work..."

Its been almost a year, the wiki / guides isn't reflecting this change so yeah, confusing.

on linux distros, the identifiction of proxmark in the description, should allow for the modemmanager to relax. I have no idea why android doesn't like both sets of vid/pid... An answer shoud lay in their codebase :)

Member

iceman1001 commented Feb 8, 2018

the problematic vid/pid... both sets of vid/pid is connected with PM3.
the old one, which is associated with HID
the new one, which is asscociated with CDC
then the swap last april, from new set to old set, where we now don't have a way of correctly identifying HID/CDC anymore. This is the major reason for all issues/threads with subjects similar to "I read the wiki, and I got a HID firmware on.. my device doesn't work..."

Its been almost a year, the wiki / guides isn't reflecting this change so yeah, confusing.

on linux distros, the identifiction of proxmark in the description, should allow for the modemmanager to relax. I have no idea why android doesn't like both sets of vid/pid... An answer shoud lay in their codebase :)

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 8, 2018

It's got nothing to with the changed vid/pid, it's just the following 2 lines about returning the Manufacturer string description.

else if ((wValue & 0x300) == 0x300)  // Return Manufacturer Description - this is needed by Android
	AT91F_USB_SendData(pUdp, StrDescManufacturer, MIN(sizeof(StrDescManufacturer), wLength));

Proxcloud commented Feb 8, 2018

It's got nothing to with the changed vid/pid, it's just the following 2 lines about returning the Manufacturer string description.

else if ((wValue & 0x300) == 0x300)  // Return Manufacturer Description - this is needed by Android
	AT91F_USB_SendData(pUdp, StrDescManufacturer, MIN(sizeof(StrDescManufacturer), wLength));
@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 8, 2018

Member

Strange, the line below that should take card of the manufacturer string, ie call to getStringDescriptor(wValue & 0xff);

Member

iceman1001 commented Feb 8, 2018

Strange, the line below that should take card of the manufacturer string, ie call to getStringDescriptor(wValue & 0xff);

Show outdated Hide outdated common/usb_cdc.c
@pwpiwi

This comment has been minimized.

Show comment
Hide comment
@pwpiwi

pwpiwi Feb 8, 2018

Contributor

Sorry, same time review. But same result as well.

Contributor

pwpiwi commented Feb 8, 2018

Sorry, same time review. But same result as well.

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 8, 2018

hahaha, finally got it. it's not the manufacturer string at all. It's a wrong length for the new product description. @pwpiwi set it to 8, but len+"pm3" is only 4 bytes. I guess other OSs don't care, but it must trigger a bug in Android and causes it to fail somehow.
Now the pull request only changes a single char. The length of the product description string.

Proxcloud commented Feb 8, 2018

hahaha, finally got it. it's not the manufacturer string at all. It's a wrong length for the new product description. @pwpiwi set it to 8, but len+"pm3" is only 4 bytes. I guess other OSs don't care, but it must trigger a bug in Android and causes it to fail somehow.
Now the pull request only changes a single char. The length of the product description string.

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 8, 2018

hmm, that's not quite right either. Setting the length to 4 doesn't account for the null bytes, making product desc only show "P". Every other length works on Android except for "8". Bazaar, but it is what it is. I have now increased the length to 9 and added an extra null byte to account for it.

Proxcloud commented Feb 8, 2018

hmm, that's not quite right either. Setting the length to 4 doesn't account for the null bytes, making product desc only show "P". Every other length works on Android except for "8". Bazaar, but it is what it is. I have now increased the length to 9 and added an extra null byte to account for it.

Show outdated Hide outdated common/usb_cdc.c
@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 8, 2018

I have changed the product description string to "proxmark3" rather than "PM3". This makes the length even, the string not be null terminated and the length no longer be 8.

Proxcloud commented Feb 8, 2018

I have changed the product description string to "proxmark3" rather than "PM3". This makes the length even, the string not be null terminated and the length no longer be 8.

@pwpiwi

This comment has been minimized.

Show comment
Hide comment
@pwpiwi

pwpiwi Feb 8, 2018

Contributor

"proxmark3" works but "PM3" doesn't? Is Android that broken?

Contributor

pwpiwi commented Feb 8, 2018

"proxmark3" works but "PM3" doesn't? Is Android that broken?

@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 8, 2018

Member

this sounds like pure guesswork. I suggest closing this PR until there is something to actaully consider merging.

Member

iceman1001 commented Feb 8, 2018

this sounds like pure guesswork. I suggest closing this PR until there is something to actaully consider merging.

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 8, 2018

It is surprising, but yes, "proxmark3" works and "PM3" doesn't. I have tested this on 3 phones, a Samsung C5 Pro (Android 6.0.1), a Blackberry Priv (Android 6.0.1) and a Huawei Honor 6X (Android 7.0)

Proxcloud commented Feb 8, 2018

It is surprising, but yes, "proxmark3" works and "PM3" doesn't. I have tested this on 3 phones, a Samsung C5 Pro (Android 6.0.1), a Blackberry Priv (Android 6.0.1) and a Huawei Honor 6X (Android 7.0)

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 8, 2018

@iceman1001 if this is not evidence enough, please let me know what is, testing a wider range of phones? Getting someone else to reproduce my results? Tracking down the bug in the Android source code?

Proxcloud commented Feb 8, 2018

@iceman1001 if this is not evidence enough, please let me know what is, testing a wider range of phones? Getting someone else to reproduce my results? Tracking down the bug in the Android source code?

@marshmellow42

This comment has been minimized.

Show comment
Hide comment
@marshmellow42

marshmellow42 Feb 12, 2018

Contributor

will this change make systems re-install the pm3 on another com port again? (like to vid/pid changes did) i don't look forward to another change like that for ppl on the forum... (sorry haven't had time to test it myself yet...)

Contributor

marshmellow42 commented Feb 12, 2018

will this change make systems re-install the pm3 on another com port again? (like to vid/pid changes did) i don't look forward to another change like that for ppl on the forum... (sorry haven't had time to test it myself yet...)

@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 12, 2018

Member

@marshmellow42 indeed, I'm also not inclined to have another of those episodes.

Member

iceman1001 commented Feb 12, 2018

@marshmellow42 indeed, I'm also not inclined to have another of those episodes.

@Proxcloud

This comment has been minimized.

Show comment
Hide comment
@Proxcloud

Proxcloud Feb 12, 2018

@marshmellow42 I just checked on win8.1 and win10. It always shows up as com3 for me, with and without this change. But having more people confirm it works would be great, please update us if you have a chance to test it.

@iceman1001 Still waiting on what you consider sufficient verification of the bug/fix to be. I can arrange to have whatever testing you think is necessary done. I agrees we don't want to introduce problems for users, so let's set out what exactly what needs to be done to verify the fix.

Proxcloud commented Feb 12, 2018

@marshmellow42 I just checked on win8.1 and win10. It always shows up as com3 for me, with and without this change. But having more people confirm it works would be great, please update us if you have a chance to test it.

@iceman1001 Still waiting on what you consider sufficient verification of the bug/fix to be. I can arrange to have whatever testing you think is necessary done. I agrees we don't want to introduce problems for users, so let's set out what exactly what needs to be done to verify the fix.

@marshmellow42

This comment has been minimized.

Show comment
Hide comment
@marshmellow42

marshmellow42 Feb 12, 2018

Contributor

Since it is just a human readable description it probably shouldn't affect the port or device identification on the PC... But run a few tests I will. :)

Contributor

marshmellow42 commented Feb 12, 2018

Since it is just a human readable description it probably shouldn't affect the port or device identification on the PC... But run a few tests I will. :)

@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 12, 2018

Member

I see no need to change a human readable descriptor unless someone proves the actual usb-enumeration on Android takes it in consideration. The source code should be available somewhere by Google, I guess.
If it turns out that it does influence it, consistency testing on different OSX, Win, Linux will be needed as mentioned before.

Member

iceman1001 commented Feb 12, 2018

I see no need to change a human readable descriptor unless someone proves the actual usb-enumeration on Android takes it in consideration. The source code should be available somewhere by Google, I guess.
If it turns out that it does influence it, consistency testing on different OSX, Win, Linux will be needed as mentioned before.

@marshmellow42

This comment has been minimized.

Show comment
Hide comment
@marshmellow42

marshmellow42 Feb 14, 2018

Contributor

i confirmed no port changes on windows 10 - 1709-16299.248
it appears @Proxcloud tested ubuntu 14
i see no harm in this PR. it has turned out to be a very minor change, if it helps someone why not accept?
(but i know very little of serial port enumeration.)

Contributor

marshmellow42 commented Feb 14, 2018

i confirmed no port changes on windows 10 - 1709-16299.248
it appears @Proxcloud tested ubuntu 14
i see no harm in this PR. it has turned out to be a very minor change, if it helps someone why not accept?
(but i know very little of serial port enumeration.)

@micolous

This comment has been minimized.

Show comment
Hide comment
@micolous

micolous Feb 16, 2018

Contributor

Per the comments in the file you were editing, you should not change the manufacturer string, ever. This is because ModemManager (Linux) scans for PM3 based on the manuafacturer string. This is because the PM3 device ID at the time was not registered, and this rule matches both "old" and "new" device IDs.

Getting changes into ModemManager takes around 2-3 years to actually land in users' Linux distributions. This allows them to use the hardware without adding extra blacklisting rules or uninstalling ModemManager.

The product string is not matched in the ModemManager rules, so it is "safe" to change this.

But as is, the changes you have proposed should have no impact on "native" Android usage, and I've tested this on a Nexus 5X, Pixel 1, and ODroid U3. I'm able to successfully communicate with it in AndProx, without root, and without flashing different firmware.

Side note: please consider working with me on AndProx rather than making "yet another Android port", we currently have three. I already have native, non-root communication with the PM3 on Android with JNI wrappers for the communications libraries.

I am interested in getting WebUSB support going at some point with the stock firmware, I'm just having troubles with the BOS on Windows 10 so that it doesn't impact "native" usage.

Contributor

micolous commented Feb 16, 2018

Per the comments in the file you were editing, you should not change the manufacturer string, ever. This is because ModemManager (Linux) scans for PM3 based on the manuafacturer string. This is because the PM3 device ID at the time was not registered, and this rule matches both "old" and "new" device IDs.

Getting changes into ModemManager takes around 2-3 years to actually land in users' Linux distributions. This allows them to use the hardware without adding extra blacklisting rules or uninstalling ModemManager.

The product string is not matched in the ModemManager rules, so it is "safe" to change this.

But as is, the changes you have proposed should have no impact on "native" Android usage, and I've tested this on a Nexus 5X, Pixel 1, and ODroid U3. I'm able to successfully communicate with it in AndProx, without root, and without flashing different firmware.

Side note: please consider working with me on AndProx rather than making "yet another Android port", we currently have three. I already have native, non-root communication with the PM3 on Android with JNI wrappers for the communications libraries.

I am interested in getting WebUSB support going at some point with the stock firmware, I'm just having troubles with the BOS on Windows 10 so that it doesn't impact "native" usage.

@micolous

This comment has been minimized.

Show comment
Hide comment
@micolous

micolous Feb 16, 2018

Contributor

And FYI, you can find my work on getting WebUSB-compatible descriptors here: https://github.com/micolous/proxmark3/blob/webusb/common/usb_cdc.c

This currently works on Linux and OSX, but doesn't work on Windows 10. I'm waiting for a USB device sniffer to be able to troubleshoot this further, as I get differing results on virtual and physical machines.

Contributor

micolous commented Feb 16, 2018

And FYI, you can find my work on getting WebUSB-compatible descriptors here: https://github.com/micolous/proxmark3/blob/webusb/common/usb_cdc.c

This currently works on Linux and OSX, but doesn't work on Windows 10. I'm waiting for a USB device sniffer to be able to troubleshoot this further, as I get differing results on virtual and physical machines.

@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 17, 2018

Member

Ok, so this PR doesn't bring anything more than changing a description string. Much ado for nothing then.
I do agree with @micolous (for awhile I thought you and @Proxcloud were the same) that we don't need more Android ports. Still waiting to see @Proxcloud firmware repo. Until then I will not support it.

Member

iceman1001 commented Feb 17, 2018

Ok, so this PR doesn't bring anything more than changing a description string. Much ado for nothing then.
I do agree with @micolous (for awhile I thought you and @Proxcloud were the same) that we don't need more Android ports. Still waiting to see @Proxcloud firmware repo. Until then I will not support it.

@micolous

This comment has been minimized.

Show comment
Hide comment
@micolous

micolous Feb 17, 2018

Contributor

Nope, different people. I also release my code as required by the license. 😎

I did find something interesting about this change. Here are the old descriptors on Android:

02-17 20:08:39.254 882-1382/? D/UsbHostManager: Added device UsbDevice[mName=/dev/bus/usb/001/005,mVendorId=39620,mProductId=19343,mClass=2,mSubclass=0,mProtocol=0,mManufacturerName=proxmark.org,mProductName=null,mVersion=2.0,mSerialNumber=null,mConfigurations=[
                                                UsbConfiguration[mId=1,mName=null,mAttributes=128,mMaxPower=75,mInterfaces=[
                                                UsbInterface[mId=0,mAlternateSetting=0,mName=null,mClass=2,mSubclass=2,mProtocol=1,mEndpoints=[
                                                UsbEndpoint[mAddress=131,mAttributes=3,mMaxPacketSize=8,mInterval=255]]
                                                UsbInterface[mId=1,mAlternateSetting=0,mName=null,mClass=10,mSubclass=0,mProtocol=0,mEndpoints=[
                                                UsbEndpoint[mAddress=1,mAttributes=2,mMaxPacketSize=64,mInterval=0]
                                                UsbEndpoint[mAddress=130,mAttributes=2,mMaxPacketSize=64,mInterval=0]]]]

And the proposed new descriptors on Android:

02-17 20:17:54.299 882-1382/? D/UsbHostManager: Added device UsbDevice[mName=/dev/bus/usb/001/009,mVendorId=39620,mProductId=19343,mClass=2,mSubclass=0,mProtocol=0,mManufacturerName=proxmark.org,mProductName=proxmark3,mVersion=2.0,mSerialNumber=null,mConfigurations=[
                                                UsbConfiguration[mId=1,mName=null,mAttributes=128,mMaxPower=75,mInterfaces=[
                                                UsbInterface[mId=0,mAlternateSetting=0,mName=null,mClass=2,mSubclass=2,mProtocol=1,mEndpoints=[
                                                UsbEndpoint[mAddress=131,mAttributes=3,mMaxPacketSize=8,mInterval=255]]
                                                UsbInterface[mId=1,mAlternateSetting=0,mName=null,mClass=10,mSubclass=0,mProtocol=0,mEndpoints=[
                                                UsbEndpoint[mAddress=1,mAttributes=2,mMaxPacketSize=64,mInterval=0]
                                                UsbEndpoint[mAddress=130,mAttributes=2,mMaxPacketSize=64,mInterval=0]]]]

If I look on a Linux box, here is before:

[40259.517558] usb 3-2.2.2: New USB device found, idVendor=9ac4, idProduct=4b8f
[40259.517562] usb 3-2.2.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[40259.517565] usb 3-2.2.2: Product: PM3
[40259.517567] usb 3-2.2.2: Manufacturer: proxmark.org
[40259.518338] cdc_acm 3-2.2.2:1.0: ttyACM0: USB ACM device

$ sudo lsusb -d 9ac4:4b8f -v

Bus 003 Device 047: ID 9ac4:4b8f J. Westhues ProxMark-3 RFID Instrument
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            2 Communications
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x9ac4 J. Westhues
  idProduct          0x4b8f ProxMark-3 RFID Instrument
  bcdDevice            0.01
  iManufacturer           1 proxmark.org
  iProduct                2 (error)
  iSerial                 0 
  bNumConfigurations      1
[snip]

And after:

[40277.980951] usb 3-2.2.2: New USB device found, idVendor=9ac4, idProduct=4b8f
[40277.980955] usb 3-2.2.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[40277.980958] usb 3-2.2.2: Product: proxmark3
[40277.980960] usb 3-2.2.2: Manufacturer: proxmark.org
[40277.981751] cdc_acm 3-2.2.2:1.0: ttyACM0: USB ACM device

$ sudo lsusb -d 9ac4:4b8f -v

Bus 003 Device 044: ID 9ac4:4b8f J. Westhues ProxMark-3 RFID Instrument
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            2 Communications
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x9ac4 J. Westhues
  idProduct          0x4b8f ProxMark-3 RFID Instrument
  bcdDevice            0.01
  iManufacturer           1 proxmark.org
  iProduct                2 proxmark3 
  iSerial                 0 
  bNumConfigurations      1

Interestingly, despite the Product Name descriptor being there before and after, both Android and lsusb had trouble reading the old value.

But the kernel log still shows that it was able to read the Product Name descriptor.

If I look in Wireshark while running lsusb, the old descriptor appears to have the PM3 stall for about a second before responding. With the new descriptor PM3 responds in just 1ms.

At this point I'm going to eat humble pie, because after poking at these descriptors and seeing differences on Android and non-Android with this change, I gave it a go on my Sony Android TV. Now, my Android port can now detect the device and communicate with it:

device-2018-02-17-205054

It's pretty impressive to me that four separate Android vendors (Blackberry, Huawei, Samsung and Sony) have managed to mess this up. I wonder how one goes about adding tests to the Android Compatibility Test Suite... 🤔

Contributor

micolous commented Feb 17, 2018

Nope, different people. I also release my code as required by the license. 😎

I did find something interesting about this change. Here are the old descriptors on Android:

02-17 20:08:39.254 882-1382/? D/UsbHostManager: Added device UsbDevice[mName=/dev/bus/usb/001/005,mVendorId=39620,mProductId=19343,mClass=2,mSubclass=0,mProtocol=0,mManufacturerName=proxmark.org,mProductName=null,mVersion=2.0,mSerialNumber=null,mConfigurations=[
                                                UsbConfiguration[mId=1,mName=null,mAttributes=128,mMaxPower=75,mInterfaces=[
                                                UsbInterface[mId=0,mAlternateSetting=0,mName=null,mClass=2,mSubclass=2,mProtocol=1,mEndpoints=[
                                                UsbEndpoint[mAddress=131,mAttributes=3,mMaxPacketSize=8,mInterval=255]]
                                                UsbInterface[mId=1,mAlternateSetting=0,mName=null,mClass=10,mSubclass=0,mProtocol=0,mEndpoints=[
                                                UsbEndpoint[mAddress=1,mAttributes=2,mMaxPacketSize=64,mInterval=0]
                                                UsbEndpoint[mAddress=130,mAttributes=2,mMaxPacketSize=64,mInterval=0]]]]

And the proposed new descriptors on Android:

02-17 20:17:54.299 882-1382/? D/UsbHostManager: Added device UsbDevice[mName=/dev/bus/usb/001/009,mVendorId=39620,mProductId=19343,mClass=2,mSubclass=0,mProtocol=0,mManufacturerName=proxmark.org,mProductName=proxmark3,mVersion=2.0,mSerialNumber=null,mConfigurations=[
                                                UsbConfiguration[mId=1,mName=null,mAttributes=128,mMaxPower=75,mInterfaces=[
                                                UsbInterface[mId=0,mAlternateSetting=0,mName=null,mClass=2,mSubclass=2,mProtocol=1,mEndpoints=[
                                                UsbEndpoint[mAddress=131,mAttributes=3,mMaxPacketSize=8,mInterval=255]]
                                                UsbInterface[mId=1,mAlternateSetting=0,mName=null,mClass=10,mSubclass=0,mProtocol=0,mEndpoints=[
                                                UsbEndpoint[mAddress=1,mAttributes=2,mMaxPacketSize=64,mInterval=0]
                                                UsbEndpoint[mAddress=130,mAttributes=2,mMaxPacketSize=64,mInterval=0]]]]

If I look on a Linux box, here is before:

[40259.517558] usb 3-2.2.2: New USB device found, idVendor=9ac4, idProduct=4b8f
[40259.517562] usb 3-2.2.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[40259.517565] usb 3-2.2.2: Product: PM3
[40259.517567] usb 3-2.2.2: Manufacturer: proxmark.org
[40259.518338] cdc_acm 3-2.2.2:1.0: ttyACM0: USB ACM device

$ sudo lsusb -d 9ac4:4b8f -v

Bus 003 Device 047: ID 9ac4:4b8f J. Westhues ProxMark-3 RFID Instrument
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            2 Communications
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x9ac4 J. Westhues
  idProduct          0x4b8f ProxMark-3 RFID Instrument
  bcdDevice            0.01
  iManufacturer           1 proxmark.org
  iProduct                2 (error)
  iSerial                 0 
  bNumConfigurations      1
[snip]

And after:

[40277.980951] usb 3-2.2.2: New USB device found, idVendor=9ac4, idProduct=4b8f
[40277.980955] usb 3-2.2.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[40277.980958] usb 3-2.2.2: Product: proxmark3
[40277.980960] usb 3-2.2.2: Manufacturer: proxmark.org
[40277.981751] cdc_acm 3-2.2.2:1.0: ttyACM0: USB ACM device

$ sudo lsusb -d 9ac4:4b8f -v

Bus 003 Device 044: ID 9ac4:4b8f J. Westhues ProxMark-3 RFID Instrument
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            2 Communications
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x9ac4 J. Westhues
  idProduct          0x4b8f ProxMark-3 RFID Instrument
  bcdDevice            0.01
  iManufacturer           1 proxmark.org
  iProduct                2 proxmark3 
  iSerial                 0 
  bNumConfigurations      1

Interestingly, despite the Product Name descriptor being there before and after, both Android and lsusb had trouble reading the old value.

But the kernel log still shows that it was able to read the Product Name descriptor.

If I look in Wireshark while running lsusb, the old descriptor appears to have the PM3 stall for about a second before responding. With the new descriptor PM3 responds in just 1ms.

At this point I'm going to eat humble pie, because after poking at these descriptors and seeing differences on Android and non-Android with this change, I gave it a go on my Sony Android TV. Now, my Android port can now detect the device and communicate with it:

device-2018-02-17-205054

It's pretty impressive to me that four separate Android vendors (Blackberry, Huawei, Samsung and Sony) have managed to mess this up. I wonder how one goes about adding tests to the Android Compatibility Test Suite... 🤔

@micolous

micolous approved these changes Feb 17, 2018 edited

Thumbs up from me with only Product Name change, per #565 (comment)

Changing the manufacturer name will cause much sadness with ModemManager (and another 3 years before we can fix it properly).

I tested with: Linux x86_64, Android 8.1, Android 6, OSX 10.13

micolous added a commit to AndProx/AndProx that referenced this pull request Feb 17, 2018

Many docs fixes, and weakref fix:
- Document Y-cable workaround for HF commands (#4)

- Document Product Name bug (Proxmark/proxmark3#565)

- More javadocs

@iceman1001 iceman1001 merged commit b8196bf into Proxmark:master Feb 17, 2018

@iceman1001

This comment has been minimized.

Show comment
Hide comment
@iceman1001

iceman1001 Feb 17, 2018

Member

there, I can also eat humble pie.

Member

iceman1001 commented Feb 17, 2018

there, I can also eat humble pie.

@taralx

This comment has been minimized.

Show comment
Hide comment
@taralx

taralx Mar 6, 2018

FYI this introduced CRLF endings to the common/usb_cdc.c file. Harmless, but noisy in the diff.

taralx commented Mar 6, 2018

FYI this introduced CRLF endings to the common/usb_cdc.c file. Harmless, but noisy in the diff.

@micolous micolous referenced this pull request Jun 15, 2018

Closed

USB state abnoraml #6

micolous added a commit to micolous/proxmark3 that referenced this pull request Sep 2, 2018

Fix USB device descriptor issue, set serial number to "iceman1001".
This is a port of upstream PR 565, which addresses USB enumeration issues on
some Android devices, described in
Proxmark#565 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment