crypto: cleanup, more testing

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
 

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

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)

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
+}

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) {

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))
+}

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.

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())
+}

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)