terra-money / wallet-provider

Library to make React dApps easier using Terra Station Extension or Terra Station Mobile.
https://www.npmjs.com/package/@terra-money/wallet-provider
Apache License 2.0
90 stars 68 forks source link

[Feature Request] arbitrary data signing #21

Closed yun-yeo closed 2 years ago

yun-yeo commented 3 years ago

Can we make new interface for signing the arbitrary data like metamask

https://docs.metamask.io/guide/signing-data.html#signing-data-with-metamask

Iā€™m not sure this already available, can we get the pubkey of address?? If not, it also good to have that feature too.

yun-yeo commented 3 years ago

Seems already had discussion https://github.com/terra-money/wallet-provider/pull/7

cosmwasm now support contract side sig verification https://lib.rs/crates/cosmwasm-crypto

so it is good time to consider this again

RonniePark commented 3 years ago

Hi.

I'm a newbie and trying to build a web/mobile application base on Terra. But I couldn't find solution to verify owner of wallet from server side like metamask+web3 flow.

To boom up many applications with Terra, this feature will be necessary and very useful. :D

There was same request as well. sign and verify message using terra.js

Thanks for support.

iamssen commented 3 years ago

https://github.com/terra-money/wallet-provider/pull/38

tkowalczyk commented 2 years ago

@iamssen is sign and verify working now?

iamssen commented 2 years ago

@tkowalczyk https://github.com/terra-money/wallet-provider/pull/38 please refer this PR

tkowalczyk commented 2 years ago

Yeah I saw it but when it be ready for production use?

iamssen commented 2 years ago

@tkowalczyk There is a third-party developer who is developing using this function, and the third-party must complete the test for this function. I am also waiting because it is not an API created by my needs.

To test this function, you need to install the test version station extension on your chrome as unpacked, and if you want to test this function, I will send you a zip file.

tk-o commented 2 years ago

Hey @iamssen, could you please provide more info on how to reach out to this 3rd party developer to see what it the expected timeline on this particular feature? Cheers šŸ¤

iamssen commented 2 years ago

@tk-o Hmm... We're currently developing this with some third-party, but there's no feedback from them. I think it is okay to develop this function with the developers of this issue.

Currently, I am focusing on work for wallet-provider 3.0 major update. Maybe late next week, I will prepare an extension and wallet-provider to test this function.

I'll let you know when I'm ready. It would be okay to proceed with API development according to your feedback.

tk-o commented 2 years ago

Excellent! This feature is crucial for off-chain authentication, so I'm happy to provide my feedback once it's available for testing. Thank you for your efforts, @iamssen šŸ’Ŗ

iamssen commented 2 years ago
import { MnemonicKey, RawKey } from '@terra-money/terra.js';
import { randomBytes } from 'crypto';
import jscrypto from 'jscrypto';
import secp256k1 from 'secp256k1';

describe('sign-bytes', () => {  
  test('terra.js secp256k1 ecdsaSign', () => {
    // create key (same as extension wallet creation)
    const mk = new MnemonicKey({ coinType: 330 });
    const privateKey = mk.privateKey.toString('hex');
    const key = new RawKey(Buffer.from(privateKey, 'hex'));
    //@ts-ignore
    const publicKey = key.publicKey!.toProto().key;

    // create message
    const bytes = Buffer.from('hello world');

    // ecdsaSign - this function transform the bytes to
    //     message = Uint8Array.from(
    //                  Buffer.from(
    //                      jscrypto.SHA256.hash(
    //                          new jscrypto.Word32Array(bytes)
    //                      ).toString(),
    //                  'hex')
    //               )
    const { signature, recid } = key.ecdsaSign(bytes);

    // (extension will return this data) return sign-bytes result to user
    const userReturn = {
      recid,
      signature,
      publicKey: Buffer.from(publicKey).toString('base64'),
    };

    // (developer verify the user data by the return data above) verify by userReturn data
    expect(
      secp256k1.ecdsaVerify(
        userReturn.signature,
        Buffer.from(
          jscrypto.SHA256.hash(new jscrypto.Word32Array(bytes)).toString(),
          'hex',
        ),
        Buffer.from(userReturn.publicKey, 'base64'),
      ),
    ).toBeTruthy();
  });
});

@tk-o @tkowalczyk I will return the recid, signature, and public_key as above as a result of the bytes received from you. Can you make what you want with this return data?

