etcdserver: don't attempt to grant nil permission to a role

Prevent etcd from crashing when given a bad grant payload, e.g.:

$ curl -d '{"name": "foo"}' http://localhost:2379/v3/auth/role/add
{"header":{"cluster_id":"14841639068965178418", ...
$ curl -d '{"name": "foo"}' http://localhost:2379/v3/auth/role/grant
curl: (52) Empty reply from server

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>
Signed-off-by: J. David Lowe <j.david.lowe@gmail.com>

auth/store.go

@@ -59,6 +59,7 @@ var (
 	ErrRoleAlreadyExist     = errors.New("auth: role already exists")
 	ErrRoleNotFound         = errors.New("auth: role not found")
 	ErrRoleEmpty            = errors.New("auth: role name is empty")
+	ErrPermissionNotGiven   = errors.New("auth: permission not given")
 	ErrAuthFailed           = errors.New("auth: authentication failed, invalid user ID or password")
 	ErrNoPasswordUser       = errors.New("auth: authentication failed, password was given for no password user")
 	ErrPermissionDenied     = errors.New("auth: permission denied")
@@ -884,6 +885,10 @@ func (perms permSlice) Swap(i, j int) {
 }
 
 func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) (*pb.AuthRoleGrantPermissionResponse, error) {
+	if r.Perm == nil {
+		return nil, ErrPermissionNotGiven
+	}
+
 	tx := as.be.BatchTx()
 	tx.Lock()
 	defer tx.Unlock()

auth/store_test.go

@@ -518,6 +518,24 @@ func TestRoleGrantPermission(t *testing.T) {
 	if !reflect.DeepEqual(perm, r.Perm[0]) {
 		t.Errorf("expected %v, got %v", perm, r.Perm[0])
 	}
+
+	// trying to grant nil permissions returns an error (and doesn't change the actual permissions!)
+	_, err = as.RoleGrantPermission(&pb.AuthRoleGrantPermissionRequest{
+		Name: "role-test-1",
+	})
+
+	if err != ErrPermissionNotGiven {
+		t.Error(err)
+	}
+
+	r, err = as.RoleGet(&pb.AuthRoleGetRequest{Role: "role-test-1"})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !reflect.DeepEqual(perm, r.Perm[0]) {
+		t.Errorf("expected %v, got %v", perm, r.Perm[0])
+	}
 }
 
 func TestRoleRevokePermission(t *testing.T) {

etcdserver/api/v3rpc/rpctypes/error.go

@@ -58,6 +58,7 @@ var (
 	ErrGRPCRoleNotFound         = status.New(codes.FailedPrecondition, "etcdserver: role name not found").Err()
 	ErrGRPCRoleEmpty            = status.New(codes.InvalidArgument, "etcdserver: role name is empty").Err()
 	ErrGRPCAuthFailed           = status.New(codes.InvalidArgument, "etcdserver: authentication failed, invalid user ID or password").Err()
+	ErrGRPCPermissionNotGiven   = status.New(codes.InvalidArgument, "etcdserver: permission not given").Err()
 	ErrGRPCPermissionDenied     = status.New(codes.PermissionDenied, "etcdserver: permission denied").Err()
 	ErrGRPCRoleNotGranted       = status.New(codes.FailedPrecondition, "etcdserver: role is not granted to the user").Err()
 	ErrGRPCPermissionNotGranted = status.New(codes.FailedPrecondition, "etcdserver: permission is not granted to the role").Err()

etcdserver/api/v3rpc/util.go

@@ -72,6 +72,7 @@ var toGRPCErrorMap = map[error]error{
 	auth.ErrRoleNotFound:         rpctypes.ErrGRPCRoleNotFound,
 	auth.ErrRoleEmpty:            rpctypes.ErrGRPCRoleEmpty,
 	auth.ErrAuthFailed:           rpctypes.ErrGRPCAuthFailed,
+	auth.ErrPermissionNotGiven:   rpctypes.ErrGRPCPermissionNotGiven,
 	auth.ErrPermissionDenied:     rpctypes.ErrGRPCPermissionDenied,
 	auth.ErrRoleNotGranted:       rpctypes.ErrGRPCRoleNotGranted,
 	auth.ErrPermissionNotGranted: rpctypes.ErrGRPCPermissionNotGranted,