From bf432648aed793834627670e4373ccaf15a8b945 Mon Sep 17 00:00:00 2001 From: Jiang Xuan Date: Thu, 3 May 2018 11:43:32 -0700 Subject: [PATCH] *: make bcrypt-cost configurable --- Documentation/op-guide/configuration.md | 4 +++ auth/store.go | 31 ++++++++++++++---- auth/store_test.go | 42 ++++++++++++++++++------- clientv3/main_test.go | 5 --- embed/config.go | 7 +++-- embed/etcd.go | 1 + etcdmain/config.go | 1 + etcdmain/help.go | 4 +++ etcdserver/config.go | 3 +- etcdserver/server.go | 2 +- integration/cluster.go | 4 ++- 11 files changed, 77 insertions(+), 27 deletions(-) diff --git a/Documentation/op-guide/configuration.md b/Documentation/op-guide/configuration.md index 8eceb4954..837a84e01 100644 --- a/Documentation/op-guide/configuration.md +++ b/Documentation/op-guide/configuration.md @@ -375,6 +375,10 @@ Follow the instructions when using these flags. + Example option of JWT: '--auth-token jwt,pub-key=app.rsa.pub,priv-key=app.rsa,sign-method=RS512,ttl=10m' + default: "simple" +### --bcrypt-cost ++ Specify the cost / strength of the bcrypt algorithm for hashing auth passwords. Valid values are between 4 and 31. ++ default: 10 + ## Experimental flags ### --experimental-corrupt-check-time diff --git a/auth/store.go b/auth/store.go index a18d5fa52..3f305a1a0 100644 --- a/auth/store.go +++ b/auth/store.go @@ -66,9 +66,6 @@ var ( ErrInvalidAuthToken = errors.New("auth: invalid auth token") ErrInvalidAuthOpts = errors.New("auth: invalid auth options") ErrInvalidAuthMgmt = errors.New("auth: invalid auth management") - - // BcryptCost is the algorithm cost / strength for hashing auth passwords - BcryptCost = bcrypt.DefaultCost ) const ( @@ -205,6 +202,7 @@ type authStore struct { rangePermCache map[string]*unifiedRangePermissions // username -> unifiedRangePermissions tokenProvider TokenProvider + bcryptCost int // the algorithm cost / strength for hashing auth passwords } func (as *authStore) AuthEnable() error { @@ -371,7 +369,7 @@ func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse, return nil, ErrUserEmpty } - hashed, err := bcrypt.GenerateFromPassword([]byte(r.Password), BcryptCost) + hashed, err := bcrypt.GenerateFromPassword([]byte(r.Password), as.bcryptCost) if err != nil { if as.lg != nil { as.lg.Warn( @@ -452,7 +450,7 @@ func (as *authStore) UserDelete(r *pb.AuthUserDeleteRequest) (*pb.AuthUserDelete func (as *authStore) UserChangePassword(r *pb.AuthUserChangePasswordRequest) (*pb.AuthUserChangePasswordResponse, error) { // TODO(mitake): measure the cost of bcrypt.GenerateFromPassword() // If the cost is too high, we should move the encryption to outside of the raft - hashed, err := bcrypt.GenerateFromPassword([]byte(r.Password), BcryptCost) + hashed, err := bcrypt.GenerateFromPassword([]byte(r.Password), as.bcryptCost) if err != nil { if as.lg != nil { as.lg.Warn( @@ -1060,7 +1058,23 @@ func (as *authStore) IsAuthEnabled() bool { } // NewAuthStore creates a new AuthStore. -func NewAuthStore(lg *zap.Logger, be backend.Backend, tp TokenProvider) *authStore { +func NewAuthStore(lg *zap.Logger, be backend.Backend, tp TokenProvider, bcryptCost int) *authStore { + if bcryptCost < bcrypt.MinCost || bcryptCost > bcrypt.MaxCost { + if lg != nil { + lg.Warn( + "use default bcrypt cost instead of the invalid given cost", + zap.Int("min-cost", bcrypt.MinCost), + zap.Int("max-cost", bcrypt.MaxCost), + zap.Int("default-cost", bcrypt.DefaultCost), + zap.Int("given-cost", bcryptCost)) + } else { + plog.Warningf("Use default bcrypt-cost %d instead of the invalid value %d", + bcrypt.DefaultCost, bcryptCost) + } + + bcryptCost = bcrypt.DefaultCost + } + tx := be.BatchTx() tx.Lock() @@ -1083,6 +1097,7 @@ func NewAuthStore(lg *zap.Logger, be backend.Backend, tp TokenProvider) *authSto enabled: enabled, rangePermCache: make(map[string]*unifiedRangePermissions), tokenProvider: tp, + bcryptCost: bcryptCost, } if enabled { @@ -1342,3 +1357,7 @@ func (as *authStore) HasRole(user, role string) bool { } return false } + +func (as *authStore) BcryptCost() int { + return as.bcryptCost +} diff --git a/auth/store_test.go b/auth/store_test.go index 73a439507..4a459232a 100644 --- a/auth/store_test.go +++ b/auth/store_test.go @@ -34,8 +34,6 @@ import ( "google.golang.org/grpc/metadata" ) -func init() { BcryptCost = bcrypt.MinCost } - func dummyIndexWaiter(index uint64) <-chan struct{} { ch := make(chan struct{}) go func() { @@ -54,27 +52,49 @@ func TestNewAuthStoreRevision(t *testing.T) { if err != nil { t.Fatal(err) } - as := NewAuthStore(zap.NewExample(), b, tp) + as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost) err = enableAuthAndCreateRoot(as) if err != nil { t.Fatal(err) } old := as.Revision() - b.Close() as.Close() + b.Close() // no changes to commit b2 := backend.NewDefaultBackend(tPath) - as = NewAuthStore(zap.NewExample(), b2, tp) + as = NewAuthStore(zap.NewExample(), b2, tp, bcrypt.MinCost) new := as.Revision() - b2.Close() as.Close() + b2.Close() if old != new { t.Fatalf("expected revision %d, got %d", old, new) } } +// TestNewAuthStoreBryptCost ensures that NewAuthStore uses default when given bcrypt-cost is invalid +func TestNewAuthStoreBcryptCost(t *testing.T) { + b, tPath := backend.NewDefaultTmpBackend() + defer os.Remove(tPath) + + tp, err := NewTokenProvider(zap.NewExample(), "simple", dummyIndexWaiter) + if err != nil { + t.Fatal(err) + } + + invalidCosts := [2]int{bcrypt.MinCost - 1, bcrypt.MaxCost + 1} + for _, invalidCost := range invalidCosts { + as := NewAuthStore(zap.NewExample(), b, tp, invalidCost) + if as.BcryptCost() != bcrypt.DefaultCost { + t.Fatalf("expected DefaultCost when bcryptcost is invalid") + } + as.Close() + } + + b.Close() +} + func setupAuthStore(t *testing.T) (store *authStore, teardownfunc func(t *testing.T)) { b, tPath := backend.NewDefaultTmpBackend() @@ -82,7 +102,7 @@ func setupAuthStore(t *testing.T) (store *authStore, teardownfunc func(t *testin if err != nil { t.Fatal(err) } - as := NewAuthStore(zap.NewExample(), b, tp) + as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost) err = enableAuthAndCreateRoot(as) if err != nil { t.Fatal(err) @@ -519,7 +539,7 @@ func TestAuthInfoFromCtxRace(t *testing.T) { if err != nil { t.Fatal(err) } - as := NewAuthStore(zap.NewExample(), b, tp) + as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost) defer as.Close() donec := make(chan struct{}) @@ -585,7 +605,7 @@ func TestRecoverFromSnapshot(t *testing.T) { if err != nil { t.Fatal(err) } - as2 := NewAuthStore(zap.NewExample(), as.be, tp) + as2 := NewAuthStore(zap.NewExample(), as.be, tp, bcrypt.MinCost) defer func(a *authStore) { a.Close() }(as2) @@ -667,7 +687,7 @@ func TestRolesOrder(t *testing.T) { if err != nil { t.Fatal(err) } - as := NewAuthStore(zap.NewExample(), b, tp) + as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost) err = enableAuthAndCreateRoot(as) if err != nil { t.Fatal(err) @@ -713,7 +733,7 @@ func TestAuthInfoFromCtxWithRoot(t *testing.T) { if err != nil { t.Fatal(err) } - as := NewAuthStore(zap.NewExample(), b, tp) + as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost) defer as.Close() if err = enableAuthAndCreateRoot(as); err != nil { diff --git a/clientv3/main_test.go b/clientv3/main_test.go index 3c525241d..a0a21d275 100644 --- a/clientv3/main_test.go +++ b/clientv3/main_test.go @@ -22,15 +22,10 @@ import ( "testing" "time" - "github.com/coreos/etcd/auth" "github.com/coreos/etcd/integration" "github.com/coreos/etcd/pkg/testutil" - - "golang.org/x/crypto/bcrypt" ) -func init() { auth.BcryptCost = bcrypt.MinCost } - // TestMain sets up an etcd cluster if running the examples. func TestMain(m *testing.M) { useCluster, hasRunArg := false, false // default to running only Test* diff --git a/embed/config.go b/embed/config.go index 23d56ad63..64eb02995 100644 --- a/embed/config.go +++ b/embed/config.go @@ -38,6 +38,7 @@ import ( "github.com/ghodss/yaml" "go.uber.org/zap" "go.uber.org/zap/zapcore" + "golang.org/x/crypto/bcrypt" "google.golang.org/grpc" ) @@ -242,7 +243,8 @@ type Config struct { // embed.StartEtcd(cfg) ServiceRegister func(*grpc.Server) `json:"-"` - AuthToken string `json:"auth-token"` + AuthToken string `json:"auth-token"` + BcryptCost uint `json:"bcrypt-cost"` ExperimentalInitialCorruptCheck bool `json:"experimental-initial-corrupt-check"` ExperimentalCorruptCheckTime time.Duration `json:"experimental-corrupt-check-time"` @@ -364,7 +366,8 @@ func NewConfig() *Config { CORS: map[string]struct{}{"*": {}}, HostWhitelist: map[string]struct{}{"*": {}}, - AuthToken: "simple", + AuthToken: "simple", + BcryptCost: uint(bcrypt.DefaultCost), PreVote: false, // TODO: enable by default in v3.5 diff --git a/embed/etcd.go b/embed/etcd.go index 397516ea0..50c8f283e 100644 --- a/embed/etcd.go +++ b/embed/etcd.go @@ -183,6 +183,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { StrictReconfigCheck: cfg.StrictReconfigCheck, ClientCertAuthEnabled: cfg.ClientTLSInfo.ClientCertAuth, AuthToken: cfg.AuthToken, + BcryptCost: cfg.BcryptCost, CORS: cfg.CORS, HostWhitelist: cfg.HostWhitelist, InitialCorruptCheck: cfg.ExperimentalInitialCorruptCheck, diff --git a/etcdmain/config.go b/etcdmain/config.go index 2637586fe..c969e8a16 100644 --- a/etcdmain/config.go +++ b/etcdmain/config.go @@ -237,6 +237,7 @@ func newConfig() *config { // auth fs.StringVar(&cfg.ec.AuthToken, "auth-token", cfg.ec.AuthToken, "Specify auth token specific options.") + fs.UintVar(&cfg.ec.BcryptCost, "bcrypt-cost", cfg.ec.BcryptCost, "Specify bcrypt algorithm cost factor for auth password hashing.") // experimental fs.BoolVar(&cfg.ec.ExperimentalInitialCorruptCheck, "experimental-initial-corrupt-check", cfg.ec.ExperimentalInitialCorruptCheck, "Enable to check data corruption before serving any client/peer traffic.") diff --git a/etcdmain/help.go b/etcdmain/help.go index 5ce256d91..5a93874ce 100644 --- a/etcdmain/help.go +++ b/etcdmain/help.go @@ -15,9 +15,11 @@ package etcdmain import ( + "fmt" "strconv" "github.com/coreos/etcd/embed" + "golang.org/x/crypto/bcrypt" ) var ( @@ -148,6 +150,8 @@ Security: Auth: --auth-token 'simple' Specify a v3 authentication token type and its options ('simple' or 'jwt'). + --bcrypt-cost ` + fmt.Sprintf("%d", bcrypt.DefaultCost) + ` + Specify the cost / strength of the bcrypt algorithm for hashing auth passwords. Valid values are between ` + fmt.Sprintf("%d", bcrypt.MinCost) + ` and ` + fmt.Sprintf("%d", bcrypt.MaxCost) + `. Profiling and Monitoring: --enable-pprof 'false' diff --git a/etcdserver/config.go b/etcdserver/config.go index a9ef6d288..767b6c3a0 100644 --- a/etcdserver/config.go +++ b/etcdserver/config.go @@ -103,7 +103,8 @@ type ServerConfig struct { // ClientCertAuthEnabled is true when cert has been signed by the client CA. ClientCertAuthEnabled bool - AuthToken string + AuthToken string + BcryptCost uint // InitialCorruptCheck is true to check data corruption on boot // before serving any peer/client traffic. diff --git a/etcdserver/server.go b/etcdserver/server.go index eb2ce8616..b1d12efd8 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -559,7 +559,7 @@ func NewServer(cfg ServerConfig) (srv *EtcdServer, err error) { } return nil, err } - srv.authStore = auth.NewAuthStore(srv.getLogger(), srv.be, tp) + srv.authStore = auth.NewAuthStore(srv.getLogger(), srv.be, tp, int(cfg.BcryptCost)) if num := cfg.AutoCompactionRetention; num != 0 { srv.compactor, err = compactor.New(cfg.Logger, cfg.AutoCompactionMode, num, srv.kv, srv) if err != nil { diff --git a/integration/cluster.go b/integration/cluster.go index 65afa0568..6e03a4ae2 100644 --- a/integration/cluster.go +++ b/integration/cluster.go @@ -54,6 +54,7 @@ import ( "github.com/soheilhy/cmux" "go.uber.org/zap" + "golang.org/x/crypto/bcrypt" "google.golang.org/grpc" "google.golang.org/grpc/grpclog" "google.golang.org/grpc/keepalive" @@ -611,7 +612,8 @@ func mustNewMember(t *testing.T, mcfg memberConfig) *member { if m.MaxRequestBytes == 0 { m.MaxRequestBytes = embed.DefaultMaxRequestBytes } - m.AuthToken = "simple" // for the purpose of integration testing, simple token is enough + m.AuthToken = "simple" // for the purpose of integration testing, simple token is enough + m.BcryptCost = uint(bcrypt.MinCost) // use min bcrypt cost to speedy up integration testing m.grpcServerOpts = []grpc.ServerOption{} if mcfg.grpcKeepAliveMinTime > time.Duration(0) {