From c94bcb4841e1f02e46121903cb539f4f1c4719f6 Mon Sep 17 00:00:00 2001 From: Cameron Schultz Date: Wed, 1 May 2024 13:53:26 -0400 Subject: [PATCH 1/4] EIP-2 cap s-value --- vms/evm/signer/kms_signer.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/vms/evm/signer/kms_signer.go b/vms/evm/signer/kms_signer.go index d8f9ab08..9a95378d 100644 --- a/vms/evm/signer/kms_signer.go +++ b/vms/evm/signer/kms_signer.go @@ -109,11 +109,11 @@ func (s *KMSSigner) SignTx(tx *types.Transaction, evmChainID *big.Int) (*types.T if err != nil { return nil, err } - - sigBytes, err := s.getEthereumSignature(h, sigAsn1.R.Bytes, sigAsn1.S.Bytes) + sigBytes, err := s.recoverEIP155Signature(h, sigAsn1.R.Bytes, sigAsn1.S.Bytes) if err != nil { return nil, err } + return tx.WithSignature(signer, sigBytes) } @@ -121,10 +121,19 @@ func (s *KMSSigner) Address() common.Address { return s.eoa } -// Recover the EIP-155 signature from the KMS signature +// Recover the EIP-155 signature from the KMS signature. // KMS returns the signature in ASN.1 format, but the EIP-155 signature is in R || S || V format, -// so we need to test both V = 0 and V = 1 against the recovered public key -func (s *KMSSigner) getEthereumSignature(txHash []byte, rBytes []byte, sBytes []byte) ([]byte, error) { +// so we need to test both V = 0 and V = 1 against the recovered public key. +// Additionally, with EIP-2 S-values are capped at secp256k1n/2, so adjust that if necessary. +func (s *KMSSigner) recoverEIP155Signature(txHash []byte, rBytes []byte, sBytes []byte) ([]byte, error) { + sBigInt := new(big.Int).SetBytes(sBytes) + secp256k1N := crypto.S256().Params().N + secp256k1HalfN := new(big.Int).Div(secp256k1N, big.NewInt(2)) + + if sBigInt.Cmp(secp256k1HalfN) > 0 { + sBytes = new(big.Int).Sub(secp256k1N, sBigInt).Bytes() + } + rsSignature := append(adjustSignatureLength(rBytes), adjustSignatureLength(sBytes)...) if len(rsSignature) != signatureLength { return nil, errors.New("rs signature length is not 64 bytes") From cf2216f91f1e337d2c106e5e2861c1dbf9e4d492 Mon Sep 17 00:00:00 2001 From: Cameron Schultz Date: Wed, 1 May 2024 14:26:00 -0400 Subject: [PATCH 2/4] recover signature tests --- vms/evm/signer/kms_signer_test.go | 74 +++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 vms/evm/signer/kms_signer_test.go diff --git a/vms/evm/signer/kms_signer_test.go b/vms/evm/signer/kms_signer_test.go new file mode 100644 index 00000000..22ccfbf7 --- /dev/null +++ b/vms/evm/signer/kms_signer_test.go @@ -0,0 +1,74 @@ +package signer + +import ( + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/assert" +) + +func TestRecoverEIP155Signature(t *testing.T) { + testCases := []struct { + name string + txHash string + rValue string + sValue string + pubKey string + expectedSignature string + expectedError bool + }{ + { + name: "valid signature", + txHash: "69963cf4839e149fea0e7b0969dd6834ea4b22fa7f9209a46683982320e5edfd", + rValue: "00a94bfca53b42454acc43e7328ce7a8f244a629026095c36c0ee82607377cbee4", + sValue: "5964b0dd9bf23723570b6a073cb945fd1ba853ebf5579c8d11bb4fdb305cd079", + pubKey: "04a3b664aa1f37bf6c46a3f2cdb209091e16070208b30244838e2cb9fe1465c4911ba2ab0c54bfc39972740ef7b24904f8740b0a69aca2bbfce0f2829429b9a5c5", + expectedSignature: "a94bfca53b42454acc43e7328ce7a8f244a629026095c36c0ee82607377cbee45964b0dd9bf23723570b6a073cb945fd1ba853ebf5579c8d11bb4fdb305cd07901", + expectedError: false, + }, + { + name: "invalid r value", + txHash: "69963cf4839e149fea0e7b0969dd6834ea4b22fa7f9209a46683982320e5edfd", + rValue: "10a94bfca53b42454acc43e7328ce7a8f244a629026095c36c0ee82607377cbee4", + sValue: "5964b0dd9bf23723570b6a073cb945fd1ba853ebf5579c8d11bb4fdb305cd079", + pubKey: "04a3b664aa1f37bf6c46a3f2cdb209091e16070208b30244838e2cb9fe1465c4911ba2ab0c54bfc39972740ef7b24904f8740b0a69aca2bbfce0f2829429b9a5c5", + expectedSignature: "a94bfca53b42454acc43e7328ce7a8f244a629026095c36c0ee82607377cbee45964b0dd9bf23723570b6a073cb945fd1ba853ebf5579c8d11bb4fdb305cd07901", + expectedError: true, + }, + { + name: "invalid s value", + txHash: "69963cf4839e149fea0e7b0969dd6834ea4b22fa7f9209a46683982320e5edfd", + rValue: "00a94bfca53b42454acc43e7328ce7a8f244a629026095c36c0ee82607377cbee4", + sValue: "1964b0dd9bf23723570b6a073cb945fd1ba853ebf5579c8d11bb4fdb305cd079", + pubKey: "04a3b664aa1f37bf6c46a3f2cdb209091e16070208b30244838e2cb9fe1465c4911ba2ab0c54bfc39972740ef7b24904f8740b0a69aca2bbfce0f2829429b9a5c5", + expectedSignature: "a94bfca53b42454acc43e7328ce7a8f244a629026095c36c0ee82607377cbee45964b0dd9bf23723570b6a073cb945fd1ba853ebf5579c8d11bb4fdb305cd07901", + expectedError: true, + }, + { + name: "s-value too high", + txHash: "e29dc6b15950a5433e239155a2208156ba8fd81eeb81ade45329d4b2fb2e8421", + rValue: "00d993636ed097bf04b7c982e0394d572ead3c8f23ecc8baba0bbdfaa91aba4376", + sValue: "00d1ac23b517ae2569522626096fa1922681c87612cceac971e855d42103fea7c6", // This is > secp256k1n/2 + pubKey: "04a3b664aa1f37bf6c46a3f2cdb209091e16070208b30244838e2cb9fe1465c4911ba2ab0c54bfc39972740ef7b24904f8740b0a69aca2bbfce0f2829429b9a5c5", + expectedSignature: "d993636ed097bf04b7c982e0394d572ead3c8f23ecc8baba0bbdfaa91aba43762e53dc4ae851da96add9d9f6905e6dd838e666d3e25dd6c9d77c8a6bcc37997b00", + expectedError: false, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + signer := &KMSSigner{ + pubKey: common.Hex2Bytes(testCase.pubKey), + } + txHash := common.Hex2Bytes(testCase.txHash) + rValue := common.Hex2Bytes(testCase.rValue) + sValue := common.Hex2Bytes(testCase.sValue) + signature, err := signer.recoverEIP155Signature(txHash, rValue, sValue) + if testCase.expectedError { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + assert.Equal(t, common.Hex2Bytes(testCase.expectedSignature), signature) + } + }) + } +} From ca829077c1c74d66dab4bf2beb33ec6529913834 Mon Sep 17 00:00:00 2001 From: Cameron Schultz Date: Wed, 1 May 2024 14:26:40 -0400 Subject: [PATCH 3/4] use bigint ctor --- vms/evm/signer/kms_signer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vms/evm/signer/kms_signer.go b/vms/evm/signer/kms_signer.go index 9a95378d..02eb37fd 100644 --- a/vms/evm/signer/kms_signer.go +++ b/vms/evm/signer/kms_signer.go @@ -126,12 +126,12 @@ func (s *KMSSigner) Address() common.Address { // so we need to test both V = 0 and V = 1 against the recovered public key. // Additionally, with EIP-2 S-values are capped at secp256k1n/2, so adjust that if necessary. func (s *KMSSigner) recoverEIP155Signature(txHash []byte, rBytes []byte, sBytes []byte) ([]byte, error) { - sBigInt := new(big.Int).SetBytes(sBytes) + sBigInt := big.NewInt(0).SetBytes(sBytes) secp256k1N := crypto.S256().Params().N - secp256k1HalfN := new(big.Int).Div(secp256k1N, big.NewInt(2)) + secp256k1HalfN := big.NewInt(0).Div(secp256k1N, big.NewInt(2)) if sBigInt.Cmp(secp256k1HalfN) > 0 { - sBytes = new(big.Int).Sub(secp256k1N, sBigInt).Bytes() + sBytes = big.NewInt(0).Sub(secp256k1N, sBigInt).Bytes() } rsSignature := append(adjustSignatureLength(rBytes), adjustSignatureLength(sBytes)...) From 0b94adcba790b7b161013fa882721fa3e0873b9c Mon Sep 17 00:00:00 2001 From: cam-schultz <78878559+cam-schultz@users.noreply.github.com> Date: Wed, 1 May 2024 14:28:08 -0400 Subject: [PATCH 4/4] Update vms/evm/signer/kms_signer.go Co-authored-by: minghinmatthewlam Signed-off-by: cam-schultz <78878559+cam-schultz@users.noreply.github.com> --- vms/evm/signer/kms_signer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vms/evm/signer/kms_signer.go b/vms/evm/signer/kms_signer.go index 02eb37fd..dc88687c 100644 --- a/vms/evm/signer/kms_signer.go +++ b/vms/evm/signer/kms_signer.go @@ -124,7 +124,7 @@ func (s *KMSSigner) Address() common.Address { // Recover the EIP-155 signature from the KMS signature. // KMS returns the signature in ASN.1 format, but the EIP-155 signature is in R || S || V format, // so we need to test both V = 0 and V = 1 against the recovered public key. -// Additionally, with EIP-2 S-values are capped at secp256k1n/2, so adjust that if necessary. +// Additionally, EIP-2 S-values are capped at secp256k1n/2, so adjust that if necessary. func (s *KMSSigner) recoverEIP155Signature(txHash []byte, rBytes []byte, sBytes []byte) ([]byte, error) { sBigInt := big.NewInt(0).SetBytes(sBytes) secp256k1N := crypto.S256().Params().N