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

Arbitrary Message Signing #102

Open
Wolog2021 opened this issue Aug 22, 2022 · 5 comments
Open

Arbitrary Message Signing #102

Wolog2021 opened this issue Aug 22, 2022 · 5 comments

Comments

@Wolog2021
Copy link
Contributor

Wolog2021 commented Aug 22, 2022

We have a proposal from Vacuum Labs that needs review and approval.

Arbitrary message signing Ledger feature proposal:

JS interface:
signMessage(message, path, options)

  • message: message to sign in hex
  • path: derivation path in form, e.g. m/44'/539'/0'/0/0 (the same as for sign call)
  • cryptoOptions: a number encoding hash function and curve used (the same as for sign call)
    Return value (the same as for sign call):
    A dictionary with following fields
    returnCode: number,
    errorMessage: string,
    signatureCompact: Buffer/null,
    signatureDER: Buffer/null,

Behavior:
The behavior depends on the message length and content. As it is infeasible for the client to review large number of screens we define MAXIMUM_SCREENS=15 (You can modify this constant if another value suits is better for you). Screens are described as for NanoS, NanoX ans NanoS+ will have slight modifications due to larger screen size.

If the length of the message length is less than 34xMAXIMUM_SCREENS bytes and it contains only ASCII characters between 32 and 127 (printable ASCII characters), then we will display the message. We will use the same approach as transaction signing uses - we will use flash disc to store the whole message and then the user will be able to review the message content as follows:
Screen 1 to k:
key: "Message (1/k)"
value: 34 bytes of the message
Screen k+1
key: APPROVE
Screen k+2
key: REJECT
In expert mode there will be additional screen showing derivation path, hash function, and curve used.
Otherwise, we will just calculate the rolling hash of the message and show it to the user.
Screen 1:
key, value: "Calculate the message hash on secure device"
Screen 2
key: "Message hash (1/2)"
value: message hash in hex, characters 1-34
Screen 3
key: "Message hash (2/2)"
value: message hash in hex, characters 35-64
Screen 4
key: APPROVE
Screen 5
key: REJECT
In expert mode there will be additional screen showing derivation path, hash function, and curve used.

Reasoning:
If the message is arbitrary and long, it is infeasible to show the user the whole message in any representation. The need of computing the hash on a secure device (or at least separate device - which makes it way harder to perform an attack using a compromised client) is very inconvenient from the user perspective. This also creates security concerns as users may ignore this request and then they are vulnerable to attack by compromised client, however there is very little else that we can reasonably do in this case. That is why we try to show the message whenever it is reasonable.

@relatko
Copy link
Contributor

relatko commented Aug 23, 2022

Just for completeness. Message input parameter is WITHOUT domain tag. The signed message will be signed with "FLOW-V0.0-user" (+ 0 bytes up to domain tag length) prefix. Thus this functionality cannot be misused to sign non-messages.

@bluesign
Copy link

bluesign commented Aug 23, 2022

Otherwise, we will just calculate the rolling hash of the message and show it to the user.

I think this is bad idea from security perspective. It is a footgun to allow dapps to sign non-ascii messages.

Usage of user signing is the problem here:

If someone wants to sign binary data, they should send the hash instead ( so it should be double hashed ) , preferably with some prefix of their own. What user is signing always has to be cleartext.

Consider one dapp, using account signing for messaging ( like IM )

Which let's users sign : Sign ( AWESOME_IM | Sender | Recipient | Message )

example: Sign ( AWESOME_IM | 0x1 | 0x2 | hello world )

now another dapp can inject some non-ascii character to message like : Sign ( 0x1 | 0x2 | hello world [some non-ascii character] )
and asked user to verify the hash on the device. And user would sign it, trusting that what dapp showed and hash on ledger is the same.

This is the same reason that we are not showing only transaction hash when signing the transaction.

@relatko
Copy link
Contributor

relatko commented Aug 23, 2022

@bluesign Thanks for feedback. Showing only hash to the user is of course not optimal from the security point of view. But if the thing to sign is arbitrary there is not much we can do. Are you suggesting that we should only sign messages with displayable ASCII characters that are reasonably short (hashes are like this when given in hex)? If this would e sufficient, it would be definitely superior from security point of view.

@bluesign
Copy link

@relatko exactly, there is no use case for user signing thats doesn’t work with cleartext.

If some dapp for some reason want to sign an arbitrary data, let them send the cleartext hash and sign it. For them nothing will change. (User will see the same hash anyway)

But it changes responsibility and threat model. It prevents malicious dapps to abuse this behavior (as I stated in my example, probably there can be a lot of more )

Also this can lead people to use user signing as intended.

@mmoyes
Copy link

mmoyes commented Aug 23, 2022

Proposal looks good to me for our needs. From a usability point of view it makes sense to limit the number of screens the user would have to scroll through. Therefore I'd suggest limiting MAXIMUM_SCREENS to a smaller number (e.g. 4-6). However, fair argument that this should be on the dapps to not have cumbersomely long messages to sign. TLDR looks good

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

No branches or pull requests

4 participants