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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 24, 2025
Merged

accounts/usbwallet: fix ledger access for latest firmware and add Ledger Flex #31004

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e18676
Fix ledger access for latest firmware
mdehoog Jan 7, 2025
d99a45b
Also support ledger flex
mdehoog Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions accounts/usbwallet/hub.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,26 +73,22 @@ func NewLedgerHub() (*Hub, error) {
return newHub(LedgerScheme, 0x2c97, []uint16{

// Device definitions taken from
// https://github.com/LedgerHQ/ledger-live/blob/38012bc8899e0f07149ea9cfe7e64b2c146bc92b/libs/ledgerjs/packages/devices/src/index.ts
// https://github.com/LedgerHQ/ledger-live/blob/595cb73b7e6622dbbcfc11867082ddc886f1bf01/libs/ledgerjs/packages/devices/src/index.ts

// Original product IDs
0x0000, /* Ledger Blue */
0x0001, /* Ledger Nano S */
0x0004, /* Ledger Nano X */
0x0005, /* Ledger Nano S Plus */
0x0006, /* Ledger Nano FTS */

0x0015, /* HID + U2F + WebUSB Ledger Blue */
0x1015, /* HID + U2F + WebUSB Ledger Nano S */
0x4015, /* HID + U2F + WebUSB Ledger Nano X */
0x5015, /* HID + U2F + WebUSB Ledger Nano S Plus */
0x6015, /* HID + U2F + WebUSB Ledger Nano FTS */

0x0011, /* HID + WebUSB Ledger Blue */
0x1011, /* HID + WebUSB Ledger Nano S */
0x4011, /* HID + WebUSB Ledger Nano X */
0x5011, /* HID + WebUSB Ledger Nano S Plus */
0x6011, /* HID + WebUSB Ledger Nano FTS */
0x0007, /* Ledger Flex */

0x0000, /* WebUSB Ledger Blue */
0x1000, /* WebUSB Ledger Nano S */
0x4000, /* WebUSB Ledger Nano X */
0x5000, /* WebUSB Ledger Nano S Plus */
0x6000, /* WebUSB Ledger Nano FTS */
0x7000, /* WebUSB Ledger Flex */
}, 0xffa0, 0, newLedgerDriver)
}

Expand Down Expand Up @@ -185,8 +181,11 @@ func (hub *Hub) refreshWallets() {

for _, info := range infos {
for _, id := range hub.productIDs {
// We check both the raw ProductID (legacy) and just the upper byte, as Ledger
// uses `MMII`, encoding a model (MM) and an interface bitfield (II)
mmOnly := info.ProductID & 0xff00
// 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) {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Member

@gballet gballet Jan 23, 2025 •
edited
Loading

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.

devices = append(devices, info)
break
}
Expand Down