diff --git a/crypto/keys/secp256k1/secp256k1_nocgo.go b/crypto/keys/secp256k1/secp256k1_nocgo.go index 60fd9577978c..5cde04e82554 100644 --- a/crypto/keys/secp256k1/secp256k1_nocgo.go +++ b/crypto/keys/secp256k1/secp256k1_nocgo.go @@ -9,12 +9,13 @@ import ( secp256k1 "github.com/btcsuite/btcd/btcec" "github.com/tendermint/tendermint/crypto" + "golang.org/x/crypto/sha3" ) // used to reject malleable signatures // see: -// - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/signature_nocgo.go#L90-L93 -// - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/crypto.go#L39 +// - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/signature_nocgo.go#L90-L93 +// - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/crypto.go#L39 var secp256k1halfN = new(big.Int).Rsh(secp256k1.S256().N, 1) // Sign creates an ECDSA signature on curve Secp256k1, using SHA256 on the msg. @@ -49,6 +50,33 @@ func (pubKey *PubKey) VerifySignature(msg []byte, sigStr []byte) bool { return signature.Verify(crypto.Sha256(msg), pub) } +// VerifyBytes verifies a signature of the form R || S. +// It rejects signatures which are not in lower-S form. +func (pubKey *PubKey) VerifySignatureEip191(msg []byte, sigStr []byte) bool { + if len(sigStr) != 64 { + return false + } + pub, err := secp256k1.ParsePubKey(pubKey.Key, secp256k1.S256()) + if err != nil { + return false + } + // parse the signature: + signature := signatureFromBytes(sigStr) + // Reject malleable signatures. libsecp256k1 does this check but btcec doesn't. + // see: https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/signature_nocgo.go#L90-L93 + if signature.S.Cmp(secp256k1halfN) > 0 { + return false + } + + return signature.Verify(keccak256(msg), pub) +} + +func keccak256(bytes []byte) []byte { + hasher := sha3.NewLegacyKeccak256() + hasher.Write(bytes) + return hasher.Sum(nil) +} + // Read Signature struct from R || S. Caller needs to ensure // that len(sigStr) == 64. func signatureFromBytes(sigStr []byte) *secp256k1.Signature { diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index fdd13ac9aa67..f05ac2ce4a4c 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -289,9 +289,9 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul if OnlyLegacyAminoSigners(sig.Data) { // If all signers are using SIGN_MODE_LEGACY_AMINO, we rely on VerifySignature to check account sequence number, // and therefore communicate sequence number as a potential cause of error. - errMsg = fmt.Sprintf("signature verification failed; please verify account number (%d), sequence (%d) and chain-id (%s)", accNum, acc.GetSequence(), chainID) + errMsg = fmt.Sprintf("signature verification failed; please verify account number (%d), sequence (%d) and chain-id (%s): %s", accNum, acc.GetSequence(), chainID, err.Error()) } else { - errMsg = fmt.Sprintf("signature verification failed; please verify account number (%d) and chain-id (%s)", accNum, chainID) + errMsg = fmt.Sprintf("signature verification failed; please verify account number (%d) and chain-id (%s): %s", accNum, chainID, err.Error()) } return ctx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, errMsg) diff --git a/x/auth/signing/verify.go b/x/auth/signing/verify.go index 5a5395de69f3..6d3a6a60902c 100644 --- a/x/auth/signing/verify.go +++ b/x/auth/signing/verify.go @@ -1,8 +1,10 @@ package signing import ( + "encoding/hex" "fmt" + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" @@ -18,8 +20,23 @@ func VerifySignature(pubKey cryptotypes.PubKey, signerData SignerData, sigData s if err != nil { return err } - if !pubKey.VerifySignature(signBytes, data.Signature) { - return fmt.Errorf("unable to verify single signer signature") + + if data.SignMode == signing.SignMode_SIGN_MODE_EIP_191 { + // do this to not have to register a new type of pubkey like here: + // https://github.com/scrtlabs/cosmos-sdk/blob/07817ad365/crypto/keys/secp256k1/keys.pb.go#L120 + + secp256k1PubKey, ok := pubKey.(*secp256k1.PubKey) + if !ok { + return fmt.Errorf("eip191 sign mode requires pubkey to be of type cosmos.crypto.secp256k1.PubKey") + } + + if !secp256k1PubKey.VerifySignatureEip191(signBytes, data.Signature) { + return fmt.Errorf("unable to verify single signer eip191 signature %s for signBytes %s", hex.EncodeToString(data.Signature), hex.EncodeToString(signBytes)) + } + } else { + if !pubKey.VerifySignature(signBytes, data.Signature) { + return fmt.Errorf("unable to verify single signer signature %s for signBytes %s", hex.EncodeToString(data.Signature), hex.EncodeToString(signBytes)) + } } return nil diff --git a/x/auth/tx/eip191.go b/x/auth/tx/eip191.go new file mode 100644 index 000000000000..d21cd13aee6b --- /dev/null +++ b/x/auth/tx/eip191.go @@ -0,0 +1,66 @@ +package tx + +import ( + "fmt" + "strconv" + + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" + "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" + "github.com/cosmos/cosmos-sdk/x/auth/signing" +) + +const EIP191MessagePrefix = "\x19Ethereum Signed Message:\n" + +const eip191NonCriticalFieldsError = "protobuf transaction contains unknown non-critical fields. This is a transaction malleability issue and SIGN_MODE_EIP_191 cannot be used." + +var _ signing.SignModeHandler = signModeEIP191Handler{} + +// signModeEIP191Handler defines the SIGN_MODE_EIP191 +// SignModeHandler. +type signModeEIP191Handler struct{} + +func (s signModeEIP191Handler) DefaultMode() signingtypes.SignMode { + return signingtypes.SignMode_SIGN_MODE_EIP_191 +} + +func (s signModeEIP191Handler) Modes() []signingtypes.SignMode { + return []signingtypes.SignMode{signingtypes.SignMode_SIGN_MODE_EIP_191} +} + +func (s signModeEIP191Handler) GetSignBytes(mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx) ([]byte, error) { + if mode != signingtypes.SignMode_SIGN_MODE_EIP_191 { + return nil, fmt.Errorf("expected %s, got %s", signingtypes.SignMode_SIGN_MODE_EIP_191, mode) + } + + protoTx, ok := tx.(*wrapper) + if !ok { + return nil, fmt.Errorf("can only handle a protobuf Tx, got %T", tx) + } + + if protoTx.txBodyHasUnknownNonCriticals { + return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, eip191NonCriticalFieldsError) + } + + body := protoTx.tx.Body + + if len(body.ExtensionOptions) != 0 || len(body.NonCriticalExtensionOptions) != 0 { + return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "SignMode_SIGN_MODE_EIP_191 does not support protobuf extension options.") + } + + aminoJSONBz := legacytx.StdSignBytes( + data.ChainID, data.AccountNumber, data.Sequence, protoTx.GetTimeoutHeight(), + legacytx.StdFee{Amount: protoTx.GetFee(), Gas: protoTx.GetGas()}, + tx.GetMsgs(), protoTx.GetMemo(), + ) + + bz := append( + []byte(EIP191MessagePrefix), + []byte(strconv.Itoa(len(aminoJSONBz)))..., + ) + + bz = append(bz, aminoJSONBz...) + + return bz, nil +} diff --git a/x/auth/tx/mode_handler.go b/x/auth/tx/mode_handler.go index f49ee16198d7..f56fa57692e4 100644 --- a/x/auth/tx/mode_handler.go +++ b/x/auth/tx/mode_handler.go @@ -11,10 +11,11 @@ import ( var DefaultSignModes = []signingtypes.SignMode{ signingtypes.SignMode_SIGN_MODE_DIRECT, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + signingtypes.SignMode_SIGN_MODE_EIP_191, } // makeSignModeHandler returns the default protobuf SignModeHandler supporting -// SIGN_MODE_DIRECT and SIGN_MODE_LEGACY_AMINO_JSON. +// SIGN_MODE_DIRECT, SIGN_MODE_LEGACY_AMINO_JSON, and SIGN_MODE_SIGN_MODE_EIP_191. func makeSignModeHandler(modes []signingtypes.SignMode) signing.SignModeHandler { if len(modes) < 1 { panic(fmt.Errorf("no sign modes enabled")) @@ -28,6 +29,8 @@ func makeSignModeHandler(modes []signingtypes.SignMode) signing.SignModeHandler handlers[i] = signModeDirectHandler{} case signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON: handlers[i] = signModeLegacyAminoJSONHandler{} + case signingtypes.SignMode_SIGN_MODE_EIP_191: + handlers[i] = signModeEIP191Handler{} default: panic(fmt.Errorf("unsupported sign mode %+v", mode)) }