Skip to content

Commit

Permalink
feat: extra checks on signatures/pubkeys + check the signature first …
Browse files Browse the repository at this point in the history 
…in antehandle (#18194)
  • Loading branch information
@bizk
bizk authored Nov 7, 2023
1 parent 5d83f92 commit 346044a
Show file tree
Hide file tree
Showing 20 changed files with 295 additions and 105 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [#17733](https://github.com/cosmos/cosmos-sdk/pull/17733) Ensure `buf export` exports all proto dependencies
* (version) [#18063](https://github.com/cosmos/cosmos-sdk/pull/18063) Include additional information in the Info struct. This change enhances the Info struct by adding support for additional information through the ExtraInfo field
* [#18204](https://github.com/cosmos/cosmos-sdk/pull/18204) Use streaming json parser to parse chain-id from genesis file.

* (crypto | x/auth) [#14372](https://github.com/cosmos/cosmos-sdk/pull/18194) Key checks on signatures antehandle

### Bug Fixes

* (baseapp) [#18383](https://github.com/cosmos/cosmos-sdk/pull/18383) Fixed a data race inside BaseApp.getContext, found by end-to-end (e2e) tests.
Expand Down
9 changes: 5 additions & 4 deletions crypto/keys/secp256k1/secp256k1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"math/big"

"github.com/cometbft/cometbft/crypto"
secp256k1 "github.com/decred/dcrd/dcrec/secp256k1/v4"
secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4"
"golang.org/x/crypto/ripemd160" //nolint: staticcheck // keep around for backwards compatibility

errorsmod "cosmossdk.io/errors"
Expand Down Expand Up @@ -39,7 +39,8 @@ func (privKey *PrivKey) Bytes() []byte {
// PubKey performs the point-scalar multiplication from the privKey on the
// generator point to get the pubkey.
func (privKey *PrivKey) PubKey() cryptotypes.PubKey {
pubkeyObject := secp256k1.PrivKeyFromBytes(privKey.Key).PubKey()
pubkeyObject := secp256k1dcrd.PrivKeyFromBytes(privKey.Key).PubKey()

pk := pubkeyObject.SerializeCompressed()
return &PubKey{Key: pk}
}
Expand Down Expand Up @@ -100,7 +101,7 @@ func genPrivKey(rand io.Reader) []byte {

d.SetBytes(privKeyBytes[:])
// break if we found a valid point (i.e. > 0 and < N == curverOrder)
isValidFieldElement := 0 < d.Sign() && d.Cmp(secp256k1.S256().N) < 0
isValidFieldElement := 0 < d.Sign() && d.Cmp(secp256k1dcrd.S256().N) < 0
if isValidFieldElement {
break
}
Expand Down Expand Up @@ -128,7 +129,7 @@ func GenPrivKeyFromSecret(secret []byte) *PrivKey {
// https://apps.nsa.gov/iaarchive/library/ia-guidance/ia-solutions-for-classified/algorithm-guidance/suite-b-implementers-guide-to-fips-186-3-ecdsa.cfm
// see also https://github.com/golang/go/blob/0380c9ad38843d523d9c9804fe300cb7edd7cd3c/src/crypto/ecdsa/ecdsa.go#L89-L101
fe := new(big.Int).SetBytes(secHash[:])
n := new(big.Int).Sub(secp256k1.S256().N, one)
n := new(big.Int).Sub(secp256k1dcrd.S256().N, one)
fe.Mod(fe, n)
fe.Add(fe, one)

Expand Down
40 changes: 20 additions & 20 deletions tests/integration/bank/keeper/deterministic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,12 @@ func TestGRPCQueryBalance(t *testing.T) {

req := banktypes.NewQueryBalanceRequest(addr, coin.GetDenom())

testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Balance, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Balance, 0, true)
})

fundAccount(f, addr1, coin1)
req := banktypes.NewQueryBalanceRequest(addr1, coin1.GetDenom())
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Balance, 1087, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Balance, 1087, false)
}

func TestGRPCQueryAllBalances(t *testing.T) {
Expand All @@ -174,7 +174,7 @@ func TestGRPCQueryAllBalances(t *testing.T) {
fundAccount(f, addr, coins...)

req := banktypes.NewQueryAllBalancesRequest(addr, testdata.PaginationGenerator(rt, uint64(numCoins)).Draw(rt, "pagination"), false)
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.AllBalances, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.AllBalances, 0, true)
})

coins := sdk.NewCoins(
Expand All @@ -185,7 +185,7 @@ func TestGRPCQueryAllBalances(t *testing.T) {
fundAccount(f, addr1, coins...)
req := banktypes.NewQueryAllBalancesRequest(addr1, nil, false)

testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.AllBalances, 357, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.AllBalances, 357, false)
}

func TestGRPCQuerySpendableBalances(t *testing.T) {
Expand All @@ -212,7 +212,7 @@ func TestGRPCQuerySpendableBalances(t *testing.T) {
assert.NilError(t, err)

req := banktypes.NewQuerySpendableBalancesRequest(addr, testdata.PaginationGenerator(rt, uint64(len(denoms))).Draw(rt, "pagination"))
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SpendableBalances, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SpendableBalances, 0, true)
})

coins := sdk.NewCoins(
Expand All @@ -224,7 +224,7 @@ func TestGRPCQuerySpendableBalances(t *testing.T) {
assert.NilError(t, err)

req := banktypes.NewQuerySpendableBalancesRequest(addr1, nil)
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SpendableBalances, 2032, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SpendableBalances, 2032, false)
}

func TestGRPCQueryTotalSupply(t *testing.T) {
Expand Down Expand Up @@ -256,7 +256,7 @@ func TestGRPCQueryTotalSupply(t *testing.T) {
Pagination: testdata.PaginationGenerator(rt, uint64(len(initialSupply))).Draw(rt, "pagination"),
}

testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.TotalSupply, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.TotalSupply, 0, true)
})

f = initDeterministicFixture(t) // reset
Expand All @@ -269,7 +269,7 @@ func TestGRPCQueryTotalSupply(t *testing.T) {
assert.NilError(t, f.bankKeeper.MintCoins(f.ctx, minttypes.ModuleName, coins))

req := &banktypes.QueryTotalSupplyRequest{}
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.TotalSupply, 150, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.TotalSupply, 150, false)
}

