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

[gnokey] Verify and Sign function are not harmonious #2311

Open
thinhnx-var opened this issue Jun 9, 2024 · 2 comments
Open

[gnokey] Verify and Sign function are not harmonious #2311

thinhnx-var opened this issue Jun 9, 2024 · 2 comments
Assignees

Comments

@thinhnx-var
Copy link
Contributor

thinhnx-var commented Jun 9, 2024

Description

Start from this disscusion, I dive into gnokey command and find out somethings that weird with Sign() and Verify()
cc: @zivkovicmilos @leohhhn
Definition:
Verify: To verify a transaction signature is correct
Sign: To Sign the document.

Lets say we save our transaction into file.tx.

gnokey maketx call -pkgpath "gno.land/r/thinhnx/render" -func "Hello" -gas-fee 1000000ugnot -gas-wanted 2000000 myKey > file.tx

Then sign it with:

gnokey sign -tx-path file.tx -chainid "dev" -account-number XX -account-sequence YY myKey

There is a tx.GetSignBytes() that is used inside Sign() gathers arguments from command, in the signOptions:

signBytes, err := tx.GetSignBytes(
  signOpts.chainID,
  signOpts.accountNumber,
  signOpts.accountSequence,
)

Then adds some informations of tx: tx.Fee, tx.Msgs, tx.Memo (from the file.tx) to contruct a SignDoc data.
This SignDoc data will be sorted and marshaled to a []byte signBytes message.
The sign() function will take this final message as a message to sign.

Then the sign() signs this msg, with the corresponse keys, and then writes it (append) into the originial file.tx.

Problems:

The problem is here. The Verify() now take the whole file.tx after signed as input to verify. So, there are things that we should make it clearer: (which I think currently not work)

  1. This Verify() will not works as if its input is the whole file after signed because file.tx is already changed and different from the signBytes. AFAIK we should have same message hash for sign and verify (?)

  2. Since it is an oneway function, and the additional information which is added into the tx.GetSignBytes() comes from command, how to reconstruct the original signBytes to verify?

  3. If we not need to / can not recontruct the orignal signBytes, then should we expose the signedMessage ?

  4. Where can user take the hex format to feed the Verify command. And since the Sign() writes with signature (JSON encoded) into the file.tx, can we remove this argument?

WIP:

I am currently working around this and something relates. I am really looking to hearing from all of you.

@thinhnx-var
Copy link
Contributor Author

I changed the verify flow into this with msg, _ := os.ReadFile(path)
and

afterSignedMsg := std.Tx{}

amino.UnmarshalJSON(msg, &afterSignedMsg)

//Test
theOrigMessageHash, err := afterSignedMsg.GetSignBytes("dev", 56, 0)
err = kb.Verify(name, theOrigMessageHash, sig)

the Verify() is valid.

Im now confusing another way to get the originalMessageHash instead of read the whole file.tx which is not true.

@thinhnx-var
Copy link
Contributor Author

I can reconstruct the messageHash with chainID, account-number and account-sequence.
Should I query for it or just take it as arguments of Verify command?
@zivkovicmilos.

@Kouteki Kouteki moved this from Triage to Backlog in 🧙‍♂️gno.land core team Jun 28, 2024
@leohhhn leohhhn changed the title [gnokey] Verify and Sign functions now are not a pair [gnokey] Verify and Sign function are not harmonious Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

2 participants