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.

Sign up for GitHub

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

Jump to bottom

feat: extra checks on signatures/pubkeys + check the signature first in antehandle #18194

Merged
merged 33 commits into from
Nov 7, 2023
Merged

feat: extra checks on signatures/pubkeys + check the signature first in antehandle #18194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
9069bb6
Renamed missleading variable name
bizk Oct 17, 2023
fd1600f
added onCurve verification
bizk Oct 17, 2023
f6150a8
added oncurve verification and tests
bizk Oct 20, 2023
f74ef04
removed multisig verification
bizk Oct 20, 2023
767e1d1
lint fix
bizk Oct 20, 2023
fd5a041
Merge branch 'main' of github.com:Zondax/cosmos-sdk into feature/ante…
bizk Oct 23, 2023
68ef7eb
added back sequence check
bizk Oct 23, 2023
eda82b0
Merge branch 'main' into feature/anteKeysChecks
bizk Oct 23, 2023
4715f21
fixed some tests and improved error handling
bizk Oct 24, 2023
78d083a
Merge branch 'feature/anteKeysChecks' of github.com:Zondax/cosmos-sdk…
bizk Oct 24, 2023
b4edd4a
lint-fix
bizk Oct 24, 2023
f9ba818
added multisig handling
bizk Oct 24, 2023
e4af4f0
lint fix
bizk Oct 24, 2023
f5fa7cd
Merge branch 'main' into feature/anteKeysChecks
bizk Oct 24, 2023
dcf7704
lint fix
bizk Oct 24, 2023
b72057b
lint fix
bizk Oct 24, 2023
078b642
Merge branch 'main' into feature/anteKeysChecks
bizk Oct 24, 2023
6393a94
Merge branch 'feature/anteKeysChecks' of github.com:Zondax/cosmos-sdk…
bizk Oct 24, 2023
fe9f412
linter fixes
bizk Oct 24, 2023
62c7085
added t.helper in the respective functions
bizk Oct 25, 2023
9c460f2
run gci write on testutil
bizk Oct 25, 2023
e9f9f4f
run gci write on testutil
bizk Oct 25, 2023
61f1d96
Merge branch 'main' into feature/anteKeysChecks
bizk Oct 25, 2023
277c69c
added coment on test
bizk Oct 25, 2023
d2642bd
Merge branch 'feature/anteKeysChecks' of github.com:Zondax/cosmos-sdk…
bizk Oct 25, 2023
46dbafb
Merge branch 'main' into feature/anteKeysChecks
bizk Oct 30, 2023
4761682
Merge branch 'main' into feature/anteKeysChecks
bizk Nov 2, 2023
5dcd350
Fixed comments
bizk Nov 2, 2023
b79aa2f
added changelog.md
bizk Nov 2, 2023
69f65e0
lint fix
bizk Nov 2, 2023
12a79af
small fix on test
bizk Nov 3, 2023
6740dea
Merge branch 'main' into feature/anteKeysChecks
bizk Nov 3, 2023
7729e00
Merge branch 'feature/anteKeysChecks' of github.com:Zondax/cosmos-sdk…
bizk Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we get more information on this, this is vague

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want me to change the changelog or just leave a comment in this PR?


### Bug Fixes

* (client/server) [#18345](https://github.com/cosmos/cosmos-sdk/pull/18345) Consistently set viper prefix in client and server. It defaults for the binary name for both client and server.
Expand Down
9 changes: 5 additions & 4 deletions crypto/keys/secp256k1/secp256k1.go
View file
Open in desktop
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
Open in desktop
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