Skip to content

Commit

Permalink
*: support creating a user without password
Browse files Browse the repository at this point in the history
This commit adds a feature for creating a user without password. The
purpose of the feature is reducing attack surface by configuring bad
passwords (CN based auth will be allowed for the user).

The feature can be used with `--no-password` of `etcdctl user add`
command.

Fix #9590
  • Loading branch information
mitake committed Jun 13, 2018
1 parent bb744f6 commit 76a93df
Show file tree
Hide file tree
Showing 11 changed files with 399 additions and 295 deletions.
2 changes: 2 additions & 0 deletions Documentation/dev-guide/api_reference_v3.md
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ Empty field.
| ----- | ----------- | ---- |
| name | | string |
| password | | string |
| no_password | | bool |



Expand Down Expand Up @@ -945,6 +946,7 @@ User is a single entry in the bucket authUsers
| name | | bytes |
| password | | bytes |
| roles | | (slice of) string |
| no_password | | bool |



4 changes: 4 additions & 0 deletions Documentation/dev-guide/apispec/swagger/rpc.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1393,6 +1393,10 @@
"name": {
"type": "string"
},
"no_password": {
"type": "boolean",
"format": "boolean"
},
"password": {
"type": "string"
}
Expand Down
79 changes: 57 additions & 22 deletions auth/authpb/auth.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions auth/authpb/auth.proto
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ message User {
bytes name = 1;
bytes password = 2;
repeated string roles = 3;
bool no_password = 4;
}

// Permission is a single entity
Expand Down
40 changes: 27 additions & 13 deletions auth/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,10 @@ func (as *authStore) Authenticate(ctx context.Context, username, password string
return nil, ErrAuthFailed
}

if user.NoPassword {
return nil, ErrAuthFailed
}

// Password checking is already performed in the API layer, so we don't need to check for now.
// Staleness of password can be detected with OCC in the API layer, too.

Expand Down Expand Up @@ -335,6 +339,10 @@ func (as *authStore) CheckPassword(username, password string) (uint64, error) {
return 0, ErrAuthFailed
}

if user.NoPassword {
return 0, ErrAuthFailed
}

