-
Notifications
You must be signed in to change notification settings - Fork 20.5k
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
accounts/usbwallet: fix ledger access for latest firmware and add Ledger Flex #31004
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, but I haven't tested.
// Windows and Macos use UsageID matching, Linux uses Interface matching | ||
if info.ProductID == id && (info.UsagePage == hub.usageID || info.Interface == hub.endpointID) { | ||
if (info.ProductID == id || mmOnly == id) && (info.UsagePage == hub.usageID || info.Interface == hub.endpointID) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not look right to me. Consider a future / non-compatible device, which identifies itself as 0x00aa
.
mmOnly := 0x00aa & 0xff00 // 0x0000
. The check mmOnly == id
would match it as a Ledger Blue
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't comment on if Ledger would release a product with an ID like this, but my understanding is that the 0x00** product IDs are legacy and new products would always follow the new MMII format.
An alternative might be to remove this product ID filtering altogether and just rely on the vendor ID lookup we're already doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the product id check would break support for legacy devices.
If they change the scheme (which they might, it's only allowing for 256 devices which they might get to eventually) we will have to store them in the table one by one, or deal with it when the next scheme comes into effect. Maybe they'll change the vendor id as well. No way to know.
The latest firmware for Ledger Nano S Plus now returns
0x5000
for it's product ID, which doesn't match any of the product IDs enumerated inhub.go
.This PR removes the assumption about the interfaces exposed, and simply checks the upper byte for a match.
Also adds support for the
0x0007
/0x7000
product ID (Ledger Flex).