search

madfish-solutions / templewallet-extension

🔐💰Cryptocurrency wallet for Tezos blockchain as Web extension for your Browser.
https://templewallet.com
MIT License
184 stars 62 forks source link

Signing messages produces an invalid message after today's Chrome extension update #315

Closed drewstaylor closed 2 years ago

drewstaylor commented 3 years ago

It would seem that message signing is broken after the latest update. Players using Temple wallet are not able to login on our platform, while other wallet providers such as Kukai do not have issues.

It would seem that Temple is hex encoding the message payload a second time, unlike the other wallet providers. This places us in a situation if we fix it for Temple it's going to break for any other wallet provider.

The data depicted in the link below should be JSON but Temple has hex encoded it: Screenshot of invalid message

drewstaylor commented 3 years ago

It seems like this commit is what caused the issue: https://github.com/madfish-solutions/templewallet-extension/commit/895c09abac5f9d18e9e45503e86873eb44649820

serg-plusplus commented 3 years ago

But that's how the Beacon protocol works: https://docs.walletbeacon.io/guides/sign-payload I have tested SigningType.RAW with Kukai, it doesn't work.

drewstaylor commented 3 years ago

Which signing type does Beacon default to? Our site definitely works for Kukai signing and we've had to add special code(for now) to handle Temple but not for any other wallets. My understanding is that Beacon must have updated recently.

lukel97 commented 3 years ago

We are running into this as well. Beacon defaults to the raw signing type. I'm not 100% sure as to why beacon went down this route, since TZIP-10 doesn't seem to mention it. The only "documentation" about this is a single comment in beacon which says it will be hashed, not encoded before signing:

RAW = 'raw', // Arbitrary payload (string), which will be hashed before signing

Is this not talking about the blake2b hash? I can't see anything that mentions hex encoding

AndreasGassmann commented 3 years ago

Sorry for the late reply, I forgot about this issue after the temple team reported it to us.

The change with the signingType was introduced because Kukai doesn't (want to) support signing of arbitrary data. https://docs.walletbeacon.io/supported-wallets Signing arbitrary data always has a risk of being exploited, so they decided not to support it. Instead they defined a micheline "wrapper" that wraps an arbitrary payload, which, in its packed form, results in a string that is 05 prefixed. This can never be a valid transaction because those need to start with 03. (See https://docs.walletbeacon.io/guides/sign-payload/#hex-prefixed-with-03 for an example that works with Kukai).

That being said, the addition of the signing type was not a breaking change. Before the signing types were introduced, all payloads were in the "raw" format. It also defaults to the raw type to not break existing implementations that don't explicitly specify a signing type.

The payload that is received in a "raw" request is an arbitrary string. This string has to be hashed (blake2b) by the wallet. This is done for 2 reasons:

  1. That way the wallet can show the original string to the user and the hash will be generated in a secure context (the wallet).
  2. The ledger can only sign data up to a certain length, so it needs to be hashed.

I'm not sure if this issue has ever been resolved, but this is the reasoning behind it.

unixvb commented 2 years ago

fixed