From 115c694af65165c6e09f6abbab43745b9b310b3a Mon Sep 17 00:00:00 2001
From: "J. David Lowe" <j.david.lowe@gmail.com>
Date: Fri, 4 Jun 2021 13:59:01 -0700
Subject: [PATCH] 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
---
 api/v3rpc/rpctypes/error.go         |  1 +
 server/auth/store.go                |  5 +++++
 server/auth/store_test.go           | 16 ++++++++++++++++
 server/etcdserver/api/v3rpc/util.go |  1 +
 4 files changed, 23 insertions(+)

diff --git a/api/v3rpc/rpctypes/error.go b/api/v3rpc/rpctypes/error.go
index 73dd84ee7..72a58b9ee 100644
--- a/api/v3rpc/rpctypes/error.go
+++ b/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()
diff --git a/server/auth/store.go b/server/auth/store.go
index 63ea7a76a..f212c3208 100644
--- a/server/auth/store.go
+++ b/server/auth/store.go
@@ -56,6 +56,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")
@@ -786,6 +787,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()
diff --git a/server/auth/store_test.go b/server/auth/store_test.go
index ec2aa9e82..4b1778268 100644
--- a/server/auth/store_test.go
+++ b/server/auth/store_test.go
@@ -445,6 +445,22 @@ func TestRoleGrantPermission(t *testing.T) {
 	}
 
 	assert.Equal(t, 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)
+	}
+
+	assert.Equal(t, perm, r.Perm[0])
 }
 
 func TestRootRoleGrantPermission(t *testing.T) {
diff --git a/server/etcdserver/api/v3rpc/util.go b/server/etcdserver/api/v3rpc/util.go
index 51cbdc66a..f61fae03b 100644
--- a/server/etcdserver/api/v3rpc/util.go
+++ b/server/etcdserver/api/v3rpc/util.go
@@ -77,6 +77,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,