Address pull request comments

* Remove flags field from key struct
* Change JSON struct fields from string to []byte
* Change GenerateNewKey API to take io.Reader for random source
* Remove mixing entropy source function
* Use testing Fatal in tests

4 files changed, 88 insertions, 215 deletions

diff --git a/crypto/key.go b/crypto/key.go
index 1a8b770e0..0e7c04275 100644
--- a/crypto/key.go
+++ b/crypto/key.go
@@ -28,43 +28,31 @@ import (
 	"code.google.com/p/go-uuid/uuid"
 	"crypto/ecdsa"
 	"crypto/elliptic"
-	crand "crypto/rand"
-	"encoding/binary"
-	"encoding/hex"
 	"encoding/json"
-	"errors"
-	"fmt"
 	"io"
-	"os"
-	"runtime"
-	"strings"
-	"time"
 )
 
 type Key struct {
-	Id    *uuid.UUID // Version 4 "random" for unique id not derived from key data
-	Flags [4]byte    // RFU
+	Id *uuid.UUID // Version 4 "random" for unique id not derived from key data
 	// we only store privkey as pubkey/address can be derived from it
 	// privkey in this struct is always in plaintext
 	PrivateKey *ecdsa.PrivateKey
 }
 
-type PlainKeyJSON struct {
-	Id         string
-	Flags      string
-	PrivateKey string
+type plainKeyJSON struct {
+	Id         []byte
+	PrivateKey []byte
 }
 
-type CipherJSON struct {
-	Salt       string
-	IV         string
-	CipherText string
+type cipherJSON struct {
+	Salt       []byte
+	IV         []byte
+	CipherText []byte
 }
 
-type EncryptedKeyJSON struct {
-	Id     string
-	Flags  string
-	Crypto CipherJSON
+type encryptedKeyJSON struct {
+	Id     []byte
+	Crypto cipherJSON
 }
 
 func (k *Key) Address() []byte {
@@ -73,48 +61,40 @@ func (k *Key) Address() []byte {
 }
 
 func (k *Key) MarshalJSON() (j []byte, err error) {
-	stringStruct := PlainKeyJSON{
-		k.Id.String(),
-		hex.EncodeToString(k.Flags[:]),
-		hex.EncodeToString(FromECDSA(k.PrivateKey)),
+	jStruct := plainKeyJSON{
+		*k.Id,
+		FromECDSA(k.PrivateKey),
 	}
-	j, err = json.Marshal(stringStruct)
+	j, err = json.Marshal(jStruct)
 	return j, err
 }
 
 func (k *Key) UnmarshalJSON(j []byte) (err error) {
-	keyJSON := new(PlainKeyJSON)
+	keyJSON := new(plainKeyJSON)
 	err = json.Unmarshal(j, &keyJSON)
 	if err != nil {
 		return err
 	}
 
 	u := new(uuid.UUID)
-	*u = uuid.Parse(keyJSON.Id)
-	if *u == nil {
-		err = errors.New("UUID parsing failed")
-		return err
-	}
+	*u = keyJSON.Id
 	k.Id = u
 
-	flagsBytes, err := hex.DecodeString(keyJSON.Flags)
-	if err != nil {
-		return err
-	}
-
-	PrivateKeyBytes, err := hex.DecodeString(keyJSON.PrivateKey)
-	if err != nil {
-		return err
-	}
-
-	copy(k.Flags[:], flagsBytes[0:4])
-	k.PrivateKey = ToECDSA(PrivateKeyBytes)
+	k.PrivateKey = ToECDSA(keyJSON.PrivateKey)
 
 	return err
 }
 
-func NewKey() *Key {
-	randBytes := GetEntropyCSPRNG(32)
+func NewKey(rand io.Reader) *Key {
+	randBytes := make([]byte, 32)
+	n, err := rand.Read(randBytes)
+	if err != nil {
+		panic("key generation: could not read from random source: " + err.Error())
+	} else {
+		if n != 32 {
+			panic("key generation: read less than required bytes from random source: " + err.Error())
+		}
+	}
 	reader := bytes.NewReader(randBytes)
 	_, x, y, err := elliptic.GenerateKey(S256(), reader)
 	if err != nil {
@@ -126,80 +106,6 @@ func NewKey() *Key {
 	key := new(Key)
 	id := uuid.NewRandom()
 	key.Id = &id
-	// flags := new([4]byte)
-	// key.Flags = flags
 	key.PrivateKey = privateKeyECDSA
 	return key
 }
-
-// plain crypto/rand. this is /dev/urandom on Unix-like systems.
-func GetEntropyCSPRNG(n int) []byte {
-	mainBuff := make([]byte, n)
-	_, err := io.ReadFull(crand.Reader, mainBuff)
-	if err != nil {
-		panic("key generation: reading from crypto/rand failed: " + err.Error())
-	}
-	return mainBuff
-}
-
-// TODO: verify. Do not use until properly discussed.
-// we start with crypt/rand, then mix in additional sources of entropy.
-// These sources are from three types: OS, go runtime and ethereum client state.
-func GetEntropyTinFoilHat() []byte {
-	startTime := time.Now().UnixNano()
-	// for each source, we XOR in it's SHA3 hash.
-	mainBuff := GetEntropyCSPRNG(32)
-	// 1. OS entropy sources
-	startTimeBytes := make([]byte, 32)
-	binary.PutVarint(startTimeBytes, startTime)
-	startTimeHash := Sha3(startTimeBytes)
-	mix32Byte(mainBuff, startTimeHash)
-
-	pid := os.Getpid()
-	pidBytes := make([]byte, 32)
-	binary.PutUvarint(pidBytes, uint64(pid))
-	pidHash := Sha3(pidBytes)
-	mix32Byte(mainBuff, pidHash)
-
-	osEnv := os.Environ()
-	osEnvBytes := []byte(strings.Join(osEnv, ""))
-	osEnvHash := Sha3(osEnvBytes)
-	mix32Byte(mainBuff, osEnvHash)
-
-	// not all OS have hostname in env variables
-	osHostName, err := os.Hostname()
-	if err != nil {
-		osHostNameBytes := []byte(osHostName)
-		osHostNameHash := Sha3(osHostNameBytes)
-		mix32Byte(mainBuff, osHostNameHash)
-	}
-
-	// 2. go runtime entropy sources
-	memStats := new(runtime.MemStats)
-	runtime.ReadMemStats(memStats)
-	memStatsBytes := []byte(fmt.Sprintf("%v", memStats))
-	memStatsHash := Sha3(memStatsBytes)
-	mix32Byte(mainBuff, memStatsHash)
-
-	// 3. Mix in ethereum / client state
-	// TODO: list of network peers structs (IP, port, etc)
-	// TODO: merkle patricia tree root hash for world state and tx list
-
-	// 4. Yo dawg we heard you like entropy so we'll grab some entropy from how
-	//    long it took to grab the above entropy. And a yield, for good measure.
-	runtime.Gosched()
-	diffTime := time.Now().UnixNano() - startTime
-	diffTimeBytes := make([]byte, 32)
-	binary.PutVarint(diffTimeBytes, diffTime)
-	diffTimeHash := Sha3(diffTimeBytes)
-	mix32Byte(mainBuff, diffTimeHash)
-
-	return mainBuff
-}
-
-func mix32Byte(buff []byte, mixBuff []byte) []byte {
-	for i := 0; i < 32; i++ {
-		buff[i] ^= mixBuff[i]
-	}
-	return buff
-}
diff --git a/crypto/key_store_passphrase.go b/crypto/key_store_passphrase.go
index eaf73422f..1e7c50f96 100644
--- a/crypto/key_store_passphrase.go
+++ b/crypto/key_store_passphrase.go
@@ -69,9 +69,10 @@ import (
 	"code.google.com/p/go.crypto/scrypt"
 	"crypto/aes"
 	"crypto/cipher"
-	"encoding/hex"
+	crand "crypto/rand"
 	"encoding/json"
 	"errors"
+	"io"
 	"os"
 	"path"
 )
@@ -94,25 +95,24 @@ func NewKeyStorePassphrase(path string) KeyStore2 {
 	return ks
 }
 
-func (ks keyStorePassphrase) GenerateNewKey(auth string) (key *Key, err error) {
-	return GenerateNewKeyDefault(ks, auth)
+func (ks keyStorePassphrase) GenerateNewKey(rand io.Reader, auth string) (key *Key, err error) {
+	return GenerateNewKeyDefault(ks, rand, auth)
 }
 
 func (ks keyStorePassphrase) GetKey(keyId *uuid.UUID, auth string) (key *Key, err error) {
-	keyBytes, flags, err := DecryptKey(ks, keyId, auth)
+	keyBytes, err := DecryptKey(ks, keyId, auth)
 	if err != nil {
 		return nil, err
 	}
 	key = new(Key)
 	key.Id = keyId
-	copy(key.Flags[:], flags[0:4])
 	key.PrivateKey = ToECDSA(keyBytes)
 	return key, err
 }
 
 func (ks keyStorePassphrase) StoreKey(key *Key, auth string) (err error) {
 	authArray := []byte(auth)
-	salt := GetEntropyCSPRNG(32)
+	salt := getEntropyCSPRNG(32)
 	derivedKey, err := scrypt.Key(authArray, salt, scryptN, scryptr, scryptp, scryptdkLen)
 	if err != nil {
 		return err
@@ -127,19 +127,18 @@ func (ks keyStorePassphrase) StoreKey(key *Key, auth string) (err error) {
 		return err
 	}
 
-	iv := GetEntropyCSPRNG(aes.BlockSize) // 16
+	iv := getEntropyCSPRNG(aes.BlockSize) // 16
 	AES256CBCEncrypter := cipher.NewCBCEncrypter(AES256Block, iv)
 	cipherText := make([]byte, len(toEncrypt))
 	AES256CBCEncrypter.CryptBlocks(cipherText, toEncrypt)
 
-	cipherStruct := CipherJSON{
-		hex.EncodeToString(salt),
-		hex.EncodeToString(iv),
-		hex.EncodeToString(cipherText),
+	cipherStruct := cipherJSON{
+		salt,
+		iv,
+		cipherText,
 	}
-	keyStruct := EncryptedKeyJSON{
-		key.Id.String(),
-		hex.EncodeToString(key.Flags[:]),
+	keyStruct := encryptedKeyJSON{
+		*key.Id,
 		cipherStruct,
 	}
 	keyJSON, err := json.Marshal(keyStruct)
@@ -152,7 +151,7 @@ func (ks keyStorePassphrase) StoreKey(key *Key, auth string) (err error) {
 
 func (ks keyStorePassphrase) DeleteKey(keyId *uuid.UUID, auth string) (err error) {
 	// only delete if correct passphrase is given
-	_, _, err = DecryptKey(ks, keyId, auth)
+	_, err = DecryptKey(ks, keyId, auth)
 	if err != nil {
 		return err
 	}
@@ -161,44 +160,30 @@ func (ks keyStorePassphrase) DeleteKey(keyId *uuid.UUID, auth string) (err error
 	return os.RemoveAll(keyDirPath)
 }
 
-func DecryptKey(ks keyStorePassphrase, keyId *uuid.UUID, auth string) (keyBytes []byte, flags []byte, err error) {
+func DecryptKey(ks keyStorePassphrase, keyId *uuid.UUID, auth string) (keyBytes []byte, err error) {
 	fileContent, err := GetKeyFile(ks.keysDirPath, keyId)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
-	keyProtected := new(EncryptedKeyJSON)
+	keyProtected := new(encryptedKeyJSON)
 	err = json.Unmarshal(fileContent, keyProtected)
 
-	flags, err = hex.DecodeString(keyProtected.Flags)
-	if err != nil {
-		return nil, nil, err
-	}
+	salt := keyProtected.Crypto.Salt
 
-	salt, err := hex.DecodeString(keyProtected.Crypto.Salt)
-	if err != nil {
-		return nil, nil, err
-	}
+	iv := keyProtected.Crypto.IV
 
-	iv, err := hex.DecodeString(keyProtected.Crypto.IV)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	cipherText, err := hex.DecodeString(keyProtected.Crypto.CipherText)
-	if err != nil {
-		return nil, nil, err
-	}
+	cipherText := keyProtected.Crypto.CipherText
 
 	authArray := []byte(auth)
 	derivedKey, err := scrypt.Key(authArray, salt, scryptN, scryptr, scryptp, scryptdkLen)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	AES256Block, err := aes.NewCipher(derivedKey)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	AES256CBCDecrypter := cipher.NewCBCDecrypter(AES256Block, iv)
@@ -208,16 +193,26 @@ func DecryptKey(ks keyStorePassphrase, keyId *uuid.UUID, auth string) (keyBytes
 	plainText := PKCS7Unpad(paddedPlainText)
 	if plainText == nil {
 		err = errors.New("Decryption failed: PKCS7Unpad failed after decryption")
-		return nil, nil, err
+		return nil, err
 	}
 
 	keyBytes = plainText[:len(plainText)-32]
 	keyBytesHash := plainText[len(plainText)-32:]
 	if !bytes.Equal(Sha3(keyBytes), keyBytesHash) {
 		err = errors.New("Decryption failed: checksum mismatch")
-		return nil, nil, err
+		return nil, err
+	}
+	return keyBytes, err
+}
+
+// plain crypto/rand. this is /dev/urandom on Unix-like systems.
+func getEntropyCSPRNG(n int) []byte {
+	mainBuff := make([]byte, n)
+	_, err := io.ReadFull(crand.Reader, mainBuff)
+	if err != nil {
+		panic("key generation: reading from crypto/rand failed: " + err.Error())
 	}
-	return keyBytes, flags, err
+	return mainBuff
 }
 
 // From https://leanpub.com/gocrypto/read#leanpub-auto-block-cipher-modes
diff --git a/crypto/key_store_plain.go b/crypto/key_store_plain.go
index 00d9767b6..2aa813f5e 100644
--- a/crypto/key_store_plain.go
+++ b/crypto/key_store_plain.go
@@ -27,6 +27,7 @@ import (
 	"code.google.com/p/go-uuid/uuid"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"os/user"
@@ -35,7 +36,8 @@ import (
 
 // TODO: rename to KeyStore when replacing existing KeyStore
 type KeyStore2 interface {
-	GenerateNewKey(string) (*Key, error)     // create and store new key, optionally using auth string
+	// create new key using io.Reader entropy source and optionally using auth string
+	GenerateNewKey(io.Reader, string) (*Key, error)
 	GetKey(*uuid.UUID, string) (*Key, error) // key from id and auth string
 	StoreKey(*Key, string) error             // store key optionally using auth string
 	DeleteKey(*uuid.UUID, string) error      // delete key by id and auth string
@@ -57,17 +59,17 @@ func NewKeyStorePlain(path string) KeyStore2 {
 	return ks
 }
 
-func (ks keyStorePlain) GenerateNewKey(auth string) (key *Key, err error) {
-	return GenerateNewKeyDefault(ks, auth)
+func (ks keyStorePlain) GenerateNewKey(rand io.Reader, auth string) (key *Key, err error) {
+	return GenerateNewKeyDefault(ks, rand, auth)
 }
 
-func GenerateNewKeyDefault(ks KeyStore2, auth string) (key *Key, err error) {
+func GenerateNewKeyDefault(ks KeyStore2, rand io.Reader, auth string) (key *Key, err error) {
 	defer func() {
 		if r := recover(); r != nil {
 			err = fmt.Errorf("GenerateNewKey error: %v", r)
 		}
 	}()
-	key = NewKey()
+	key = NewKey(rand)
 	err = ks.StoreKey(key, auth)
 	return key, err
 }
diff --git a/crypto/key_store_test.go b/crypto/key_store_test.go
index 8469d2369..16c0e476d 100644
--- a/crypto/key_store_test.go
+++ b/crypto/key_store_test.go
@@ -1,7 +1,7 @@
 package crypto
 
 import (
-	"fmt"
+	crand "crypto/rand"
 	"reflect"
 	"testing"
 )
@@ -9,107 +9,77 @@ import (
 func TestKeyStorePlain(t *testing.T) {
 	ks := NewKeyStorePlain(DefaultDataDir())
 	pass := "" // not used but required by API
-	k1, err := ks.GenerateNewKey(pass)
+	k1, err := ks.GenerateNewKey(crand.Reader, pass)
 	if err != nil {
-		t.Error(err)
-		t.FailNow()
+		t.Fatal(err)
 	}
 
 	k2 := new(Key)
 	k2, err = ks.GetKey(k1.Id, pass)
 	if err != nil {
-		t.Error(err)
-		t.FailNow()
+		t.Fatal(err)
 	}
 
 	if !reflect.DeepEqual(k1.Id, k2.Id) {
-		fmt.Println("key Id mismatch")
-		t.FailNow()
-	}
-
-	if k1.Flags != k2.Flags {
-		fmt.Println("key Flags mismatch")
-		t.FailNow()
+		t.Fatal(err)
 	}
 
 	if !reflect.DeepEqual(k1.PrivateKey, k2.PrivateKey) {
-		fmt.Println("key PrivateKey mismatch")
-		t.FailNow()
+		t.Fatal(err)
 	}
 
 	err = ks.DeleteKey(k2.Id, pass)
 	if err != nil {
-		t.Error(err)
-		t.FailNow()
+		t.Fatal(err)
 	}
 }
 
 func TestKeyStorePassphrase(t *testing.T) {
 	ks := NewKeyStorePassphrase(DefaultDataDir())
 	pass := "foo"
-	k1, err := ks.GenerateNewKey(pass)
+	k1, err := ks.GenerateNewKey(crand.Reader, pass)
 	if err != nil {
-		t.Error(err)
-		t.FailNow()
+		t.Fatal(err)
 	}
-
 	k2 := new(Key)
 	k2, err = ks.GetKey(k1.Id, pass)
 	if err != nil {
-		t.Error(err)
-		t.FailNow()
+		t.Fatal(err)
 	}
-
 	if !reflect.DeepEqual(k1.Id, k2.Id) {
-		fmt.Println("key Id mismatch")
-		t.FailNow()
-	}
-
-	if k1.Flags != k2.Flags {
-		fmt.Println("key Flags mismatch")
-		t.FailNow()
+		t.Fatal(err)
 	}
 
 	if !reflect.DeepEqual(k1.PrivateKey, k2.PrivateKey) {
-		fmt.Println("key PrivateKey mismatch")
-		t.FailNow()
+		t.Fatal(err)
 	}
 
 	err = ks.DeleteKey(k2.Id, pass) // also to clean up created files
 	if err != nil {
-		t.Error(err)
-		t.FailNow()
+		t.Fatal(err)
 	}
 }
 
 func TestKeyStorePassphraseDecryptionFail(t *testing.T) {
 	ks := NewKeyStorePassphrase(DefaultDataDir())
 	pass := "foo"
-	k1, err := ks.GenerateNewKey(pass)
+	k1, err := ks.GenerateNewKey(crand.Reader, pass)
 	if err != nil {
-		t.Error(err)
-		t.FailNow()
+		t.Fatal(err)
 	}
 
 	_, err = ks.GetKey(k1.Id, "bar") // wrong passphrase
-	// t.Error(err)
 	if err == nil {
-		t.FailNow()
+		t.Fatal(err)
 	}
 
 	err = ks.DeleteKey(k1.Id, "bar") // wrong passphrase
 	if err == nil {
-		t.Error(err)
-		t.FailNow()
+		t.Fatal(err)
 	}
 
 	err = ks.DeleteKey(k1.Id, pass) // to clean up
 	if err != nil {
-		t.Error(err)
-		t.FailNow()
+		t.Fatal(err)
 	}
 }
-
-func TestKeyMixedEntropy(t *testing.T) {
-	GetEntropyTinFoilHat()
-}