From abafd8bf9dbb7f92a428c7829e8635950c545a20 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Wed, 13 Feb 2019 15:26:43 +0100 Subject: [PATCH 1/4] Add Bcrypt benchmarks --- PENDING.md | 2 ++ crypto/keys/mintkey/README.md | 24 +++++++++++++++++++++ crypto/keys/mintkey/mintkey_bench_test.go | 26 +++++++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 crypto/keys/mintkey/README.md create mode 100644 crypto/keys/mintkey/mintkey_bench_test.go diff --git a/PENDING.md b/PENDING.md index a41bbe0ca..3a1e1a1ae 100644 --- a/PENDING.md +++ b/PENDING.md @@ -35,6 +35,8 @@ IMPROVEMENTS * SDK + * \# Add Bcrypt benchmarks & justification of security parameter choice + * Tendermint * [\#3618] Upgrade to Tendermint 0.30.03 diff --git a/crypto/keys/mintkey/README.md b/crypto/keys/mintkey/README.md new file mode 100644 index 000000000..38da0fbe7 --- /dev/null +++ b/crypto/keys/mintkey/README.md @@ -0,0 +1,24 @@ +To run Bcrypt benchmarks: + +```bash +go test -bench . +``` + +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. diff --git a/crypto/keys/mintkey/mintkey_bench_test.go b/crypto/keys/mintkey/mintkey_bench_test.go new file mode 100644 index 000000000..fbc3e58dd --- /dev/null +++ b/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) + } + }) + } +} From 4f5ad4f1edb9bfd1fbf7f064efd5e1da20273beb Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Wed, 13 Feb 2019 15:28:09 +0100 Subject: [PATCH 2/4] Add PR number to PENDING.md --- PENDING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PENDING.md b/PENDING.md index 3a1e1a1ae..30f1048c5 100644 --- a/PENDING.md +++ b/PENDING.md @@ -35,7 +35,7 @@ IMPROVEMENTS * SDK - * \# Add Bcrypt benchmarks & justification of security parameter choice + * \#3638 Add Bcrypt benchmarks & justification of security parameter choice * Tendermint * [\#3618] Upgrade to Tendermint 0.30.03 From d21ab4efc546bb081324ea942a8c2e5336ac291e Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Wed, 13 Feb 2019 15:36:03 +0100 Subject: [PATCH 3/4] Add justification on security parameter choice --- crypto/keys/mintkey/README.md | 12 ++++++++++++ crypto/keys/mintkey/mintkey.go | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crypto/keys/mintkey/README.md b/crypto/keys/mintkey/README.md index 38da0fbe7..ca3d4c21c 100644 --- a/crypto/keys/mintkey/README.md +++ b/crypto/keys/mintkey/README.md @@ -1,3 +1,15 @@ +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. + +Benchmarking +------------ + To run Bcrypt benchmarks: ```bash diff --git a/crypto/keys/mintkey/mintkey.go b/crypto/keys/mintkey/mintkey.go index 3b06415e2..11743013a 100644 --- a/crypto/keys/mintkey/mintkey.go +++ b/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 //----------------------------------------------------------------- From bb1fd58b1e8b5cec202e7c5e8148502b846bbe0d Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Wed, 13 Feb 2019 17:26:14 +0100 Subject: [PATCH 4/4] Address @alexanderbez comments --- crypto/keys/mintkey/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/keys/mintkey/README.md b/crypto/keys/mintkey/README.md index ca3d4c21c..5180e50bd 100644 --- a/crypto/keys/mintkey/README.md +++ b/crypto/keys/mintkey/README.md @@ -5,7 +5,7 @@ The present Bcrypt security parameter used is 12, which should take about a quar 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. +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 ------------ @@ -13,7 +13,7 @@ Benchmarking To run Bcrypt benchmarks: ```bash -go test -bench . +go test -v --bench github.com/cosmos/cosmos-sdk/crypto/keys/mintkey ``` On the test machine (midrange ThinkPad; i7 6600U), this results in: