Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R4R: Bcrypt investigations / benchmarks #3638

Merged
merged 4 commits into from
Feb 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ IMPROVEMENTS

* SDK

* \#3638 Add Bcrypt benchmarks & justification of security parameter choice

* Tendermint
* [\#3618] Upgrade to Tendermint 0.30.03

Expand Down
36 changes: 36 additions & 0 deletions crypto/keys/mintkey/README.md
Original file line number Diff line number Diff line change
@@ -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).
cwgoes marked this conversation as resolved.
Show resolved Hide resolved

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.
2 changes: 1 addition & 1 deletion crypto/keys/mintkey/mintkey.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

//-----------------------------------------------------------------
Expand Down
26 changes: 26 additions & 0 deletions crypto/keys/mintkey/mintkey_bench_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}