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.

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

Support signing messages encoded as hex string #2688

Closed
AndreasGassmann opened this issue Feb 5, 2024 · 5 comments · Fixed by #2690
Closed

Support signing messages encoded as hex string #2688

AndreasGassmann opened this issue Feb 5, 2024 · 5 comments · Fixed by #2690
Assignees

Comments

@AndreasGassmann
Copy link

Is your feature request related to a problem?
When invoking a deeplink to sign a message, the message is expected to be encoded as utf8. Example deeplink:

https://wallet.superhero.com/sign-message?message=<message>&x-success=<success-url>&x-cancel=<cancel-url>

In our use-case, the message is in bytes, usually represented as a hex-string. In the messageToHash method in the aepp-sdk, which is used by signMessage, the buffer is created by using the utf8 encoding. See here.

We have considered simply converting the bytes to utf8, so the correct message would be signed, but it looks like the decoding will not result in the same output as the input was if the bytes are not valid utf8.

Describe the solution you'd like
An additional URL parameter that would allow for signing of bytes represented as a hex-string. Eg.

https://wallet.superhero.com/sign-message?encoding=hex&message=0x1234

Describe alternatives you've considered
As described, we considered using the utf8 encoding, but that does not seem to work.

We cannot change how the verification of the signature is done because it needs to be in a certain format for it to be accepted.

@martinkaintas
Copy link
Contributor

Hello @AndreasGassmann, could you test #2690 and let us know if it resolves your request?

@davidyuk
Copy link
Member

davidyuk commented Feb 6, 2024

Can you elaborate on the use case?

the bytes are not valid utf8

It is the intentional restriction that personal messages are strings (not bytes) because they are meant to be human-readable. As a workaround, developer may sign raw bytes encoded into base-64.

If the message contains special chars used in URLs you need to encode it using encodeURIComponent or so. I'm not sure that there are cases of URL encoding breaking the payload.

If you need to sign a record (made of raw bytes), you can use https://docs.aeternity.com/aepp-sdk-js/v13.2.2/guides/typed-data/ but currently, it is not exposed on the wallet side via deeplinks. This way wallet will be able to show the binary record to sign in a readable way.

@smaroudasunicorn
Copy link
Collaborator

@AndreasGassmann may you provide your insight to the above in order to proceed?

@AndreasGassmann
Copy link
Author

As a workaround, developer may sign raw bytes encoded into base-64.

Could you please give me an example of how that would be possible?

It is the intentional restriction that personal messages are strings (not bytes) because they are meant to be human-readable.

I'm assuming this is done for security? As far as I'm aware, the message is prefixed by something in the end anyway, making it impossible to be mistaken for a valid AE transaction, right?


Our use case is that we would like to use the private key of the user stored in Superhero Wallet to sign transactions for another blockchain. This will allow for easier onboarding of Aeternity / Superhero Wallet users onto our project and allow them to get started quickly, without setting up an additional wallet. For the other blockchain to be able to verify the signature, the original message needs to be in bytes and can potentially contain non utf8 characters (because it was never meant to be human readable).

I hope that makes sense.

@davidyuk
Copy link
Member

davidyuk commented Feb 9, 2024

an example of how that would be possible

import { AeSdk, MemoryAccount } from '@aeternity/aepp-sdk';

const aeSdk = new AeSdk({
  accounts: [MemoryAccount.generate()],
});

const buffer = Buffer.from('deadbeef', 'hex');
const encoded = buffer.toString('base64');
// open(`https://wallet.superhero.com/sign-message?message=${encoded}`);
// it would return the same as:
console.log(await aeSdk.signMessage(encoded));
// but it won't be the same as:
console.log(await aeSdk.signMessage(buffer.toString()))

in this case, the user will see base64-encoded data as a string, the downside is that it is not so byte-efficient, and aepp needs to do extra encoding/decoding to prepare/verify the signature

I'm assuming this is done for security?

I consider providing arbitrary bytes as api misusage. How wallet should show the message to sign if it is bytes? Showing it as text may look like garbage. Showing it as a hex would make it unreadable. If there were multiple aepps asking the user to sign arbitrary bytes then the user wouldn't be able to distinguish the recipient of the message. A malicious aepps would be able to sign a message for a third-party aepp without the user noticing that.

To protect against that aepps should sign messages like "<action> on domain.com", this would hint to the user on what is being done in the above case. But again it won't work if the message shows as hex because the user would see the domain in hex as well.

As far as I'm aware, the message is prefixed by something in the end anyway, making it impossible to be mistaken for a valid AE transaction, right?

Correct, here is no issues.

to sign transactions for another blockchain

You can use the existing sign-message deeplink, but construct the message in a more sophisticated way. Like "I confirm transaction <transaction hash encoded as text> on <blockchain name>". If you are verifying message signature on a contract side you need a utility to convert a binary transaction to some text representation, it may be tricky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants