Merge pull request #9687 from jxuan/config-bcryptcost

*: make bcrypt-cost configurable

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

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
+}

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 {

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*

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
 

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,

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.")

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'

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.

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 {

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) {