diff --git a/CHANGELOG.md b/CHANGELOG.md index 8499de717..972723cbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ and provided directly the IAVL store. * (modules) [\#5572](https://github.com/cosmos/cosmos-sdk/pull/5572) Move account balance logic and APIs from `x/auth` to `x/bank`. * (types) [\#5533](https://github.com/cosmos/cosmos-sdk/pull/5533) Refactored `AppModuleBasic` and `AppModuleGenesis` to now accept a `codec.JSONMarshaler` for modular serialization of genesis state. +* (crypto/keys) [\#5735](https://github.com/cosmos/cosmos-sdk/pull/5735) Keyring's Update() function is now no-op. ### Features diff --git a/client/keys/show.go b/client/keys/show.go index 539fb111e..c959ff157 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -136,7 +136,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) { return nil } - return crypto.LedgerShowAddress(*hdpath, info.GetPubKey()) + return crypto.LedgerShowAddress(*hdpath, info.GetPubKey(), sdk.GetConfig().GetBech32AccountAddrPrefix()) } return nil diff --git a/crypto/keys/keybase.go b/crypto/keys/keybase.go index 9d95a5ccd..b55c7b68b 100644 --- a/crypto/keys/keybase.go +++ b/crypto/keys/keybase.go @@ -212,7 +212,7 @@ func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig []byte, pub t } case ledgerInfo: - return kb.base.SignWithLedger(info, msg) + return SignWithLedger(info, msg) case offlineInfo, multiInfo: return kb.base.DecodeSignature(info, msg) diff --git a/crypto/keys/keybase_base.go b/crypto/keys/keybase_base.go index 3003b0882..47c7e60d7 100644 --- a/crypto/keys/keybase_base.go +++ b/crypto/keys/keybase_base.go @@ -104,24 +104,6 @@ func SecpPrivKeyGen(bz []byte) tmcrypto.PrivKey { return secp256k1.PrivKeySecp256k1(bzArr) } -// SignWithLedger signs a binary message with the ledger device referenced by an Info object -// and returns the signed bytes and the public key. It returns an error if the device could -// not be queried or it returned an error. -func (kb baseKeybase) SignWithLedger(info Info, msg []byte) (sig []byte, pub tmcrypto.PubKey, err error) { - i := info.(ledgerInfo) - priv, err := crypto.NewPrivKeyLedgerSecp256k1Unsafe(i.Path) - if err != nil { - return - } - - sig, err = priv.Sign(msg) - if err != nil { - return nil, nil, err - } - - return sig, priv.PubKey(), nil -} - // DecodeSignature decodes a an length-prefixed binary signature from standard input // and return it as a byte slice. func (kb baseKeybase) DecodeSignature(info Info, msg []byte) (sig []byte, pub tmcrypto.PubKey, err error) { @@ -296,3 +278,30 @@ func IsSupportedAlgorithm(supported []SigningAlgo, algo SigningAlgo) bool { } return false } + +// SignWithLedger signs a binary message with the ledger device referenced by an Info object +// and returns the signed bytes and the public key. It returns an error if the device could +// not be queried or it returned an error. +func SignWithLedger(info Info, msg []byte) (sig []byte, pub tmcrypto.PubKey, err error) { + switch info.(type) { + case *ledgerInfo, ledgerInfo: + default: + return nil, nil, errors.New("not a ledger object") + } + path, err := info.GetPath() + if err != nil { + return + } + + priv, err := crypto.NewPrivKeyLedgerSecp256k1Unsafe(*path) + if err != nil { + return + } + + sig, err = priv.Sign(msg) + if err != nil { + return nil, nil, err + } + + return sig, priv.PubKey(), nil +} diff --git a/crypto/keys/keybase_test.go b/crypto/keys/keybase_test.go index 0c2396afb..cbd212a75 100644 --- a/crypto/keys/keybase_test.go +++ b/crypto/keys/keybase_test.go @@ -2,7 +2,9 @@ package keys import ( + "errors" "fmt" + "io" "testing" "github.com/stretchr/testify/assert" @@ -278,7 +280,7 @@ func TestSignVerify(t *testing.T) { // Now try to sign data with a secret-less key _, _, err = cstore.Sign(n3, p3, d3) - require.NotNil(t, err) + require.True(t, errors.Is(io.EOF, err)) } func assertPassword(t *testing.T, cstore Keybase, name, pass, badpass string) { diff --git a/crypto/keys/keyerror/errors_test.go b/crypto/keys/keyerror/errors_test.go new file mode 100644 index 000000000..80aaf9a69 --- /dev/null +++ b/crypto/keys/keyerror/errors_test.go @@ -0,0 +1,24 @@ +package keyerror_test + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/crypto/keys/keyerror" +) + +func TestErrors(t *testing.T) { + err := keyerror.NewErrKeyNotFound("test") + require.True(t, keyerror.IsErrKeyNotFound(err)) + require.Equal(t, "Key test not found", err.Error()) + require.False(t, keyerror.IsErrKeyNotFound(errors.New("test"))) + require.False(t, keyerror.IsErrKeyNotFound(nil)) + + err = keyerror.NewErrWrongPassword() + require.True(t, keyerror.IsErrWrongPassword(err)) + require.Equal(t, "invalid account password", err.Error()) + require.False(t, keyerror.IsErrWrongPassword(errors.New("test"))) + require.False(t, keyerror.IsErrWrongPassword(nil)) +} diff --git a/crypto/keys/keyring.go b/crypto/keys/keyring.go index 3e79c796b..d21e55743 100644 --- a/crypto/keys/keyring.go +++ b/crypto/keys/keyring.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "os" "path/filepath" - "reflect" "sort" "strings" @@ -218,7 +217,7 @@ func (kb keyringKeybase) Sign(name, passphrase string, msg []byte) (sig []byte, } case ledgerInfo: - return kb.base.SignWithLedger(info, msg) + return SignWithLedger(info, msg) case offlineInfo, multiInfo: return kb.base.DecodeSignature(info, msg) @@ -419,29 +418,7 @@ func (kb keyringKeybase) Delete(name, _ string, _ bool) error { // The oldpass must be the current passphrase used for encryption, getNewpass is // a function to get the passphrase to permanently replace the current passphrase. func (kb keyringKeybase) Update(name, oldpass string, getNewpass func() (string, error)) error { - info, err := kb.Get(name) - if err != nil { - return err - } - - switch linfo := info.(type) { - case localInfo: - key, _, err := mintkey.UnarmorDecryptPrivKey(linfo.PrivKeyArmor, oldpass) - if err != nil { - return err - } - - newpass, err := getNewpass() - if err != nil { - return err - } - - kb.writeLocalKey(name, key, newpass, linfo.GetAlgo()) - return nil - - default: - return fmt.Errorf("locally stored key required; received: %v", reflect.TypeOf(info).String()) - } + return errors.New("unsupported operation") } // SupportedAlgos returns a list of supported signing algorithms. diff --git a/crypto/keys/keyring_test.go b/crypto/keys/keyring_test.go index 4c024fab9..dd179a0c1 100644 --- a/crypto/keys/keyring_test.go +++ b/crypto/keys/keyring_test.go @@ -2,6 +2,7 @@ package keys import ( + "bytes" "testing" "github.com/stretchr/testify/assert" @@ -94,8 +95,46 @@ func TestLazyKeyManagementKeyRing(t *testing.T) { require.Equal(t, 1, len(keyS)) // addr cache gets nuked - and test skip flag - err = kb.Delete(n2, "", true) + require.NoError(t, kb.Delete(n2, "", true)) + + require.NotPanics(t, kb.CloseDB) +} + +// TestSignVerify does some detailed checks on how we sign and validate +// signatures +func TestLazySignVerifyKeyRingWithLedger(t *testing.T) { + dir, cleanup := tests.NewTestCaseDir(t) + t.Cleanup(cleanup) + kb, err := NewKeyring("keybasename", "test", dir, nil) require.NoError(t, err) + + i1, err := kb.CreateLedger("key", Secp256k1, "cosmos", 0, 0) + if err != nil { + require.Equal(t, "ledger nano S: support for ledger devices is not available in this executable", err.Error()) + t.Skip("ledger nano S: support for ledger devices is not available in this executable") + return + } + require.Equal(t, "key", i1.GetName()) + + p1 := "1234" + d1 := []byte("my first message") + s1, pub1, err := kb.Sign("key", p1, d1) + require.NoError(t, err) + + s2, pub2, err := SignWithLedger(i1, d1) + require.NoError(t, err) + + require.Equal(t, i1.GetPubKey(), pub1) + require.Equal(t, i1.GetPubKey(), pub2) + require.True(t, pub1.VerifyBytes(d1, s1)) + require.True(t, i1.GetPubKey().VerifyBytes(d1, s1)) + require.True(t, bytes.Equal(s1, s2)) + + localInfo, _, err := kb.CreateMnemonic("test", English, p1, Secp256k1) + require.NoError(t, err) + _, _, err = SignWithLedger(localInfo, d1) + require.Error(t, err) + require.Equal(t, "not a ledger object", err.Error()) } func TestLazySignVerifyKeyRing(t *testing.T) { @@ -325,3 +364,50 @@ func TestLazySeedPhraseKeyRing(t *testing.T) { require.Equal(t, info.GetPubKey().Address(), newInfo.GetPubKey().Address()) require.Equal(t, info.GetPubKey(), newInfo.GetPubKey()) } + +func TestKeyringKeybaseExportImportPrivKey(t *testing.T) { + dir, cleanup := tests.NewTestCaseDir(t) + t.Cleanup(cleanup) + kb, err := NewKeyring("keybasename", "test", dir, nil) + require.NoError(t, err) + _, _, err = kb.CreateMnemonic("john", English, "password", Secp256k1) + require.NoError(t, err) + + // no error, password is irrelevant, keystr cointains ASCII armored private key + keystr, err := kb.ExportPrivKey("john", "wrongpassword", "password") + require.NoError(t, err) + require.NotEmpty(t, keystr) + + // try import the key - wrong password + err = kb.ImportPrivKey("john2", keystr, "somepassword") + require.Equal(t, "failed to decrypt private key: ciphertext decryption failed", err.Error()) + + // try import the key with the correct password + require.NoError(t, kb.ImportPrivKey("john2", keystr, "password")) + + // overwrite is not allowed + err = kb.ImportPrivKey("john2", keystr, "password") + require.Equal(t, "cannot overwrite key: john2", err.Error()) + + // try export non existing key + _, err = kb.ExportPrivKey("john3", "wrongpassword", "password") + require.Equal(t, "The specified item could not be found in the keyring", err.Error()) +} + +func TestKeyringKeybaseUpdate(t *testing.T) { + dir, cleanup := tests.NewTestCaseDir(t) + t.Cleanup(cleanup) + kb, err := NewKeyring("keybasename", "test", dir, nil) + require.NoError(t, err) + require.Equal(t, "unsupported operation", kb.Update("john", "oldpassword", + func() (string, error) { return "", nil }).Error()) +} + +func TestSupportedAlgos(t *testing.T) { + dir, cleanup := tests.NewTestCaseDir(t) + t.Cleanup(cleanup) + kb, err := NewKeyring("keybasename", "test", dir, nil) + require.NoError(t, err) + require.Equal(t, []SigningAlgo([]SigningAlgo{"secp256k1"}), kb.SupportedAlgos()) + require.Equal(t, []SigningAlgo([]SigningAlgo{"secp256k1"}), kb.SupportedAlgosLedger()) +} diff --git a/crypto/keys/mintkey/mintkey.go b/crypto/keys/mintkey/mintkey.go index 8b4588498..7ce836d68 100644 --- a/crypto/keys/mintkey/mintkey.go +++ b/crypto/keys/mintkey/mintkey.go @@ -11,9 +11,8 @@ import ( cryptoAmino "github.com/tendermint/tendermint/crypto/encoding/amino" "github.com/tendermint/tendermint/crypto/xsalsa20symmetric" - tmos "github.com/tendermint/tendermint/libs/os" - "github.com/cosmos/cosmos-sdk/crypto/keys/keyerror" + "github.com/cosmos/cosmos-sdk/types/errors" ) const ( @@ -134,7 +133,7 @@ func encryptPrivKey(privKey crypto.PrivKey, passphrase string) (saltBytes []byte saltBytes = crypto.CRandBytes(16) key, err := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), BcryptSecurityParameter) if err != nil { - tmos.Exit("Error generating bcrypt key from passphrase: " + err.Error()) + panic(errors.Wrap(err, "error generating bcrypt key from passphrase")) } key = crypto.Sha256(key) // get 32 bytes privKeyBytes := privKey.Bytes() @@ -151,7 +150,7 @@ func UnarmorDecryptPrivKey(armorStr string, passphrase string) (privKey crypto.P return privKey, "", fmt.Errorf("unrecognized armor type: %v", blockType) } if header["kdf"] != "bcrypt" { - return privKey, "", fmt.Errorf("unrecognized KDF type: %v", header["KDF"]) + return privKey, "", fmt.Errorf("unrecognized KDF type: %v", header["kdf"]) } if header["salt"] == "" { return privKey, "", fmt.Errorf("missing salt bytes") @@ -171,7 +170,7 @@ func UnarmorDecryptPrivKey(armorStr string, passphrase string) (privKey crypto.P func decryptPrivKey(saltBytes []byte, encBytes []byte, passphrase string) (privKey crypto.PrivKey, err error) { key, err := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), BcryptSecurityParameter) if err != nil { - tmos.Exit("error generating bcrypt key from passphrase: " + err.Error()) + return privKey, errors.Wrap(err, "error generating bcrypt key from passphrase") } key = crypto.Sha256(key) // Get 32 bytes privKeyBytes, err := xsalsa20symmetric.DecryptSymmetric(encBytes, key) diff --git a/crypto/keys/mintkey/mintkey_test.go b/crypto/keys/mintkey/mintkey_test.go index ef5f77d33..79c158344 100644 --- a/crypto/keys/mintkey/mintkey_test.go +++ b/crypto/keys/mintkey/mintkey_test.go @@ -1,11 +1,19 @@ package mintkey_test import ( + "bytes" + "errors" + "fmt" + "io" "testing" "github.com/stretchr/testify/require" + "github.com/tendermint/crypto/bcrypt" + "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/armor" cryptoAmino "github.com/tendermint/tendermint/crypto/encoding/amino" "github.com/tendermint/tendermint/crypto/secp256k1" + "github.com/tendermint/tendermint/crypto/xsalsa20symmetric" "github.com/cosmos/cosmos-sdk/crypto/keys" "github.com/cosmos/cosmos-sdk/crypto/keys/mintkey" @@ -13,13 +21,48 @@ import ( func TestArmorUnarmorPrivKey(t *testing.T) { priv := secp256k1.GenPrivKey() - armor := mintkey.EncryptArmorPrivKey(priv, "passphrase", "") - _, _, err := mintkey.UnarmorDecryptPrivKey(armor, "wrongpassphrase") + armored := mintkey.EncryptArmorPrivKey(priv, "passphrase", "") + _, _, err := mintkey.UnarmorDecryptPrivKey(armored, "wrongpassphrase") require.Error(t, err) - decrypted, algo, err := mintkey.UnarmorDecryptPrivKey(armor, "passphrase") + decrypted, algo, err := mintkey.UnarmorDecryptPrivKey(armored, "passphrase") require.NoError(t, err) require.Equal(t, string(keys.Secp256k1), algo) require.True(t, priv.Equals(decrypted)) + + // empty string + decrypted, algo, err = mintkey.UnarmorDecryptPrivKey("", "passphrase") + require.Error(t, err) + require.True(t, errors.Is(io.EOF, err)) + require.Nil(t, decrypted) + require.Empty(t, algo) + + // wrong key type + armored = mintkey.ArmorPubKeyBytes(priv.PubKey().Bytes(), "") + decrypted, algo, err = mintkey.UnarmorDecryptPrivKey(armored, "passphrase") + require.Error(t, err) + require.Contains(t, err.Error(), "unrecognized armor type") + + // armor key manually + encryptPrivKeyFn := func(privKey crypto.PrivKey, passphrase string) (saltBytes []byte, encBytes []byte) { + saltBytes = crypto.CRandBytes(16) + key, err := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), mintkey.BcryptSecurityParameter) + require.NoError(t, err) + key = crypto.Sha256(key) // get 32 bytes + privKeyBytes := privKey.Bytes() + return saltBytes, xsalsa20symmetric.EncryptSymmetric(privKeyBytes, key) + } + saltBytes, encBytes := encryptPrivKeyFn(priv, "passphrase") + + // wrong kdf header + headerWrongKdf := map[string]string{ + "kdf": "wrong", + "salt": fmt.Sprintf("%X", saltBytes), + "type": "secp256k", + } + armored = armor.EncodeArmor("TENDERMINT PRIVATE KEY", headerWrongKdf, encBytes) + _, _, err = mintkey.UnarmorDecryptPrivKey(armored, "passphrase") + require.Error(t, err) + require.Equal(t, "unrecognized KDF type: wrong", err.Error()) } func TestArmorUnarmorPubKey(t *testing.T) { @@ -29,11 +72,66 @@ func TestArmorUnarmorPubKey(t *testing.T) { // Add keys and see they return in alphabetical order info, _, err := cstore.CreateMnemonic("Bob", keys.English, "passphrase", keys.Secp256k1) require.NoError(t, err) - armor := mintkey.ArmorPubKeyBytes(info.GetPubKey().Bytes(), "") - pubBytes, algo, err := mintkey.UnarmorPubKeyBytes(armor) + armored := mintkey.ArmorPubKeyBytes(info.GetPubKey().Bytes(), "") + pubBytes, algo, err := mintkey.UnarmorPubKeyBytes(armored) require.NoError(t, err) pub, err := cryptoAmino.PubKeyFromBytes(pubBytes) require.NoError(t, err) require.Equal(t, string(keys.Secp256k1), algo) require.True(t, pub.Equals(info.GetPubKey())) + + armored = mintkey.ArmorPubKeyBytes(info.GetPubKey().Bytes(), "unknown") + pubBytes, algo, err = mintkey.UnarmorPubKeyBytes(armored) + require.NoError(t, err) + pub, err = cryptoAmino.PubKeyFromBytes(pubBytes) + require.NoError(t, err) + require.Equal(t, "unknown", algo) + require.True(t, pub.Equals(info.GetPubKey())) + + // armor pubkey manually + header := map[string]string{ + "version": "0.0.0", + "type": "unknown", + } + armored = armor.EncodeArmor("TENDERMINT PUBLIC KEY", header, pubBytes) + _, algo, err = mintkey.UnarmorPubKeyBytes(armored) + require.NoError(t, err) + // return secp256k1 if version is 0.0.0 + require.Equal(t, "secp256k1", algo) + + // missing version header + header = map[string]string{ + "type": "unknown", + } + armored = armor.EncodeArmor("TENDERMINT PUBLIC KEY", header, pubBytes) + bz, algo, err := mintkey.UnarmorPubKeyBytes(armored) + require.Nil(t, bz) + require.Empty(t, algo) + require.Error(t, err) + require.Contains(t, err.Error(), "unrecognized version") +} + +func TestArmorInfoBytes(t *testing.T) { + bs := []byte("test") + armoredString := mintkey.ArmorInfoBytes(bs) + unarmoredBytes, err := mintkey.UnarmorInfoBytes(armoredString) + require.NoError(t, err) + require.True(t, bytes.Equal(bs, unarmoredBytes)) +} + +func TestUnarmorInfoBytesErrors(t *testing.T) { + unarmoredBytes, err := mintkey.UnarmorInfoBytes("") + require.Error(t, err) + require.True(t, errors.Is(io.EOF, err)) + require.Nil(t, unarmoredBytes) + + header := map[string]string{ + "type": "Info", + "version": "0.0.1", + } + unarmoredBytes, err = mintkey.UnarmorInfoBytes(armor.EncodeArmor( + "TENDERMINT KEY INFO", header, []byte("plain-text"))) + require.Error(t, err) + require.Equal(t, "unrecognized version: 0.0.1", err.Error()) + require.Nil(t, unarmoredBytes) } diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index eb1c9efe5..2c346d435 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -12,7 +12,6 @@ import ( tmsecp256k1 "github.com/tendermint/tendermint/crypto/secp256k1" "github.com/cosmos/cosmos-sdk/crypto/keys/hd" - sdk "github.com/cosmos/cosmos-sdk/types" ) var ( @@ -103,7 +102,9 @@ func (pkl PrivKeyLedgerSecp256k1) Sign(message []byte) ([]byte, error) { } // LedgerShowAddress triggers a ledger device to show the corresponding address. -func LedgerShowAddress(path hd.BIP44Params, expectedPubKey tmcrypto.PubKey) error { +func LedgerShowAddress(path hd.BIP44Params, expectedPubKey tmcrypto.PubKey, + accountAddressPrefix string) error { + device, err := getLedgerDevice() if err != nil { return err @@ -119,8 +120,7 @@ func LedgerShowAddress(path hd.BIP44Params, expectedPubKey tmcrypto.PubKey) erro return fmt.Errorf("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones") } - config := sdk.GetConfig() - pubKey2, _, err := getPubKeyAddrSafe(device, path, config.GetBech32AccountAddrPrefix()) + pubKey2, _, err := getPubKeyAddrSafe(device, path, accountAddressPrefix) if err != nil { return err } diff --git a/crypto/ledger_test.go b/crypto/ledger_test.go index 0f6c4572a..fbcf9239b 100644 --- a/crypto/ledger_test.go +++ b/crypto/ledger_test.go @@ -104,6 +104,8 @@ func TestPublicKeySafe(t *testing.T) { require.Nil(t, err, "%s", err) require.NotNil(t, priv) + require.Nil(t, LedgerShowAddress(path, priv.PubKey(), sdk.GetConfig().GetBech32AccountAddrPrefix())) + require.Equal(t, "eb5ae98721034fef9cd7c4c63588d3b03feb5281b9d232cba34d6f3d71aee59211ffbfe1fe87", fmt.Sprintf("%x", priv.PubKey().Bytes()), "Is your device using test mnemonic: %s ?", tests.TestMnemonic)