search

summa-tx / coins

Rust implementations of BIP32/39 and Ledger device comms
Other
90 stars 31 forks source link

Hard-coded CLA for app-ethereum (`0xE0`) #137

Open xJonathanLEI opened 4 weeks ago

xJonathanLEI commented 4 weeks ago

In the implementation of APDUCommand's write_to() method, the first serialized byte (CLA) is hard-coded to be 0xE0, the application identifier for app-ethereum:

https://github.com/summa-tx/coins/blob/51c72c14cb1c0d55bb78b292b80846dd1c1d7a2c/crates/ledger/src/common.rs#L104

This has resulted in the coins-ledger app not being functional with anything other than the Ethereum Ledger app. It seems to me that this crate contains otherwise nothing Ethereum-specific. So I guess this should be considered a bug.

xJonathanLEI commented 4 weeks ago

The appropriate fix here seems to add a cla field to APDUCommand. That would be a breaking change though. What do you think? @prestwich

prestwich commented 3 weeks ago

can you provide some citation? I have used this library with bitcoin a number of times

xJonathanLEI commented 3 weeks ago

Yep. So it looks like all the OG apps use 0xE0, including app-ethereum and app-bitcoin.

So first for app-ethereum you can see here it's using CLA of 0xE0.

Then for legacy app-bitcoin here shows it's also using that. This explains why you've been able to use it with the Bitcoin app as well.

Apparently the new Bitcoin app (i.e. app-bitcoin-new) uses 0xE1 instead, as shown here. What this means is that using this library even for the new Bitcoin app wouldn't work.

Then now you have the Starknet app which uses 0x5A.

Hopefully this shows how CLA should be configurable instead of hard-coded :)