if bcrypt.CompareHashAndPassword(user.Password, []byte(password)) != nil {
if as.lg != nil {
as.lg.Info("invalid password", zap.String("user-name", username))
Expand Down Expand Up @@ -372,18 +380,23 @@ func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse,
return nil, ErrUserEmpty
}

hashed, err := bcrypt.GenerateFromPassword([]byte(r.Password), as.bcryptCost)
if err != nil {
if as.lg != nil {
as.lg.Warn(
"failed to bcrypt hash password",
zap.String("user-name", r.Name),
zap.Error(err),
)
} else {
plog.Errorf("failed to hash password: %s", err)
hashed := make([]byte, 0)
var err error

if !r.NoPassword {
hashed, err = bcrypt.GenerateFromPassword([]byte(r.Password), as.bcryptCost)
if err != nil {
if as.lg != nil {
as.lg.Warn(
"failed to bcrypt hash password",
zap.String("user-name", r.Name),
zap.Error(err),
)
} else {
plog.Errorf("failed to hash password: %s", err)
}
return nil, err
}
return nil, err
}

tx := as.be.BatchTx()
Expand All @@ -396,8 +409,9 @@ func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse,
}

newUser := &authpb.User{
Name: []byte(r.Name),
Password: hashed,
Name: []byte(r.Name),
Password: hashed,
NoPassword: r.NoPassword,
}

putUser(as.lg, tx, newUser)
Expand Down
6 changes: 3 additions & 3 deletions clientv3/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ type Auth interface {
AuthDisable(ctx context.Context) (*AuthDisableResponse, error)

// UserAdd adds a new user to an etcd cluster.
UserAdd(ctx context.Context, name string, password string) (*AuthUserAddResponse, error)
UserAdd(ctx context.Context, name string, password string, noPassword bool) (*AuthUserAddResponse, error)

// UserDelete deletes a user from an etcd cluster.
UserDelete(ctx context.Context, name string) (*AuthUserDeleteResponse, error)
Expand Down Expand Up @@ -123,8 +123,8 @@ func (auth *authClient) AuthDisable(ctx context.Context) (*AuthDisableResponse,
return (*AuthDisableResponse)(resp), toErr(ctx, err)
}

func (auth *authClient) UserAdd(ctx context.Context, name string, password string) (*AuthUserAddResponse, error) {
resp, err := auth.remote.UserAdd(ctx, &pb.AuthUserAddRequest{Name: name, Password: password}, auth.callOpts...)
func (auth *authClient) UserAdd(ctx context.Context, name string, password string, noPassword bool) (*AuthUserAddResponse, error) {
resp, err := auth.remote.UserAdd(ctx, &pb.AuthUserAddRequest{Name: name, Password: password, NoPassword: noPassword}, auth.callOpts...)
return (*AuthUserAddResponse)(resp), toErr(ctx, err)
}

Expand Down
4 changes: 2 additions & 2 deletions clientv3/example_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func ExampleAuth() {
if _, err = cli.RoleAdd(context.TODO(), "root"); err != nil {
log.Fatal(err)
}
if _, err = cli.UserAdd(context.TODO(), "root", "123"); err != nil {
if _, err = cli.UserAdd(context.TODO(), "root", "123", true); err != nil {
log.Fatal(err)
}
if _, err = cli.UserGrantRole(context.TODO(), "root", "root"); err != nil {
Expand All @@ -55,7 +55,7 @@ func ExampleAuth() {
); err != nil {
log.Fatal(err)
}
if _, err = cli.UserAdd(context.TODO(), "u", "123"); err != nil {
if _, err = cli.UserAdd(context.TODO(), "u", "123", true); err != nil {
log.Fatal(err)
}
if _, err = cli.UserGrantRole(context.TODO(), "u", "r"); err != nil {
Expand Down
38 changes: 22 additions & 16 deletions etcdctl/ctlv3/command/user_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func NewUserCommand() *cobra.Command {
var (
passwordInteractive bool
passwordFromFlag string
noPassword bool
)

func newUserAddCommand() *cobra.Command {
Expand All @@ -59,6 +60,7 @@ func newUserAddCommand() *cobra.Command {

cmd.Flags().BoolVar(&passwordInteractive, "interactive", true, "Read password from stdin instead of interactive terminal")
cmd.Flags().StringVar(&passwordFromFlag, "new-user-password", "", "Supply password from the command line flag")
cmd.Flags().BoolVar(&noPassword, "no-password", false, "Create a user without password (CN based auth only)")

return &cmd
}
Expand Down Expand Up @@ -128,28 +130,32 @@ func userAddCommandFunc(cmd *cobra.Command, args []string) {
var password string
var user string

if passwordFromFlag != "" {
user = args[0]
password = passwordFromFlag
} else {
splitted := strings.SplitN(args[0], ":", 2)
if len(splitted) < 2 {
if !noPassword {
if passwordFromFlag != "" {
user = args[0]
if !passwordInteractive {
fmt.Scanf("%s", &password)
} else {
password = readPasswordInteractive(args[0])
}
password = passwordFromFlag
} else {
user = splitted[0]
password = splitted[1]
if len(user) == 0 {
ExitWithError(ExitBadArgs, fmt.Errorf("empty user name is not allowed."))
splitted := strings.SplitN(args[0], ":", 2)
if len(splitted) < 2 {
user = args[0]
if !passwordInteractive {
fmt.Scanf("%s", &password)
} else {
password = readPasswordInteractive(args[0])
}
} else {
user = splitted[0]
password = splitted[1]
if len(user) == 0 {
ExitWithError(ExitBadArgs, fmt.Errorf("empty user name is not allowed."))
}
}
}
} else {
user = args[0]
}

resp, err := mustClientFromCmd(cmd).Auth.UserAdd(context.TODO(), user, password)
resp, err := mustClientFromCmd(cmd).Auth.UserAdd(context.TODO(), user, password, noPassword)
if err != nil {
ExitWithError(ExitError, err)
}
Expand Down
Loading

0 comments on commit 76a93df

Please sign in to comment.