Merge PR #1449: crypto/keys: make bcrypt security param a var

This is done so that the time spent on bcrypt during test cases
can be reduced. This change reduces the amount of time lcd tests
spend on bcrypt from 76% to 40%. (We need to reduce the number of
calls to bcrypt in a seperate PR, along with fixing other sources
of slowness)

Making the bcrypt security parameter a var shouldn't be a security issue:
One can't verify an invalid key by maliciously changing the bcrypt
parameter during a runtime vulnerability. The main security
threat this then exposes would be something that changes this during
runtime before the user creates their key. This vulnerability must
succeed to update this to that same value before every subsequent call
to gaiacli keys in future startups / or the attacker must get access
to the filesystem. However, with this same threat model (changing
variables in runtime), one can cause the user to sign a different tx
than what they see, which is a significantly cheaper attack then breaking
a bcrypt hash. (Recall that the nonce still exists to break rainbow
tables)

client/lcd/lcd_test.go

@@ -29,6 +29,10 @@ import (
 	stakerest "github.com/cosmos/cosmos-sdk/x/stake/client/rest"
 )
 
+func init() {
+	cryptoKeys.BcryptSecurityParameter = 1
+}
+
 func TestKeys(t *testing.T) {
 	name, password := "test", "1234567890"
 	addr, seed := CreateAddr(t, "test", password, GetKB(t))

crypto/keys/mintkey.go

@@ -16,6 +16,21 @@ const (
 	blockTypePubKey  = "TENDERMINT PUBLIC KEY"
 )
 
+// Make bcrypt security parameter var, so it can be changed within the lcd test
+// Making the bcrypt security parameter a var shouldn't be a security issue:
+// One can't verify an invalid key by maliciously changing the bcrypt
+// parameter during a runtime vulnerability. The main security
+// threat this then exposes would be something that changes this during
+// runtime before the user creates their key. This vulnerability must
+// succeed to update this to that same value before every subsequent call
+// to gaiacli keys in future startups / or the attacker must get access
+// to the filesystem. However, with a similar threat model (changing
+// variables in runtime), one can cause the user to sign a different tx
+// than what they see, which is a significantly cheaper attack then breaking
+// a bcrypt hash. (Recall that the nonce still exists to break rainbow tables)
+// TODO: Consider increasing default
+var BcryptSecurityParameter = 12
+
 func armorInfoBytes(bz []byte) string {
 	return armorBytes(bz, blockTypeKeyInfo)
 }
@@ -91,7 +106,7 @@ func unarmorDecryptPrivKey(armorStr string, passphrase string) (crypto.PrivKey,
 
 func encryptPrivKey(privKey crypto.PrivKey, passphrase string) (saltBytes []byte, encBytes []byte) {
 	saltBytes = crypto.CRandBytes(16)
-	key, err := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), 12) // TODO parameterize.  12 is good today (2016)
+	key, err := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), BcryptSecurityParameter)
 	if err != nil {
 		cmn.Exit("Error generating bcrypt key from passphrase: " + err.Error())
 	}
@@ -101,7 +116,7 @@ func encryptPrivKey(privKey crypto.PrivKey, passphrase string) (saltBytes []byte
 }
 
 func decryptPrivKey(saltBytes []byte, encBytes []byte, passphrase string) (privKey crypto.PrivKey, err error) {
-	key, err := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), 12) // TODO parameterize.  12 is good today (2016)
+	key, err := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), BcryptSecurityParameter)
 	if err != nil {
 		cmn.Exit("Error generating bcrypt key from passphrase: " + err.Error())
 	}