Merge PR #3638: Document and justify bcrypt parameters

PENDING.md

@@ -35,6 +35,8 @@ IMPROVEMENTS
 
 * SDK
 
+  * \#3638 Add Bcrypt benchmarks & justification of security parameter choice
+
 * Tendermint
   * [\#3618] Upgrade to Tendermint 0.30.03
 

crypto/keys/mintkey/README.md

@@ -0,0 +1,36 @@
+Security parameter choice
+-------------------------
+
+The present Bcrypt security parameter used is 12, which should take about a quarter of a second on midrange consumer hardware (see [Benchmarking](#benchmarking) section below).
+
+For some background into security parameter considerations, see [here](https://auth0.com/blog/hashing-in-action-understanding-bcrypt/) and [here](https://security.stackexchange.com/questions/3959/recommended-of-iterations-when-using-pkbdf2-sha256/3993#3993).
+
+Given our security model, where an attacker would need to already have access to a victim's computer and copy the `~/.gaiacli` directory (as opposed to e.g. web authentication), this parameter choice seems sufficient for the time being. Bcrypt always generates a 448-bit key, so the security in practice is determined by the length & complexity of a user's password and the time taken to generate a Bcrypt key from their password (which we can choose with the security parameter). Users would be well-advised to use difficult-to-guess passwords.
+
+Benchmarking
+------------
+
+To run Bcrypt benchmarks:
+
+```bash
+go test -v --bench github.com/cosmos/cosmos-sdk/crypto/keys/mintkey
+```
+
+On the test machine (midrange ThinkPad; i7 6600U), this results in:
+
+```bash
+goos: linux
+goarch: amd64
+pkg: github.com/cosmos/cosmos-sdk/crypto/keys/mintkey
+BenchmarkBcryptGenerateFromPassword/benchmark-security-param-9-4         	      50	  34609268 ns/op
+BenchmarkBcryptGenerateFromPassword/benchmark-security-param-10-4        	      20	  67874471 ns/op
+BenchmarkBcryptGenerateFromPassword/benchmark-security-param-11-4        	      10	 135515404 ns/op
+BenchmarkBcryptGenerateFromPassword/benchmark-security-param-12-4        	       5	 274824600 ns/op
+BenchmarkBcryptGenerateFromPassword/benchmark-security-param-13-4        	       2	 547012903 ns/op
+BenchmarkBcryptGenerateFromPassword/benchmark-security-param-14-4        	       1	1083685904 ns/op
+BenchmarkBcryptGenerateFromPassword/benchmark-security-param-15-4        	       1	2183674041 ns/op
+PASS
+ok  	github.com/cosmos/cosmos-sdk/crypto/keys/mintkey	12.093s
+```
+
+Benchmark results are in nanoseconds, so security parameter 12 takes about a quarter of a second to generate the Bcrypt key, security param 13 takes half a second, and so on.

crypto/keys/mintkey/mintkey.go

@@ -34,7 +34,7 @@ const (
 // 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
+// For further notes on security parameter choice, see README.md
 var BcryptSecurityParameter = 12
 
 //-----------------------------------------------------------------

crypto/keys/mintkey/mintkey_bench_test.go

@@ -0,0 +1,26 @@
+package mintkey
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/bcrypt"
+
+	"github.com/tendermint/tendermint/crypto"
+)
+
+func BenchmarkBcryptGenerateFromPassword(b *testing.B) {
+	passphrase := []byte("passphrase")
+	for securityParam := 9; securityParam < 16; securityParam++ {
+		param := securityParam
+		b.Run(fmt.Sprintf("benchmark-security-param-%d", param), func(b *testing.B) {
+			saltBytes := crypto.CRandBytes(16)
+			b.ResetTimer()
+			for i := 0; i < b.N; i++ {
+				_, err := bcrypt.GenerateFromPassword(saltBytes, passphrase, param)
+				require.Nil(b, err)
+			}
+		})
+	}
+}