func TestGRPCQueryTotalSupplyOf(t *testing.T) {
Expand All @@ -285,14 +285,14 @@ func TestGRPCQueryTotalSupplyOf(t *testing.T) {
assert.NilError(t, f.bankKeeper.MintCoins(f.ctx, minttypes.ModuleName, sdk.NewCoins(coin)))

req := &banktypes.QuerySupplyOfRequest{Denom: coin.GetDenom()}
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SupplyOf, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SupplyOf, 0, true)
})

coin := sdk.NewCoin("bar", math.NewInt(100))

assert.NilError(t, f.bankKeeper.MintCoins(f.ctx, minttypes.ModuleName, sdk.NewCoins(coin)))
req := &banktypes.QuerySupplyOfRequest{Denom: coin.GetDenom()}
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SupplyOf, 1021, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SupplyOf, 1021, false)
}

func TestGRPCQueryParams(t *testing.T) {
Expand All @@ -314,7 +314,7 @@ func TestGRPCQueryParams(t *testing.T) {
assert.NilError(t, err)

req := &banktypes.QueryParamsRequest{}
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Params, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Params, 0, true)
})

enabledStatus := banktypes.SendEnabled{
Expand All @@ -330,7 +330,7 @@ func TestGRPCQueryParams(t *testing.T) {
err := f.bankKeeper.SetParams(f.ctx, params)
assert.NilError(t, err)
req := &banktypes.QueryParamsRequest{}
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Params, 1003, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Params, 1003, false)
}

func createAndReturnMetadatas(t *rapid.T, count int) []banktypes.Metadata {
Expand Down Expand Up @@ -385,15 +385,15 @@ func TestGRPCDenomsMetadata(t *testing.T) {
Pagination: testdata.PaginationGenerator(rt, uint64(count)).Draw(rt, "pagination"),
}

testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomsMetadata, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomsMetadata, 0, true)
})

f = initDeterministicFixture(t) // reset

f.bankKeeper.SetDenomMetaData(f.ctx, metadataAtom)

req := &banktypes.QueryDenomsMetadataRequest{}
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomsMetadata, 660, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomsMetadata, 660, false)
}

func TestGRPCDenomMetadata(t *testing.T) {
Expand All @@ -409,7 +409,7 @@ func TestGRPCDenomMetadata(t *testing.T) {
Denom: denomMetadata[0].Base,
}

testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomMetadata, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomMetadata, 0, true)
})

f.bankKeeper.SetDenomMetaData(f.ctx, metadataAtom)
Expand All @@ -418,7 +418,7 @@ func TestGRPCDenomMetadata(t *testing.T) {
Denom: metadataAtom.Base,
}

testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomMetadata, 1300, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomMetadata, 1300, false)
}

func TestGRPCSendEnabled(t *testing.T) {
Expand Down Expand Up @@ -448,7 +448,7 @@ func TestGRPCSendEnabled(t *testing.T) {
// Pagination is only taken into account when `denoms` is an empty array
Pagination: testdata.PaginationGenerator(rt, uint64(len(allDenoms))).Draw(rt, "pagination"),
}
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SendEnabled, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SendEnabled, 0, true)
})

coin1 := banktypes.SendEnabled{
Expand All @@ -467,7 +467,7 @@ func TestGRPCSendEnabled(t *testing.T) {
Denoms: []string{coin1.GetDenom(), coin2.GetDenom()},
}

testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SendEnabled, 4063, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SendEnabled, 4063, false)
}

func TestGRPCDenomOwners(t *testing.T) {
Expand All @@ -493,7 +493,7 @@ func TestGRPCDenomOwners(t *testing.T) {
Denom: denom,
Pagination: testdata.PaginationGenerator(rt, uint64(numAddr)).Draw(rt, "pagination"),
}
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomOwners, 0, true)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomOwners, 0, true)
})

denomOwners := []*banktypes.DenomOwner{
Expand All @@ -518,5 +518,5 @@ func TestGRPCDenomOwners(t *testing.T) {
req := &banktypes.QueryDenomOwnersRequest{
Denom: coin1.GetDenom(),
}
testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomOwners, 2516, false)
testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomOwners, 2516, false)
}
Loading

0 comments on commit 346044a

Please sign in to comment.