Merge pull request #59 from tendermint/sdk2-cleanup

Sdk2 cleanup

circle.yml

@@ -18,4 +18,4 @@ dependencies:
 test:
   override:
     - "go version"
-    - "cd $PROJECT_PATH && make all"
+    - "cd $PROJECT_PATH && make get_tools && make all"

keys/keybase.go

@@ -8,6 +8,7 @@ import (
 	crypto "github.com/tendermint/go-crypto"
 	dbm "github.com/tendermint/tmlibs/db"
 
+	"github.com/tendermint/go-crypto/keys/words"
 	"github.com/tendermint/go-crypto/nano"
 )
 
@@ -19,10 +20,10 @@ import (
 // a full-featured key manager
 type dbKeybase struct {
 	db    dbm.DB
-	codec Codec
+	codec words.Codec
 }
 
-func New(db dbm.DB, codec Codec) dbKeybase {
+func New(db dbm.DB, codec words.Codec) dbKeybase {
 	return dbKeybase{
 		db:    db,
 		codec: codec,
@@ -31,59 +32,61 @@ func New(db dbm.DB, codec Codec) dbKeybase {
 
 var _ Keybase = dbKeybase{}
 
-// Create adds a new key to the storage engine, returning error if
-// another key already stored under this name
-//
-// algo must be a supported go-crypto algorithm: ed25519, secp256k1
-func (kb dbKeybase) Create(name, passphrase, algo string) (Info, string, error) {
-	// 128-bits are the all the randomness we can make use of
+// Create generates a new key and persists it storage, encrypted using the passphrase.
+// It returns the generated seedphrase (mnemonic) and the key Info.
+// It returns an error if it fails to generate a key for the given algo type,
+// or if another key is already stored under the same name.
+func (kb dbKeybase) Create(name, passphrase, algo string) (string, Info, error) {
+	// NOTE: secret is SHA256 hashed by secp256k1 and ed25519.
+	// 16 byte secret corresponds to 12 BIP39 words.
+	// XXX: Ledgers use 24 words now - should we ?
 	secret := crypto.CRandBytes(16)
 	key, err := generate(algo, secret)
 	if err != nil {
-		return Info{}, "", err
+		return "", Info{}, err
 	}
 
+	// encrypt and persist the key
 	public := kb.writeKey(key, name, passphrase)
 
-	// we append the type byte to the serialized secret to help with recovery
-	// ie [secret] = [secret] + [type]
-	typ := key.Bytes()[0]
-	secret = append(secret, typ)
-
-	seed, err := kb.codec.BytesToWords(secret)
-	phrase := strings.Join(seed, " ")
-	return public, phrase, err
+	// return the mnemonic phrase
+	words, err := kb.codec.BytesToWords(secret)
+	seedphrase := strings.Join(words, " ")
+	return seedphrase, public, err
 }
 
-// Recover takes a seed phrase and tries to recover the private key.
-//
-// If the seed phrase is valid, it will create the private key and store
-// it under name, protected by passphrase.
-//
-// Result similar to New(), except it doesn't return the seed again...
-func (kb dbKeybase) Recover(name, passphrase, seedphrase string) (Info, error) {
-	words := strings.Split(strings.TrimSpace(seedphrase), " ")
-	secret, err := kb.codec.WordsToBytes(words)
+// Recover converts a seedphrase to a private key and persists it, encrypted with the given passphrase.
+// Functions like Create, but seedphrase is input not output.
+func (kb dbKeybase) Recover(name, passphrase, algo string, seedphrase string) (Info, error) {
+
+	key, err := kb.SeedToPrivKey(algo, seedphrase)
 	if err != nil {
 		return Info{}, err
 	}
 
-	// secret is comprised of the actual secret with the type appended
-	// ie [secret] = [secret] + [type]
-	l := len(secret)
-	secret, typ := secret[:l-1], secret[l-1]
+	// Valid seedphrase. Encrypt key and persist to disk.
+	public := kb.writeKey(key, name, passphrase)
+	return public, nil
+}
 
-	key, err := generateByType(typ, secret)
+// SeedToPrivKey returns the private key corresponding to a seedphrase
+// without persisting the private key.
+// TODO: enable the keybase to just hold these in memory so we can sign without persisting (?)
+func (kb dbKeybase) SeedToPrivKey(algo, seedphrase string) (crypto.PrivKey, error) {
+	words := strings.Split(strings.TrimSpace(seedphrase), " ")
+	secret, err := kb.codec.WordsToBytes(words)
 	if err != nil {
-		return Info{}, err
+		return crypto.PrivKey{}, err
 	}
 
-	// d00d, it worked!  create the bugger....
-	public := kb.writeKey(key, name, passphrase)
-	return public, err
+	key, err := generate(algo, secret)
+	if err != nil {
+		return crypto.PrivKey{}, err
+	}
+	return key, nil
 }
 
-// List loads the keys from the storage and enforces alphabetical order
+// List returns the keys from storage in alphabetical order.
 func (kb dbKeybase) List() ([]Info, error) {
 	var res []Info
 	iter := kb.db.Iterator(nil, nil)
@@ -101,20 +104,19 @@ func (kb dbKeybase) List() ([]Info, error) {
 	return res, nil
 }
 
-// Get returns the public information about one key
+// Get returns the public information about one key.
 func (kb dbKeybase) Get(name string) (Info, error) {
 	bs := kb.db.Get(pubName(name))
 	return readInfo(bs)
 }
 
-// Sign will modify the Signable in order to attach a valid signature with
-// this public key
-//
-// If no key for this name, or the passphrase doesn't match, returns an error
+// Sign signs the msg with the named key.
+// It returns an error if the key doesn't exist or the decryption fails.
+// TODO: what if leddger fails ?
 func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig crypto.Signature, pk crypto.PubKey, err error) {
 	var key crypto.PrivKey
-	bs := kb.db.Get(privName(name))
-	key, err = unarmorDecryptPrivKey(string(bs), passphrase)
+	armorStr := kb.db.Get(privName(name))
+	key, err = unarmorDecryptPrivKey(string(armorStr), passphrase)
 	if err != nil {
 		return
 	}
@@ -124,15 +126,15 @@ func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig crypto.Signat
 	return
 }
 
-// Export decodes the private key with the current password, encodes
-// it with a secure one-time password and generates a sequence that can be
-// Imported by another dbKeybase
+// Export decodes the private key with the current password, encrypts
+// it with a secure one-time password and generates an armored private key
+// that can be Imported by another dbKeybase.
 //
 // This is designed to copy from one device to another, or provide backups
 // during version updates.
 func (kb dbKeybase) Export(name, oldpass, transferpass string) ([]byte, error) {
-	bs := kb.db.Get(privName(name))
-	key, err := unarmorDecryptPrivKey(string(bs), oldpass)
+	armorStr := kb.db.Get(privName(name))
+	key, err := unarmorDecryptPrivKey(string(armorStr), oldpass)
 	if err != nil {
 		return nil, err
 	}
@@ -140,11 +142,11 @@ func (kb dbKeybase) Export(name, oldpass, transferpass string) ([]byte, error) {
 	if transferpass == "" {
 		return key.Bytes(), nil
 	}
-	res := encryptArmorPrivKey(key, transferpass)
-	return []byte(res), nil
+	armorBytes := encryptArmorPrivKey(key, transferpass)
+	return []byte(armorBytes), nil
 }
 
-// Import accepts bytes generated by Export along with the same transferpass
+// Import accepts bytes generated by Export along with the same transferpass.
 // If they are valid, it stores the password under the given name with the
 // new passphrase.
 func (kb dbKeybase) Import(name, newpass, transferpass string, data []byte) (err error) {
@@ -163,7 +165,7 @@ func (kb dbKeybase) Import(name, newpass, transferpass string, data []byte) (err
 }
 
 // Delete removes key forever, but we must present the
-// proper passphrase before deleting it (for security)
+// proper passphrase before deleting it (for security).
 func (kb dbKeybase) Delete(name, passphrase string) error {
 	// verify we have the proper password before deleting
 	bs := kb.db.Get(privName(name))
@@ -176,10 +178,10 @@ func (kb dbKeybase) Delete(name, passphrase string) error {
 	return nil
 }
 
-// Update changes the passphrase with which a already stored key is encoded.
+// Update changes the passphrase with which an already stored key is encrypted.
 //
-// oldpass must be the current passphrase used for encoding, newpass will be
-// the only valid passphrase from this time forward
+// oldpass must be the current passphrase used for encryption, newpass will be
+// the only valid passphrase from this time forward.
 func (kb dbKeybase) Update(name, oldpass, newpass string) error {
 	bs := kb.db.Get(privName(name))
 	key, err := unarmorDecryptPrivKey(string(bs), oldpass)
@@ -187,26 +189,37 @@ func (kb dbKeybase) Update(name, oldpass, newpass string) error {
 		return err
 	}
 
-	// we must delete first, as Putting over an existing name returns an error
-	kb.db.DeleteSync(pubName(name))
-	kb.db.DeleteSync(privName(name))
-	kb.writeKey(key, name, newpass)
+	// Generate the public bytes and the encrypted privkey
+	public := info(name, key)
+	private := encryptArmorPrivKey(key, newpass)
+
+	// We must delete first, as Putting over an existing name returns an error.
+	// Must be done atomically with the write or we could lose the key.
+	batch := kb.db.NewBatch()
+	batch.Delete(pubName(name))
+	batch.Delete(privName(name))
+	batch.Set(pubName(name), public.bytes())
+	batch.Set(privName(name), []byte(private))
+	batch.Write()
+
 	return nil
 }
 
+//---------------------------------------------------------------------------------------
+
 func (kb dbKeybase) writeKey(priv crypto.PrivKey, name, passphrase string) Info {
-	// generate the public bytes
+	// Generate the public bytes and the encrypted privkey
 	public := info(name, priv)
-	// generate the encrypted privkey
 	private := encryptArmorPrivKey(priv, passphrase)
 
-	// write them both
+	// Write them both
 	kb.db.SetSync(pubName(name), public.bytes())
 	kb.db.SetSync(privName(name), []byte(private))
 
 	return public
 }
 
+// TODO: use a `type TypeKeyAlgo string` (?)
 func generate(algo string, secret []byte) (crypto.PrivKey, error) {
 	switch algo {
 	case crypto.NameEd25519:
@@ -214,27 +227,13 @@ func generate(algo string, secret []byte) (crypto.PrivKey, error) {
 	case crypto.NameSecp256k1:
 		return crypto.GenPrivKeySecp256k1FromSecret(secret).Wrap(), nil
 	case nano.NameLedgerEd25519:
-		return nano.NewPrivKeyLedgerEd25519Ed25519()
+		return nano.NewPrivKeyLedgerEd25519()
 	default:
 		err := errors.Errorf("Cannot generate keys for algorithm: %s", algo)
 		return crypto.PrivKey{}, err
 	}
 }
 
-func generateByType(typ byte, secret []byte) (crypto.PrivKey, error) {
-	switch typ {
-	case crypto.TypeEd25519:
-		return crypto.GenPrivKeyEd25519FromSecret(secret).Wrap(), nil
-	case crypto.TypeSecp256k1:
-		return crypto.GenPrivKeySecp256k1FromSecret(secret).Wrap(), nil
-	case nano.TypeLedgerEd25519:
-		return nano.NewPrivKeyLedgerEd25519Ed25519()
-	default:
-		err := errors.Errorf("Cannot generate keys for algorithm: %X", typ)
-		return crypto.PrivKey{}, err
-	}
-}
-
 func pubName(name string) []byte {
 	return []byte(fmt.Sprintf("%s.pub", name))
 }

keys/keybase_test.go

@@ -13,6 +13,7 @@ import (
 
 	crypto "github.com/tendermint/go-crypto"
 	"github.com/tendermint/go-crypto/keys"
+	"github.com/tendermint/go-crypto/keys/words"
 	"github.com/tendermint/go-crypto/nano"
 )
 
@@ -23,7 +24,7 @@ func TestKeyManagement(t *testing.T) {
 	// make the storage with reasonable defaults
 	cstore := keys.New(
 		dbm.NewMemDB(),
-		keys.MustLoadCodec("english"),
+		words.MustLoadCodec("english"),
 	)
 
 	algo := crypto.NameEd25519
@@ -38,7 +39,7 @@ func TestKeyManagement(t *testing.T) {
 	// create some keys
 	_, err = cstore.Get(n1)
 	assert.NotNil(err)
-	i, _, err := cstore.Create(n1, p1, algo)
+	_, i, err := cstore.Create(n1, p1, algo)
 	require.Equal(n1, i.Name)
 	require.Nil(err)
 	_, _, err = cstore.Create(n2, p2, algo)
@@ -91,7 +92,7 @@ func TestSignVerify(t *testing.T) {
 	// make the storage with reasonable defaults
 	cstore := keys.New(
 		dbm.NewMemDB(),
-		keys.MustLoadCodec("english"),
+		words.MustLoadCodec("english"),
 	)
 	algo := crypto.NameSecp256k1
 
@@ -99,10 +100,10 @@ func TestSignVerify(t *testing.T) {
 	p1, p2 := "1234", "foobar"
 
 	// create two users and get their info
-	i1, _, err := cstore.Create(n1, p1, algo)
+	_, i1, err := cstore.Create(n1, p1, algo)
 	require.Nil(err)
 
-	i2, _, err := cstore.Create(n2, p2, algo)
+	_, i2, err := cstore.Create(n2, p2, algo)
 	require.Nil(err)
 
 	// let's try to sign some messages
@@ -165,13 +166,13 @@ func TestSignWithLedger(t *testing.T) {
 	// make the storage with reasonable defaults
 	cstore := keys.New(
 		dbm.NewMemDB(),
-		keys.MustLoadCodec("english"),
+		words.MustLoadCodec("english"),
 	)
 	n := "nano-s"
 	p := "hard2hack"
 
 	// create a nano user
-	c, _, err := cstore.Create(n, p, nano.NameLedgerEd25519)
+	_, c, err := cstore.Create(n, p, nano.NameLedgerEd25519)
 	require.Nil(err, "%+v", err)
 	assert.Equal(c.Name, n)
 	_, ok := c.PubKey.Unwrap().(nano.PubKeyLedgerEd25519)
@@ -219,7 +220,7 @@ func TestImportUnencrypted(t *testing.T) {
 	// make the storage with reasonable defaults
 	cstore := keys.New(
 		dbm.NewMemDB(),
-		keys.MustLoadCodec("english"),
+		words.MustLoadCodec("english"),
 	)
 
 	key := crypto.GenPrivKeyEd25519FromSecret(cmn.RandBytes(16)).Wrap()
@@ -245,7 +246,7 @@ func TestAdvancedKeyManagement(t *testing.T) {
 	// make the storage with reasonable defaults
 	cstore := keys.New(
 		dbm.NewMemDB(),
-		keys.MustLoadCodec("english"),
+		words.MustLoadCodec("english"),
 	)
 
 	algo := crypto.NameSecp256k1
@@ -288,7 +289,7 @@ func TestSeedPhrase(t *testing.T) {
 	// make the storage with reasonable defaults
 	cstore := keys.New(
 		dbm.NewMemDB(),
-		keys.MustLoadCodec("english"),
+		words.MustLoadCodec("english"),
 	)
 
 	algo := crypto.NameEd25519
@@ -296,7 +297,7 @@ func TestSeedPhrase(t *testing.T) {
 	p1, p2 := "1234", "foobar"
 
 	// make sure key works with initial password
-	info, seed, err := cstore.Create(n1, p1, algo)
+	seed, info, err := cstore.Create(n1, p1, algo)
 	require.Nil(err, "%+v", err)
 	assert.Equal(n1, info.Name)
 	assert.NotEmpty(seed)
@@ -308,7 +309,7 @@ func TestSeedPhrase(t *testing.T) {
 	require.NotNil(err)
 
 	// let us re-create it from the seed-phrase
-	newInfo, err := cstore.Recover(n2, p2, seed)
+	newInfo, err := cstore.Recover(n2, p2, algo, seed)
 	require.Nil(err, "%+v", err)
 	assert.Equal(n2, newInfo.Name)
 	assert.Equal(info.Address(), newInfo.Address())
@@ -319,13 +320,13 @@ func ExampleNew() {
 	// Select the encryption and storage for your cryptostore
 	cstore := keys.New(
 		dbm.NewMemDB(),
-		keys.MustLoadCodec("english"),
+		words.MustLoadCodec("english"),
 	)
 	ed := crypto.NameEd25519
 	sec := crypto.NameSecp256k1
 
 	// Add keys and see they return in alphabetical order
-	bob, _, err := cstore.Create("Bob", "friend", ed)
+	_, bob, err := cstore.Create("Bob", "friend", ed)
 	if err != nil {
 		// this should never happen
 		fmt.Println(err)

keys/types.go

@@ -38,9 +38,9 @@ type Keybase interface {
 	// Sign some bytes
 	Sign(name, passphrase string, msg []byte) (crypto.Signature, crypto.PubKey, error)
 	// Create a new keypair
-	Create(name, passphrase, algo string) (_ Info, seedphrase string, _ error)
+	Create(name, passphrase, algo string) (seedphrase string, _ Info, _ error)
 	// Recover takes a seedphrase and loads in the key
-	Recover(name, passphrase, seedphrase string) (Info, error)
+	Recover(name, passphrase, algo, seedphrase string) (Info, error)
 	List() ([]Info, error)
 	Get(name string) (Info, error)
 	Update(name, oldpass, newpass string) error

nano/keys.go

@@ -33,21 +33,21 @@ func getLedger() (*ledger.Ledger, error) {
 	return device, err
 }
 
-func signLedger(device *ledger.Ledger, msg []byte) (pk crypto.PubKey, sig crypto.Signature, err error) {
+func signLedger(device *ledger.Ledger, msg []byte) (pub crypto.PubKey, sig crypto.Signature, err error) {
 	var resp []byte
 
 	packets := generateSignRequests(msg)
 	for _, pack := range packets {
 		resp, err = device.Exchange(pack, Timeout)
 		if err != nil {
-			return pk, sig, err
+			return pub, sig, err
 		}
 	}
 
 	// the last call is the result we want and needs to be parsed
 	key, bsig, err := parseDigest(resp)
 	if err != nil {
-		return pk, sig, err
+		return pub, sig, err
 	}
 
 	var b [32]byte
@@ -64,9 +64,9 @@ type PrivKeyLedgerEd25519 struct {
 	CachedPubKey crypto.PubKey
 }
 
-// NewPrivKeyLedgerEd25519Ed25519 will generate a new key and store the
+// NewPrivKeyLedgerEd25519 will generate a new key and store the
 // public key for later use.
-func NewPrivKeyLedgerEd25519Ed25519() (crypto.PrivKey, error) {
+func NewPrivKeyLedgerEd25519() (crypto.PrivKey, error) {
 	var pk PrivKeyLedgerEd25519
 	// getPubKey will cache the pubkey for later use,
 	// this allows us to return an error early if the ledger
@@ -94,13 +94,13 @@ func (pk *PrivKeyLedgerEd25519) ValidateKey() error {
 // AssertIsPrivKeyInner fulfils PrivKey Interface
 func (pk *PrivKeyLedgerEd25519) AssertIsPrivKeyInner() {}
 
-// Bytes fulfils pk Interface - stores the cached pubkey so we can verify
+// Bytes fulfils PrivKey Interface - but it stores the cached pubkey so we can verify
 // the same key when we reconnect to a ledger
 func (pk *PrivKeyLedgerEd25519) Bytes() []byte {
 	return wire.BinaryBytes(pk.Wrap())
 }
 
-// Sign calls the ledger and stores the pk for future use
+// Sign calls the ledger and stores the PubKey for future use
 //
 // XXX/TODO: panics if there is an error communicating with the ledger.
 //

nano/keys_test.go

@@ -83,7 +83,7 @@ func TestRealLedger(t *testing.T) {
 	}
 	msg := []byte("kuhehfeohg")
 
-	priv, err := NewPrivKeyLedgerEd25519Ed25519()
+	priv, err := NewPrivKeyLedgerEd25519()
 	require.Nil(err, "%+v", err)
 	pub := priv.PubKey()
 	sig := priv.Sign(msg)
@@ -123,7 +123,7 @@ func TestRealLedgerErrorHandling(t *testing.T) {
 
 	// first, try to generate a key, must return an error
 	// (no panic)
-	_, err := NewPrivKeyLedgerEd25519Ed25519()
+	_, err := NewPrivKeyLedgerEd25519()
 	require.Error(err)
 
 	led := PrivKeyLedgerEd25519{} // empty