keys/keybase.go: comments and fixes

keys/keybase.go

@@ -32,59 +32,61 @@ func New(db dbm.DB, codec words.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)
@@ -102,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
 	}
@@ -125,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
 	}
@@ -141,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) {
@@ -164,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))
@@ -177,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)
@@ -188,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:
@@ -222,20 +234,6 @@ func generate(algo string, secret []byte) (crypto.PrivKey, error) {
 	}
 }
 
-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.NewPrivKeyLedgerEd25519()
-	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

@@ -39,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)
@@ -100,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
@@ -172,7 +172,7 @@ func TestSignWithLedger(t *testing.T) {
 	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)
@@ -297,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)
@@ -309,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())
@@ -326,7 +326,7 @@ func ExampleNew() {
 	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