From 91f120c55dcdbfd15f99f0e9c4a017b5a131089e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 16 Aug 2018 14:04:19 -0700 Subject: [PATCH] signature verification not working --- handlers/ante.go | 28 +++++++++++++-- handlers/ante_test.go | 82 +++++++++++++++++++++++++++++++++++++++---- types/test_common.go | 10 ++++-- types/tx.go | 42 ++++++++++------------ types/tx_test.go | 23 +++++++++--- types/utils_test.go | 2 +- types/wire.go | 3 ++ 7 files changed, 148 insertions(+), 42 deletions(-) diff --git a/handlers/ante.go b/handlers/ante.go index a62be214..7b3b8ed5 100644 --- a/handlers/ante.go +++ b/handlers/ante.go @@ -32,9 +32,9 @@ type internalAnteHandler func( func AnteHandler(am auth.AccountMapper) sdk.AnteHandler { return func(sdkCtx sdk.Context, tx sdk.Tx) (newCtx sdk.Context, res sdk.Result, abort bool) { var ( - gasLimit int64 handler internalAnteHandler - ) + gasLimit int64 + ) switch tx := tx.(type) { case types.Transaction: @@ -47,8 +47,15 @@ func AnteHandler(am auth.AccountMapper) sdk.AnteHandler { return sdkCtx, sdk.ErrInternal(fmt.Sprintf("invalid transaction: %T", tx)).Result(), true } + /* + fmt.Println("handler begin") + fmt.Println(am.GetAccount(sdkCtx, tx.)) + fmt.Println("handler end")*/ + newCtx = sdkCtx.WithGasMeter(sdk.NewGasMeter(gasLimit)) + + // AnteHandlers must have their own defer/recover in order for the // BaseApp to know how much gas was used! This is because the GasMeter // is created in the AnteHandler, but if it panics the context won't be @@ -92,12 +99,27 @@ func handleEthTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk.Cont // validate signature sdkCtx.GasMeter().ConsumeGas(verifySigCost, "ante verify") - _, err := ethTx.VerifySig(chainID) + addr, err := ethTx.VerifySig(chainID) + + fmt.Println(addr) + fmt.Println(chainID) if err != nil { return sdkCtx, sdk.ErrUnauthorized("signature verification failed").Result(), true } + // validate AccountNonce (called Sequence in AccountMapper) + acc := am.GetAccount(sdkCtx, addr[:]) + seq := acc.GetSequence() + if ethTx.Data.AccountNonce != uint64(seq) { + return sdkCtx, sdk.ErrInvalidSequence(fmt.Sprintf("Wrong AccountNonce: expected %d", seq)).Result(), true + } + err = acc.SetSequence(seq + 1) + if err != nil { + panic(err) + } + am.SetAccount(sdkCtx, acc) + return sdkCtx, sdk.Result{GasWanted: int64(ethTx.Data.GasLimit)}, false } diff --git a/handlers/ante_test.go b/handlers/ante_test.go index 2cfc6285..37bb19db 100644 --- a/handlers/ante_test.go +++ b/handlers/ante_test.go @@ -1,6 +1,7 @@ package handlers import ( + "fmt" "testing" "math/big" "crypto/ecdsa" @@ -8,7 +9,10 @@ import ( "github.com/cosmos/ethermint/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" + stake "github.com/cosmos/cosmos-sdk/x/stake/types" + abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/libs/log" ethcmn "github.com/ethereum/go-ethereum/common" @@ -29,7 +33,7 @@ func TestBadSig(t *testing.T) { _, res, abort := handler(ctx, tx) require.True(t, abort, "Antehandler did not abort") - require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad tx") + require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log)) } @@ -47,15 +51,41 @@ func TestInsufficientGas(t *testing.T) { _, res, abort := handler(ctx, tx) require.True(t, abort, "Antehandler did not abort") - require.Equal(t, sdk.ABCICodeType(0x1000c), res.Code, "Result is OK on bad tx") + require.Equal(t, sdk.ABCICodeType(0x1000c), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log)) } -func TestWrongNonce(t *testing.T) { +func TestIncorrectNonce(t *testing.T) { tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] tx.Data.AccountNonce = 12 + ms, key := SetupMultiStore() + ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, log.NewNopLogger()) + + accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) + + // Set account in accountMapper + acc := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:]) + accountMapper.SetAccount(ctx, acc) + + handler := AnteHandler(accountMapper) + + fmt.Println("start test") + fmt.Println(accountMapper.GetAccount(ctx, types.TestAddr1[:])) + fmt.Println(types.TestAddr1) + fmt.Println("end test") + + _, res, abort := handler(ctx, tx) + + require.True(t, abort, "Antehandler did not abort") + require.Equal(t, sdk.ABCICodeType(0x10003), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log)) + +} + +func TestValidTx(t *testing.T) { + tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] + ms, key := SetupMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) @@ -64,7 +94,47 @@ func TestWrongNonce(t *testing.T) { _, res, abort := handler(ctx, tx) - require.True(t, abort, "Antehandler did not abort") - require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad tx") + require.False(t, abort, "Antehandler abort on valid tx") + require.Equal(t, sdk.CodeOK, res.Code, fmt.Sprintf("Result not OK on valid Tx: %s", res.Log)) -} \ No newline at end of file +} + +func TestValidEmbeddedTx(t *testing.T) { + cdc := types.NewTestCodec() + // Create msg to be embedded + msgs := []sdk.Msg{stake.NewMsgDelegate(types.TestAddr1[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)})} + + tx := types.NewTestSDKTxs(cdc, types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey1}, []int64{0}, []int64{0}, types.NewStdFee())[0] + + ms, key := SetupMultiStore() + ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + + accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) + handler := AnteHandler(accountMapper) + + _, res, abort := handler(ctx, tx) + + require.False(t, abort, "Antehandler abort on valid embedded tx") + require.Equal(t, sdk.CodeOK, res.Code, fmt.Sprintf("Result not OK on valid Tx: %s", res.Log)) +} + +/* +func TestInvalidEmbeddedTx(t *testing.T) { + cdc := types.NewTestCodec() + // Create msg to be embedded + msgs := []sdk.Msg{stake.NewMsgCreateValidator(types.TestAddr1[:], nil, sdk.Coin{"steak", sdk.NewInt(50)}, stake.Description{})} + + tx := types.NewTestSDKTxs(cdc, types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey1}, []int64{0}, []int64{0}, types.NewStdFee())[0] + + ms, key := SetupMultiStore() + ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + + accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) + handler := AnteHandler(accountMapper) + + _, res, abort := handler(ctx, tx) + + require.True(t, abort, "Antehandler did not abort on invalid embedded tx") + require.Equal(t, sdk.ABCICodeType(0x1000c), res.Code, "Result is OK on bad tx") +} +*/ \ No newline at end of file diff --git a/types/test_common.go b/types/test_common.go index ab2aa111..5c0664a4 100644 --- a/types/test_common.go +++ b/types/test_common.go @@ -7,6 +7,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" "github.com/cosmos/cosmos-sdk/x/auth" + stake "github.com/cosmos/cosmos-sdk/x/stake/types" ethcrypto "github.com/ethereum/go-ethereum/crypto" ethtypes "github.com/ethereum/go-ethereum/core/types" @@ -31,10 +32,13 @@ func NewTestCodec() *wire.Codec { RegisterWire(codec) codec.RegisterConcrete(&sdk.TestMsg{}, "test/TestMsg", nil) + // Register any desired SDK msgs to be embedded + stake.RegisterWire(codec) + return codec } -func newStdFee() auth.StdFee { +func NewStdFee() auth.StdFee { return auth.NewStdFee(5000, sdk.NewCoin("photon", 150)) } @@ -46,7 +50,7 @@ func newTestEmbeddedTx( sigs := make([][]byte, len(pKeys)) for i, priv := range pKeys { - signEtx := EmbeddedTxSign{chainID.String(), accNums[i], seqs[i], msgs, newStdFee()} + signEtx := EmbeddedTxSign{chainID.String(), accNums[i], seqs[i], msgs, fee} signBytes, err := signEtx.Bytes() if err != nil { @@ -86,7 +90,7 @@ func NewTestEthTxs(chainID sdk.Int, pKeys []*ecdsa.PrivateKey, addrs []ethcmn.Ad for i, priv := range pKeys { emintTx := NewTransaction( - uint64(i), addrs[i], sdk.NewInt(10), 100, sdk.NewInt(100), nil, + uint64(i), addrs[i], sdk.NewInt(10), 1000, sdk.NewInt(100), nil, ) emintTx.Sign(chainID, priv) diff --git a/types/tx.go b/types/tx.go index 873a8572..d29c2f0c 100644 --- a/types/tx.go +++ b/types/tx.go @@ -182,31 +182,16 @@ func (tx Transaction) VerifySig(chainID *big.Int) (ethcmn.Address, error) { } } - var signBytes ethcmn.Hash - if tx.Data.Signature.v.BitLen() < 8 { - v := tx.Data.Signature.v.Uint64() - if v == 27 || v == 28 { - // Unprotected tx has no cross-chain replay protection - signBytes = rlpHash([]interface{}{ - tx.Data.AccountNonce, - tx.Data.Price, - tx.Data.GasLimit, - tx.Data.Recipient, - tx.Data.Amount, - tx.Data.Payload, - }) - } - } else { - signBytes = rlpHash([]interface{}{ - tx.Data.AccountNonce, - tx.Data.Price, - tx.Data.GasLimit, - tx.Data.Recipient, - tx.Data.Amount, - tx.Data.Payload, - chainID, uint(0), uint(0), + + signBytes := rlpHash([]interface{}{ + tx.Data.AccountNonce, + tx.Data.Price, + tx.Data.GasLimit, + tx.Data.Recipient, + tx.Data.Amount, + tx.Data.Payload, + chainID, uint(0), uint(0), }) - } sig := recoverEthSig(tx.Data.Signature, chainID) @@ -279,6 +264,15 @@ func (tx Transaction) GetEmbeddedTx(codec *wire.Codec) (EmbeddedTx, sdk.Error) { return etx, nil } +// Copies Ethereum tx's Protected function +func (tx Transaction) protected() bool { + if tx.Data.Signature.v.BitLen() <= 8 { + v := tx.Data.Signature.v.Uint64() + return v != 27 && v != 28 + } + return true +} + // ---------------------------------------------------------------------------- // embedded SDK transaction // ---------------------------------------------------------------------------- diff --git a/types/tx_test.go b/types/tx_test.go index 7d3ac20e..b7a3dcb5 100644 --- a/types/tx_test.go +++ b/types/tx_test.go @@ -53,7 +53,7 @@ func TestHasEmbeddedTx(t *testing.T) { sdkTxs := NewTestSDKTxs( testCodec, TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) require.True(t, sdkTxs[0].HasEmbeddedTx(TestSDKAddress)) @@ -76,7 +76,7 @@ func TestGetEmbeddedTx(t *testing.T) { ) sdkTxs := NewTestSDKTxs( testCodec, TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) etx, err := sdkTxs[0].GetEmbeddedTx(testCodec) @@ -102,7 +102,7 @@ func TestTransactionGetMsgs(t *testing.T) { expectedMsgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} etx := newTestEmbeddedTx( TestChainID, expectedMsgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) msgs = etx.GetMsgs() @@ -114,13 +114,26 @@ func TestGetRequiredSigners(t *testing.T) { msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} etx := newTestEmbeddedTx( TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) signers := etx.(EmbeddedTx).GetRequiredSigners() require.Equal(t, []sdk.AccAddress{sdk.AccAddress(TestAddr1.Bytes())}, signers) } +func TestVerifySig(t *testing.T) { + ethTx := NewTestEthTxs( + TestChainID, + []*ecdsa.PrivateKey{TestPrivKey1}, + []ethcmn.Address{TestAddr1}, + )[0] + + addr, err := ethTx.VerifySig(TestChainID.BigInt()) + + require.Nil(t, err, "Sig verification failed") + require.Equal(t, TestAddr1, addr, "Address is not the same") +} + func TestTxDecoder(t *testing.T) { testCodec := NewTestCodec() txDecoder := TxDecoder(testCodec, TestSDKAddress) @@ -141,7 +154,7 @@ func TestTxDecoder(t *testing.T) { // create embedded transaction and encode etx := newTestEmbeddedTx( TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) payload := testCodec.MustMarshalBinary(etx) diff --git a/types/utils_test.go b/types/utils_test.go index 341b66db..430c5b03 100644 --- a/types/utils_test.go +++ b/types/utils_test.go @@ -13,7 +13,7 @@ func TestValidateSigner(t *testing.T) { msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} // create message signing structure - signEtx := EmbeddedTxSign{TestChainID.String(), 0, 0, msgs, newStdFee()} + signEtx := EmbeddedTxSign{TestChainID.String(), 0, 0, msgs, NewStdFee()} // create signing bytes and sign signBytes, err := signEtx.Bytes() diff --git a/types/wire.go b/types/wire.go index 5414fcdc..818b1742 100644 --- a/types/wire.go +++ b/types/wire.go @@ -3,6 +3,7 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" + "github.com/cosmos/cosmos-sdk/x/auth" ) var typesCodec = wire.NewCodec() @@ -15,6 +16,8 @@ func init() { // codec. func RegisterWire(codec *wire.Codec) { sdk.RegisterWire(codec) + wire.RegisterCrypto(codec) + auth.RegisterWire(codec) codec.RegisterConcrete(&EthSignature{}, "types/EthSignature", nil) codec.RegisterConcrete(TxData{}, "types/TxData", nil) codec.RegisterConcrete(Transaction{}, "types/Transaction", nil)