xaionaro-go / cryptoWallet

A library to access to basic functions of hardware cryptocurrencies wallets
GNU Lesser General Public License v3.0
8 stars 6 forks source link

EncryptKey/DecryptKey return whole PB message, not just value #7

Closed karelbilek closed 5 years ago

karelbilek commented 5 years ago

EncryptKey/DecryptKey return whole PB message, not just value

This code just returns bytes from the PB message https://github.com/xaionaro-go/cryptoWallet/blob/47f9f6877e4324a8bc47fc5661c32d2fe6d29586/internal/wallets/satoshilabs/trezor/trezor.go#L124-L127

and that's used directly as a key here

https://github.com/xaionaro-go/cryptoWallet/blob/47f9f6877e4324a8bc47fc5661c32d2fe6d29586/internal/wallets/satoshilabs/trezor/trezor.go#L186-L196

but it's never de-marshaled from protobuf.

https://github.com/trezor/trezor-common/blob/04bac52951764d5516206f5359cfc5f5c24eb38f/protob/messages-crypto.proto#L28-L30

karelbilek commented 5 years ago

I am currently re-writing xaionaro-go/cryptoWallet to use my fork of trezord-go as a library here

https://github.com/trezor/trezord-go/pull/147

which will cause the code here to simplify a lot (I am basically removing all the intermediary classes) and improve the code here. (And I am also removing the dependency on tesoro, which is unmaintained.)

karelbilek commented 5 years ago

Similarly, Ping result is not de-marshaled, as isn't Features. (I wonder how could it work for you :D )

karelbilek commented 5 years ago

I see JSON unmarshaling here, which makes no sense, as JSON is not used anywhere in trezor

https://github.com/xaionaro-go/cryptoWallet/blob/57b96a3e8d609d2e0c043d92aa9472bc9e462639/internal/wallets/satoshilabs/trezor/trezor.go#L99

xaionaro commented 5 years ago

Hello. I have no time on Trezors right now, so the only thing I can do is review and merge patches :(

I see JSON unmarshaling here, which makes no sense, as JSON is not used anywhere in trezor

If this's true then it's very strange. Because I remember for sure that the answer was in JSON. May be the device returns it that way or something. Or, may be it's just obsolete.

Similarly, Ping result is not de-marshaled, as isn't Features. (I wonder how could it work for you :D )

Well, it really worked, too :) I cannot answer anything here because of no time on investigations... Sorry :(

but it's never de-marshaled from protobuf.

Yep, seems to be a mistake.

xaionaro commented 5 years ago

UPD:

I've just looked into the code a little because I couldn't believe I was so careless. And it seems that actually everything is OK. The unmarshaling was done here:

https://github.com/conejoninja/tesoro/blob/master/tesoro.go#L864

And JSON messages are about there, too.

karelbilek commented 5 years ago

OH OK! I looked at wrong place. In that case, closing.

(as I said, I am rewriting this library + tesoro. I will make you a PR once it's done, but basically, at that point, this library will be basically almost empty.)