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

Create Example for signMessage #434

Closed
lostpebble opened this issue Aug 30, 2022 · 6 comments
Closed

Create Example for signMessage #434

lostpebble opened this issue Aug 30, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@lostpebble
Copy link
Contributor

Hi,

Could it be possible for the Guestbook example to do more than just alert out the result of the verifyOwner() API method, currently it looks like this:

const handleVerifyOwner = async () => {
    const wallet = await selector.wallet();
    try {
      const owner = await wallet.verifyOwner({
        message: "test message for verification",
      });

      if (owner) {
        alert(`Signature for verification: ${JSON.stringify(owner)}`);
      }
    } catch (err) {
      const message =
        err instanceof Error ? err.message : "Something went wrong";
      alert(message);
    }
  }

From what I can gather, this won't throw any errors if the signing was done incorrectly. It would be great if it actually ran some kind of authentication to make sure that the API is working as intended.

I'm busy building out the API for Meteor, and it seems like I can just use the account.connection.signer.signMessage() method which is provided by the Near JS library directly in our dapp-side code- but I think that its probably signing the message with the wrong private key (the local key for the Dapp)- and I'm guessing that this verifyOwner() method actually requires that the message is signed by a different private key- one that is only accessible from inside the Wallet application.

If I could have some confirmation on this it would be great- and for our testing, it would be great if there was an easy way we could test this using the Guestbook application as well.

I'd be happy to make a PR for this as well, I just need to be pointed a bit in the right direction here.

Thanks!

@lostpebble lostpebble added the enhancement New feature or request label Aug 30, 2022
@lostpebble
Copy link
Contributor Author

lostpebble commented Sep 1, 2022

So, in building this functionality out for Meteor, I've figured out I can verify the returned payload like so:

const owner = await wallet.verifyOwner({
  message: "test message for verification",
});

const publicKeyString = `ed25519:${bs58.encode(
  Buffer.from(owner.publicKey, "base64"),
)}`;

const createdPublicKey = utils.PublicKey.from(publicKeyString);

const recreatedMessage: any = {
  ...owner,
};

delete recreatedMessage.signature;

const stringified = JSON.stringify(recreatedMessage);

const verifiedSignature = createdPublicKey.verify(
  new Uint8Array(sha256.array(stringified)),
  Buffer.from(verifyResponse.payload.verifiedOwner.signature, "base64"),
);

Here verifiedSignature will equal true at the end (if the signing was done correctly).

This is based off of the interfaces given inside the code here in Wallet Selector, and that of the Near Wallet github.

I think there are some worries about this whole interface. The first one being that because the message that needs to be validated against is actually not the passed message itself, but its an object of the returned value without the signature applied to JSON.stringify(). In Near Wallet's implementation it looks like this:

const data = {
    accountId,
    message,
    blockId: blockInfo.header.hash,
    publicKey: Buffer.from(publicKey.data).toString('base64'),
    keyType: publicKey.keyType
};

This object has to be recreated again, perfectly and in the exact same property order to create the "payload" to verify that the message was signed correctly. If the properties are not in the exact same order, it will fail- because the JSON.stringify() will create a string of the object that's not equal to how it was originally made. Dapps would need to be informed of the exact order required for recreation. Overall, this seems a bit fickle.

I understand that the message is not encoded directly because of security concerns. But surely there could have been a more "stable" data structure to use in this case. Perhaps just an array with the interface of [accountId, message, blockId, publicKey] (with the public key being a fully formed string key so we don't need the keyType)? And in this case, would blockId even be necessary? I'm not very clued up on how this could be abused, but I would think an output data structure like this probably doesn't have an input area that could be abused? Not having blockId would mean we don't have to make any external calls at all in this process.

And on the next note- why are we not just sending the public key as a ed25519:* string from the get go? This would prevent having to send keyType and having to convert it using bs58 on the Dapp side again. This is also the string format which can be directly compared to the public keys which are returned from the Near API for a certain account.

There is also another concern about the keyType field, as its a bit ambiguous inside the code of Near Wallet, this is what it returns after the signing of the message things:

return {
    ...data,
    signature: Buffer.from(signed.signed.signature).toString('base64'),
    keyType: signed.signed.publicKey.keyType
};

As you can see, it is being overridden by signed.signed.publicKey.keyType - so in this case, the original publicKey's type is unknown (though most likely it is the same...).

All in all, it feels like this interface is quite important to get done in a future proof and standardized way for all future wallets and dapps to implement exactly the same way. I haven't seen too much about the standardization around the data structures here, besides some mentions inside github issues.

@exalate-issue-sync exalate-issue-sync bot changed the title Actually verify the signed message in the Guestbook example Create Example for Verify Owner Nov 18, 2022
@gagdiez
Copy link

gagdiez commented Nov 29, 2022

NEP413 is ready to be reviewed. This means we could start implementing it, since no major changes are expected to the interface.

@gagdiez gagdiez changed the title Create Example for Verify Owner Create Example for signMessage Nov 29, 2022
@kujtimprenkuSQA
Copy link
Contributor

NEP413 is ready to be reviewed. This means we could start implementing it, since no major changes are expected to the interface.

@gagdiez, it's a bit confusing for us since the NEP changed from the initial proposal.

We see that the name of the function has been changed from verifyOwner to signMessage does that mean we need to replace verifyOwner in BaseWalletBehaviour with signMessage in wallet-selector or should it still be named verifyOwner in this repo?

What will be the name of the function in wallets?

This is going to be a breaking change either way in the wallet selector since even the arguments and their types this function takes have been changed.

@gagdiez
Copy link

gagdiez commented Dec 13, 2022

@kujtimprenkuSQA the name has changed to signMessage, since verifiOwner was not a great name for a method that signs a message.

The new naming convention should be used, thus renaming it everywhere.

@frol
Copy link
Collaborator

frol commented Mar 2, 2023

@kujtimprenkuSQA
Copy link
Contributor

Closing this issue as the PR for signMessage #779 has been merged and in both of our examples react and angular it is shown how to sign and verify the message based on the signMessage NEP near/NEPs#413

Yesterday a new version v8.3.0 of wallet-selector has been published and it includes support for signMessage too:
https://www.npmjs.com/org/near-wallet-selector

Currently only Meteor Wallet supports this feature as per NEP near/NEPs#413.

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

No branches or pull requests

4 participants