tk-o commented 2 years ago

My use case is authentication:

  1. A user shares their public key/wallet address requests sign in action
  2. A server responds with a randomly generated string (a challenge)
  3. The user signs the challenge with their private key and sends the signature back to server
  4. The server verifies if the provided signature matches the public key from point 1.

Depending on the verification, server considers the user "signed in", or returns authentication error.

iamssen commented 2 years ago

I updated include this feature in wallet-provider@3.0

https://github.com/terra-money/wallet-provider/releases/tag/v3.0.0

This feature will not be supported by the current Terra Station extension. You can test it by referring to this link. https://www.npmjs.com/package/terra-web-extension

This feature will also be updated to the current Terra Station extension in a while.

tkowalczyk commented 2 years ago

I just noticed that discord supporting service - Lunar Assistant has a feature for signing transactions: https://www.lunarassistant.com/

I am wondering how did they accomplish to do this, please check the screens:

obraz

Any ideas guys?

obraz

tk-o commented 2 years ago

@tkowalczyk here's how:

But from what I can see the verification step is hardcoded to always consider signature valid: https://github.com/GraviDAO/lunar-assistant-app/blob/main/src/pages/api/lunarVerify.ts#L34-L40


Sir @iamssen,

The Web 3.0 is about "do not trust, verify". Following this theme, we should focus on having an easy API to

Perhaps we could have a crypto-utils package build on top of https://github.com/polkadot-js/common/tree/master/packages/util-crypto to save a lot of work, and just connect it to the Terra Station via an adapter?

Any thoughts on this approach?

iamtrex commented 2 years ago

Hey @iamssen , I'm looking forward to this feature as well. Any idea when it will be supported in the official extension?

Ruborcalor commented 2 years ago

Hey everyone!

Yes as you pointed out @tk-o, the verification step is currently hardcoded to always consider the signature valid. I used to do proper verification, but then breaking changes were introduced and I haven't been able to get it working since then.

Because signBytes is not yet available, I was using a workaround of signing a dummy transaction which sends 0 ust to a dummy address. I'm also looking forward to signBytes becoming available via the Terra Station extension.

@iamssen , do you have advice for how to verify a signed transaction? This is my attempt:

                  // Sign the transaction
                  const verificationTransaction = await connectedWallet.sign({
                    fee: new Fee(0, '0uusd'),
                    msgs: [
                      new MsgSend(
                        connectedWallet.walletAddress,
                        'terra1f5u6ds3q95jwl2y5ellsczuwd2349g68u8af4l',
                        { uusd: 0 },
                      ),
                    ],
                  });

                  // Prepare the relevant pieces of the transaction
                  const pub_key =
                    verificationTransaction.result.auth_info.signer_infos[0]
                      .public_key;

                  const body: LunarVerifyRequest = {
                    signature: verificationTransaction.result.signatures[0],
                    message: SHA256.hash(
                      JSON.stringify(
                        verificationTransaction.result.body.messages[0].toJSON(),
                      ),
                    ).toString(),
                    walletAddress: connectedWallet.walletAddress,
                    jwt: Array.isArray(jwtString)
                      ? jwtString.join()
                      : jwtString || '',
                    publicKey:
                      pub_key instanceof SimplePublicKey
                        ? pub_key.key
                        : pub_key instanceof ValConsPublicKey
                        ? pub_key.key
                        : '',
                  };

                  // Try to verify the transaction

                  // get the signature buffer
                  const sigBuffer = Buffer.from(
                    body.signature,
                    'base64',
                  );

                  // get the hash buffer
                  const hashBuffer = Buffer.from(
                    body.message,
                    'hex',
                  );

                  // get the public key buffer
                  const pubBuffer = Buffer.from(
                    body.publicKey,
                    'base64',
                  );

                  // verify the transaction is legit
                  // This does NOT work
                  const verified = secp256k1.ecdsaVerify(
                    Uint8Array.from(sigBuffer),
                    Uint8Array.from(hashBuffer),
                    Uint8Array.from(pubBuffer),
                  );

Thanks! :)

iamssen commented 2 years ago

Hey @iamssen , I'm looking forward to this feature as well. Any idea when it will be supported in the official extension?

@iamtrex I think it will be soon.

iamssen commented 2 years ago

