From 43f971e05383859184270c82d39a84d7bb0885dd Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Fri, 20 Nov 2020 12:22:28 +0100 Subject: [PATCH 1/3] Updated signatures to ethereum's Text Sign format Ethereum is hasing a message for signing with a dedicated hashing function that is adding more information to a signed message: keccak256("\x19Ethereum Signed Message:\n"${message length}${message}) In this commit we are using ethereum's hashing function to obtain digest to sign. We also extracted parts of the functions to be able to test them. Having these changes signatures should be verifiable on mycrypto.com site. --- cmd/signing_ethereum.go | 68 ++++++++++++------- cmd/signing_ethereum_test.go | 127 +++++++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 25 deletions(-) create mode 100644 cmd/signing_ethereum_test.go diff --git a/cmd/signing_ethereum.go b/cmd/signing_ethereum.go index 2d1a22fc8..2e4bb9e9c 100644 --- a/cmd/signing_ethereum.go +++ b/cmd/signing_ethereum.go @@ -2,15 +2,16 @@ package cmd import ( "bytes" - "crypto/sha256" - "encoding/hex" "encoding/json" "fmt" "io/ioutil" "os" "path/filepath" + "github.com/ethereum/go-ethereum/accounts" + "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/crypto" "github.com/keep-network/keep-common/pkg/chain/ethereum/ethutil" "github.com/keep-network/keep-ecdsa/internal/config" @@ -58,9 +59,10 @@ var EthereumSigningCommand = cli.Command{ } const ethereumSignDescription = `Calculates an ethereum signature for a given message. -The message is expected to be provided as a string, it is later hashed with SHA-256 -and passed to Ethereum ECDSA signing. Signature is calculated in Ethereum specific -format as a hexadecimal string representation of 65-byte {R, S, V} parameters. +The message is expected to be provided as a string, it is later hashed with +ethereum's hashing algorithm and passed to Ethereum ECDSA signing. Signature is +calculated in Ethereum specific format as a hexadecimal string representation of +65-byte {R, S, V} parameters, where V is 0 or 1. It requires an Ethereum key to be provided in an encrypted file. A path to the key file can be configured in a config file or specified directly with an 'eth-key-file' flag. @@ -134,18 +136,9 @@ func EthereumSign(c *cli.Context) error { ) } - digest := sha256.Sum256([]byte(message)) - - signature, err := crypto.Sign(digest[:], ethereumKey.PrivateKey) + ethereumSignature, err := sign(ethereumKey, message) if err != nil { - return fmt.Errorf("failed to sign: [%v]", err) - } - - ethereumSignature := &EthereumSignature{ - Address: ethereumKey.Address, - Message: message, - Signature: hex.EncodeToString(signature), - Version: ethSignatureVersion, + return fmt.Errorf("signing failed: [%v]", err) } marshaledSignature, err := json.Marshal(ethereumSignature) @@ -181,6 +174,37 @@ func EthereumVerify(c *cli.Context) error { return fmt.Errorf("failed to unmarshal ethereum signature: [%v]", err) } + err = verify(ethereumSignature) + if err != nil { + return fmt.Errorf("signature verification failed: [%v]", err) + } + + fmt.Printf( + "signature verified correctly, message [%s] was signed by [%s]\n", + ethereumSignature.Message, + ethereumSignature.Address.Hex(), + ) + + return nil +} + +func sign(ethereumKey *keystore.Key, message string) (*EthereumSignature, error) { + digest := accounts.TextHash([]byte(message)) + + signature, err := crypto.Sign(digest[:], ethereumKey.PrivateKey) + if err != nil { + return nil, fmt.Errorf("failed to sign: [%v]", err) + } + + return &EthereumSignature{ + Address: ethereumKey.Address, + Message: message, + Signature: hexutil.Encode(signature), + Version: ethSignatureVersion, + }, nil +} + +func verify(ethereumSignature *EthereumSignature) error { if ethereumSignature.Version != ethSignatureVersion { return fmt.Errorf( "unsupported ethereum signature version\n"+ @@ -191,9 +215,9 @@ func EthereumVerify(c *cli.Context) error { ) } - digest := sha256.Sum256([]byte(ethereumSignature.Message)) + digest := accounts.TextHash([]byte(ethereumSignature.Message)) - signatureBytes, err := hex.DecodeString(ethereumSignature.Signature) + signatureBytes, err := hexutil.Decode(ethereumSignature.Signature) if err != nil { return fmt.Errorf("failed to decode signature: [%v]", err) } @@ -207,7 +231,7 @@ func EthereumVerify(c *cli.Context) error { if !bytes.Equal(recoveredAddress.Bytes(), ethereumSignature.Address.Bytes()) { return fmt.Errorf( - "signature verification failed: invalid signer\n"+ + "invalid signer\n"+ "\texpected: %s\n"+ "\tactual: %s", ethereumSignature.Address.Hex(), @@ -215,11 +239,5 @@ func EthereumVerify(c *cli.Context) error { ) } - fmt.Printf( - "signature verified correctly, message [%s] was signed by [%s]\n", - ethereumSignature.Message, - recoveredAddress.Hex(), - ) - return nil } diff --git a/cmd/signing_ethereum_test.go b/cmd/signing_ethereum_test.go new file mode 100644 index 000000000..5a7ea4948 --- /dev/null +++ b/cmd/signing_ethereum_test.go @@ -0,0 +1,127 @@ +package cmd + +import ( + "fmt" + "reflect" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/keep-network/keep-common/pkg/chain/ethereum/ethutil" +) + +// Signatures should match a signature format on mycrypto.com. A signing/verification +// tool available on https://mycrypto.com/sign-and-verify-message can be used to +// cross-check correctness of signatures provided by our implementation. + +var validSignature = EthereumSignature{ + Address: common.HexToAddress("0x4BCFC3099F12C53D01Da46695CC8776be584b946"), + Message: "verySecretMessage", + Signature: "0xc8be189ab0ee691de7019eaa3de58558b84775085d9a0840908343ac690e02ca3f6e3d2dc70025b9b214d96c30e38c41f818cccd6f06b7a81c4afd26cbe6d6d600", + Version: 2, +} + +func TestSign(t *testing.T) { + message := "verySecretMessage" + keyFilePath := "../internal/testdata/eth_key.json" + keyFilePassword := "password" + + expectedResult := &EthereumSignature{ + Address: common.HexToAddress("0x4BCFC3099F12C53D01Da46695CC8776be584b946"), + Message: message, + Signature: "0xc8be189ab0ee691de7019eaa3de58558b84775085d9a0840908343ac690e02ca3f6e3d2dc70025b9b214d96c30e38c41f818cccd6f06b7a81c4afd26cbe6d6d600", + Version: 2, + } + + ethereumKey, err := ethutil.DecryptKeyFile(keyFilePath, keyFilePassword) + if err != nil { + t.Fatalf( + "failed to read key file [%s]: [%v]", + keyFilePath, + err, + ) + } + + ethereumSignature, err := sign(ethereumKey, message) + if err != nil { + t.Errorf("signing failed: [%v]", err) + } + + if !reflect.DeepEqual(ethereumSignature, expectedResult) { + t.Errorf( + "unexpected signature\nexpected: %v\nactual: %v", + expectedResult, + ethereumSignature, + ) + } +} + +func TestVerify_V0(t *testing.T) { + err := verify(&validSignature) + if err != nil { + t.Errorf("unexpected error: [%v]", err) + } +} + +func TestVerify_V27(t *testing.T) { + // go-ethereum library produces a signature with V value of 0 or 1. In some + // chains the V value is expected to be 27 or 28. Even ethereum is sometimes + // inconsistent about that across their libraries. In our implementation we + // expect V to be 0 or 1, we're not currently supporting 27 or 28. + ethereumSignature := validSignature + ethereumSignature.Signature = "0xc8be189ab0ee691de7019eaa3de58558b84775085d9a0840908343ac690e02ca3f6e3d2dc70025b9b214d96c30e38c41f818cccd6f06b7a81c4afd26cbe6d6d61b" + + expectedError := fmt.Errorf("could not recover public key from signature [invalid signature recovery id]") + + err := verify(ðereumSignature) + if !reflect.DeepEqual(expectedError, err) { + t.Errorf("unexpected error\nexpected: [%v]\nactual: [%v]", expectedError, err) + } +} + +func TestVerify_WrongAddress(t *testing.T) { + ethereumSignature := validSignature + ethereumSignature.Address = common.HexToAddress("0x93df7c54c41A9D7FB17C1E8039d387a2A924708c") + + expectedError := fmt.Errorf("invalid signer\n\texpected: 0x93df7c54c41A9D7FB17C1E8039d387a2A924708c\n\tactual: 0x4BCFC3099F12C53D01Da46695CC8776be584b946") + + err := verify(ðereumSignature) + if !reflect.DeepEqual(expectedError, err) { + t.Errorf("unexpected error\nexpected: [%v]\nactual: [%v]", expectedError, err) + } +} + +func TestVerify_WrongMessage(t *testing.T) { + ethereumSignature := validSignature + ethereumSignature.Message = "notTheSignedMessage" + + expectedError := fmt.Errorf("invalid signer\n\texpected: 0x4BCFC3099F12C53D01Da46695CC8776be584b946\n\tactual: 0x19882d7da145A10d5AEEFEe217Fd87dE679b4bb1") + + err := verify(ðereumSignature) + if !reflect.DeepEqual(expectedError, err) { + t.Errorf("unexpected error\nexpected: [%v]\nactual: [%v]", expectedError, err) + } +} + +func TestVerify_WrongSignature(t *testing.T) { + ethereumSignature := validSignature + ethereumSignature.Signature = "0xc8be189ab0ee691de7019eaa3de58558b84775085d9a0840908343ac690e02ca3f6e3d2dc70025b9b214d96c30e38c41f818cccd6f06b7a81c4afd26cbe6d6d601" + + expectedError := fmt.Errorf("invalid signer\n\texpected: 0x4BCFC3099F12C53D01Da46695CC8776be584b946\n\tactual: 0xb560e6c746138528509de08B782E3144E031a6B1") + + err := verify(ðereumSignature) + if !reflect.DeepEqual(expectedError, err) { + t.Errorf("unexpected error\nexpected: [%v]\nactual: [%v]", expectedError, err) + } +} + +func TestVerify_WrongVersion(t *testing.T) { + ethereumSignature := validSignature + ethereumSignature.Version = 1 + + expectedError := fmt.Errorf("unsupported ethereum signature version\n\texpected: 2\n\tactual: 1") + + err := verify(ðereumSignature) + if !reflect.DeepEqual(expectedError, err) { + t.Errorf("unexpected error\nexpected: [%v]\nactual: [%v]", expectedError, err) + } +} From 529fc792f24554d46542beb61562b1deac530067 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Mon, 23 Nov 2020 11:32:41 +0100 Subject: [PATCH 2/3] Change Version field type to string Schema for signature in mycrypto assumes the Version field to be of type string, although int also should work. We want to be consistent and updated the field type to string. --- cmd/signing_ethereum.go | 8 ++++---- cmd/signing_ethereum_test.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/signing_ethereum.go b/cmd/signing_ethereum.go index 2e4bb9e9c..ccd55d27b 100644 --- a/cmd/signing_ethereum.go +++ b/cmd/signing_ethereum.go @@ -100,10 +100,10 @@ type EthereumSignature struct { Address common.Address `json:"address"` Message string `json:"msg"` Signature string `json:"sig"` - Version uint `json:"version"` + Version string `json:"version"` } -const ethSignatureVersion uint = 2 +const ethSignatureVersion = "2" // EthereumSign signs a string using operator's ethereum key. func EthereumSign(c *cli.Context) error { @@ -208,8 +208,8 @@ func verify(ethereumSignature *EthereumSignature) error { if ethereumSignature.Version != ethSignatureVersion { return fmt.Errorf( "unsupported ethereum signature version\n"+ - "\texpected: %d\n"+ - "\tactual: %d", + "\texpected: %s\n"+ + "\tactual: %s", ethSignatureVersion, ethereumSignature.Version, ) diff --git a/cmd/signing_ethereum_test.go b/cmd/signing_ethereum_test.go index 5a7ea4948..f332c6214 100644 --- a/cmd/signing_ethereum_test.go +++ b/cmd/signing_ethereum_test.go @@ -17,7 +17,7 @@ var validSignature = EthereumSignature{ Address: common.HexToAddress("0x4BCFC3099F12C53D01Da46695CC8776be584b946"), Message: "verySecretMessage", Signature: "0xc8be189ab0ee691de7019eaa3de58558b84775085d9a0840908343ac690e02ca3f6e3d2dc70025b9b214d96c30e38c41f818cccd6f06b7a81c4afd26cbe6d6d600", - Version: 2, + Version: "2", } func TestSign(t *testing.T) { @@ -29,7 +29,7 @@ func TestSign(t *testing.T) { Address: common.HexToAddress("0x4BCFC3099F12C53D01Da46695CC8776be584b946"), Message: message, Signature: "0xc8be189ab0ee691de7019eaa3de58558b84775085d9a0840908343ac690e02ca3f6e3d2dc70025b9b214d96c30e38c41f818cccd6f06b7a81c4afd26cbe6d6d600", - Version: 2, + Version: "2", } ethereumKey, err := ethutil.DecryptKeyFile(keyFilePath, keyFilePassword) @@ -116,7 +116,7 @@ func TestVerify_WrongSignature(t *testing.T) { func TestVerify_WrongVersion(t *testing.T) { ethereumSignature := validSignature - ethereumSignature.Version = 1 + ethereumSignature.Version = "1" expectedError := fmt.Errorf("unsupported ethereum signature version\n\texpected: 2\n\tactual: 1") From 83153ddd19e9127fbbd2b8e098eb3fd23d09a9cf Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Mon, 23 Nov 2020 11:39:28 +0100 Subject: [PATCH 3/3] Added keyfile used in tests --- internal/testdata/eth_key.json | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 internal/testdata/eth_key.json diff --git a/internal/testdata/eth_key.json b/internal/testdata/eth_key.json new file mode 100644 index 000000000..1e7d048bc --- /dev/null +++ b/internal/testdata/eth_key.json @@ -0,0 +1,21 @@ +{ + "address": "4bcfc3099f12c53d01da46695cc8776be584b946", + "crypto": { + "cipher": "aes-128-ctr", + "ciphertext": "904cb53d4e6c3e7dc6fb2443ad5db42b502e221b4144276ac150d51cea3cd638", + "cipherparams": { + "iv": "be8d7c7197c45d371cb6d97f9878636e" + }, + "kdf": "scrypt", + "kdfparams": { + "dklen": 32, + "n": 262144, + "p": 1, + "r": 8, + "salt": "b93d46f5ff9a9cf15a3546b05a0941d17e1cb9467e2484f5e1495a030616f258" + }, + "mac": "87bb79dbb0e0f056d8d2e91d7c7a7c18367a52fae5324b5c75423272d849860f" + }, + "id": "bffd9951-e141-41a3-9d63-82c8dd92c1d5", + "version": 3 +}