@Ruborcalor Um... Actually, I don't know exactly what this function is for. šŸ˜…

So, I'm asking if this signBytes() is the function you want.

https://www.npmjs.com/package/terra-web-extension You may be testing through this new extension. signBytes() will soon be added to the current extension. (I heard that signBytes() will be added soon at today's meeting)

tk-o commented 2 years ago

signBytes() will soon be added to the current extension. (I heard that signBytes() will be added soon at today's meeting)

Amazing news šŸš€

Ruborcalor commented 2 years ago

@iamssen Yes signBytes() is the function I want, but in the meantime I was looking for a function similar to verifyBytes that I could use to verify the output of a call to sign(). This is the function I'm trying to construct above.

Echoing tk-0 that it's awesome to hear signBytes() will soon be added to the current extension!

iamssen commented 2 years ago

https://github.com/terra-money/wallet-provider/pull/49

iamssen commented 2 years ago

v3.3.0 released

npm install @terra-money/wallet-provider@latest
Ruborcalor commented 2 years ago

@tkowalczyk here's how:

* https://github.com/GraviDAO/lunar-assistant-app/blob/main/src/components/VerificationSteps.tsx#L225-L286

* https://github.com/GraviDAO/lunar-assistant-app/blob/main/src/pages/api/lunarVerify.ts

But from what I can see the verification step is hardcoded to always consider signature valid: https://github.com/GraviDAO/lunar-assistant-app/blob/main/src/pages/api/lunarVerify.ts#L34-L40

@tkowalczyk I have now updated the code to properly check whether or not a signature is valid using signBytes if you want to take a look.`

RonniePark commented 2 years ago

Hi.

Thanks for support this feature and it works fine as the guide.

It looks like verify step in client side as below example if I understand correctly. https://github.com/terra-money/wallet-provider/blob/main/templates/next/pages/sign-bytes-sample.tsx#L35

Is there any way to verify in backend server side as well? (ex. terra.js in node) Because, I need to mapping who is owner of wallet with a user account in backend side after user signed.

This is an example in Ethereum case what I looking for, https://metamask.github.io/eth-sig-util/modules.html#recoverPersonalSignature

Thanks. :)

tk-o commented 2 years ago

Hey @RonniePark,

I could not make it work on Node.js end (there was some issue with having terrajs lib loaded, I can't remember now).

What I did was a bit of code gymnastics on the client side:

    // `bytes` is a var of type `Buffer`
    const { result: signBytesResultRaw } = await connectedWallet.signBytes(bytes)
    const publicKeyWrapped = signBytesResultRaw.public_key?.toProto()

    return {
      signature: Buffer.from(signBytesResultRaw.signature),
      message: bytes,
      // @ts-ignore
      publicKey: publicKeyWrapped.key,
    }

And then I send the object from the return statement to the Node.js API.

Here's the Node.js piece of code:

import secp256k1 from 'secp256k1'
import jscrypto from 'jscrypto'

function authenticationHandler(req, res) {
  const body = req.body

  let messageBytes = Uint8Array.from(body.message)
  let signatureBytes = Uint8Array.from(body.signature)
  let publicKeyBytes = Uint8Array.from(body.publicKey)

  res.json({
    isVerified: verify({
      messageBytes,
      signatureBytes,
      publicKeyBytes,
    }),
  })
}

interface VerifyProps {
  messageBytes: Uint8Array
  signatureBytes: Uint8Array
  publicKeyBytes: Uint8Array
}

function verify({ messageBytes, signatureBytes, publicKeyBytes }: VerifyProps): boolean {
  let messageAltBytes = jscrypto.SHA256.hash(new jscrypto.Word32Array(messageBytes)).toUint8Array()

  return secp256k1.ecdsaVerify(signatureBytes, messageAltBytes, publicKeyBytes)
}
RonniePark commented 2 years ago

Thanks @tk-o a lot for quick response, and let me take a look and try. It's useful and great reference for me. :)

From the verify step in above original question, I just wanna make sure and clarify that the requestor from client is exactly same with owner of that wallet, also this feature would be big impact whenever sdk update.

Then I thought like this is need to be support officially through sdk instead of implement by individual, because of that would be more safe and reduce dependency in future update.

I will try as @tk-o mentioned for now and it would be great if @iamssen consider that verify step in backend server side sdk as well. :D

Thanks.