etcdserver: do not allow creating empty role

Like user, we should not allow creating empty role.

Related #10905
release-3.4
Sahdev P. Zala 2019-07-18 23:36:46 -04:00
parent d137fa9d4a
commit 1cef112a79
10 changed files with 33 additions and 10 deletions

View File

@ -355,6 +355,9 @@ See [security doc](https://github.com/etcd-io/etcd/blob/master/Documentation/op-
- Now, WAL directory that contains only `lost+found` or a file that's not suffixed with `.wal` is considered non-initialized.
- Fix [`ETCD_CONFIG_FILE` env variable parsing in `etcd`](https://github.com/etcd-io/etcd/pull/10762).
- Fix [race condition in `rafthttp` transport pause/resume](https://github.com/etcd-io/etcd/pull/10826).
- Fix [server crash from creating an empty role](https://github.com/etcd-io/etcd/pull/10907).
- Previously, creating a role with an empty name crashed etcd server with an error code `Unavailable`.
- Now, creating a role with an empty name is not allowed with an error code `InvalidArgument`.
### API

View File

@ -11,14 +11,14 @@ This is a generated documentation. Please read the proto files for more.
| AuthEnable | AuthEnableRequest | AuthEnableResponse | AuthEnable enables authentication. |
| AuthDisable | AuthDisableRequest | AuthDisableResponse | AuthDisable disables authentication. |
| Authenticate | AuthenticateRequest | AuthenticateResponse | Authenticate processes an authenticate request. |
| UserAdd | AuthUserAddRequest | AuthUserAddResponse | UserAdd adds a new user. |
| UserAdd | AuthUserAddRequest | AuthUserAddResponse | UserAdd adds a new user. User name cannot be empty. |
| UserGet | AuthUserGetRequest | AuthUserGetResponse | UserGet gets detailed user information. |
| UserList | AuthUserListRequest | AuthUserListResponse | UserList gets a list of all users. |
| UserDelete | AuthUserDeleteRequest | AuthUserDeleteResponse | UserDelete deletes a specified user. |
| UserChangePassword | AuthUserChangePasswordRequest | AuthUserChangePasswordResponse | UserChangePassword changes the password of a specified user. |
| UserGrantRole | AuthUserGrantRoleRequest | AuthUserGrantRoleResponse | UserGrant grants a role to a specified user. |
| UserRevokeRole | AuthUserRevokeRoleRequest | AuthUserRevokeRoleResponse | UserRevokeRole revokes a role of specified user. |
| RoleAdd | AuthRoleAddRequest | AuthRoleAddResponse | RoleAdd adds a new role. |
| RoleAdd | AuthRoleAddRequest | AuthRoleAddResponse | RoleAdd adds a new role. Role name cannot be empty. |
| RoleGet | AuthRoleGetRequest | AuthRoleGetResponse | RoleGet gets detailed role information. |
| RoleList | AuthRoleListRequest | AuthRoleListResponse | RoleList gets lists of all roles. |
| RoleDelete | AuthRoleDeleteRequest | AuthRoleDeleteResponse | RoleDelete deletes a specified role. |

View File

@ -101,7 +101,7 @@
"tags": [
"Auth"
],
"summary": "RoleAdd adds a new role.",
"summary": "RoleAdd adds a new role. Role name cannot be empty.",
"operationId": "RoleAdd",
"parameters": [
{
@ -263,7 +263,7 @@
"tags": [
"Auth"
],
"summary": "UserAdd adds a new user.",
"summary": "UserAdd adds a new user. User name cannot be empty.",
"operationId": "UserAdd",
"parameters": [
{

View File

@ -57,6 +57,7 @@ var (
ErrUserNotFound = errors.New("auth: user not found")
ErrRoleAlreadyExist = errors.New("auth: role already exists")
ErrRoleNotFound = errors.New("auth: role not found")
ErrRoleEmpty = errors.New("auth: role name is empty")
ErrAuthFailed = errors.New("auth: authentication failed, invalid user ID or password")
ErrPermissionDenied = errors.New("auth: permission denied")
ErrRoleNotGranted = errors.New("auth: role is not granted to the user")
@ -796,6 +797,10 @@ func (as *authStore) RoleDelete(r *pb.AuthRoleDeleteRequest) (*pb.AuthRoleDelete
}
func (as *authStore) RoleAdd(r *pb.AuthRoleAddRequest) (*pb.AuthRoleAddResponse, error) {
if len(r.Name) == 0 {
return nil, ErrRoleEmpty
}
tx := as.be.BatchTx()
tx.Lock()
defer tx.Unlock()

View File

@ -269,6 +269,12 @@ func TestRoleAdd(t *testing.T) {
if err != nil {
t.Fatal(err)
}
// add a role with empty name
_, err = as.RoleAdd(&pb.AuthRoleAddRequest{Name: ""})
if err != ErrRoleEmpty {
t.Fatal(err)
}
}
func TestUserGrant(t *testing.T) {

View File

@ -40,4 +40,9 @@ func TestRoleError(t *testing.T) {
if err != rpctypes.ErrRoleAlreadyExist {
t.Fatalf("expected %v, got %v", rpctypes.ErrRoleAlreadyExist, err)
}
_, err = authapi.RoleAdd(context.TODO(), "")
if err != rpctypes.ErrRoleEmpty {
t.Fatalf("expected %v, got %v", rpctypes.ErrRoleEmpty, err)
}
}

View File

@ -54,6 +54,7 @@ var (
ErrGRPCUserNotFound = status.New(codes.FailedPrecondition, "etcdserver: user name not found").Err()
ErrGRPCRoleAlreadyExist = status.New(codes.FailedPrecondition, "etcdserver: role name already exists").Err()
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()
ErrGRPCPermissionDenied = status.New(codes.PermissionDenied, "etcdserver: permission denied").Err()
ErrGRPCRoleNotGranted = status.New(codes.FailedPrecondition, "etcdserver: role is not granted to the user").Err()
@ -110,6 +111,7 @@ var (
ErrorDesc(ErrGRPCUserNotFound): ErrGRPCUserNotFound,
ErrorDesc(ErrGRPCRoleAlreadyExist): ErrGRPCRoleAlreadyExist,
ErrorDesc(ErrGRPCRoleNotFound): ErrGRPCRoleNotFound,
ErrorDesc(ErrGRPCRoleEmpty): ErrGRPCRoleEmpty,
ErrorDesc(ErrGRPCAuthFailed): ErrGRPCAuthFailed,
ErrorDesc(ErrGRPCPermissionDenied): ErrGRPCPermissionDenied,
ErrorDesc(ErrGRPCRoleNotGranted): ErrGRPCRoleNotGranted,
@ -168,6 +170,7 @@ var (
ErrUserNotFound = Error(ErrGRPCUserNotFound)
ErrRoleAlreadyExist = Error(ErrGRPCRoleAlreadyExist)
ErrRoleNotFound = Error(ErrGRPCRoleNotFound)
ErrRoleEmpty = Error(ErrGRPCRoleEmpty)
ErrAuthFailed = Error(ErrGRPCAuthFailed)
ErrPermissionDenied = Error(ErrGRPCPermissionDenied)
ErrRoleNotGranted = Error(ErrGRPCRoleNotGranted)

View File

@ -69,6 +69,7 @@ var toGRPCErrorMap = map[error]error{
auth.ErrUserNotFound: rpctypes.ErrGRPCUserNotFound,
auth.ErrRoleAlreadyExist: rpctypes.ErrGRPCRoleAlreadyExist,
auth.ErrRoleNotFound: rpctypes.ErrGRPCRoleNotFound,
auth.ErrRoleEmpty: rpctypes.ErrGRPCRoleEmpty,
auth.ErrAuthFailed: rpctypes.ErrGRPCAuthFailed,
auth.ErrPermissionDenied: rpctypes.ErrGRPCPermissionDenied,
auth.ErrRoleNotGranted: rpctypes.ErrGRPCRoleNotGranted,

View File

@ -4537,7 +4537,7 @@ type AuthClient interface {
AuthDisable(ctx context.Context, in *AuthDisableRequest, opts ...grpc.CallOption) (*AuthDisableResponse, error)
// Authenticate processes an authenticate request.
Authenticate(ctx context.Context, in *AuthenticateRequest, opts ...grpc.CallOption) (*AuthenticateResponse, error)
// UserAdd adds a new user.
// UserAdd adds a new user. User name cannot be empty.
UserAdd(ctx context.Context, in *AuthUserAddRequest, opts ...grpc.CallOption) (*AuthUserAddResponse, error)
// UserGet gets detailed user information.
UserGet(ctx context.Context, in *AuthUserGetRequest, opts ...grpc.CallOption) (*AuthUserGetResponse, error)
@ -4551,7 +4551,7 @@ type AuthClient interface {
UserGrantRole(ctx context.Context, in *AuthUserGrantRoleRequest, opts ...grpc.CallOption) (*AuthUserGrantRoleResponse, error)
// UserRevokeRole revokes a role of specified user.
UserRevokeRole(ctx context.Context, in *AuthUserRevokeRoleRequest, opts ...grpc.CallOption) (*AuthUserRevokeRoleResponse, error)
// RoleAdd adds a new role.
// RoleAdd adds a new role. Role name cannot be empty.
RoleAdd(ctx context.Context, in *AuthRoleAddRequest, opts ...grpc.CallOption) (*AuthRoleAddResponse, error)
// RoleGet gets detailed role information.
RoleGet(ctx context.Context, in *AuthRoleGetRequest, opts ...grpc.CallOption) (*AuthRoleGetResponse, error)
@ -4726,7 +4726,7 @@ type AuthServer interface {
AuthDisable(context.Context, *AuthDisableRequest) (*AuthDisableResponse, error)
// Authenticate processes an authenticate request.
Authenticate(context.Context, *AuthenticateRequest) (*AuthenticateResponse, error)
// UserAdd adds a new user.
// UserAdd adds a new user. User name cannot be empty.
UserAdd(context.Context, *AuthUserAddRequest) (*AuthUserAddResponse, error)
// UserGet gets detailed user information.
UserGet(context.Context, *AuthUserGetRequest) (*AuthUserGetResponse, error)
@ -4740,7 +4740,7 @@ type AuthServer interface {
UserGrantRole(context.Context, *AuthUserGrantRoleRequest) (*AuthUserGrantRoleResponse, error)
// UserRevokeRole revokes a role of specified user.
UserRevokeRole(context.Context, *AuthUserRevokeRoleRequest) (*AuthUserRevokeRoleResponse, error)
// RoleAdd adds a new role.
// RoleAdd adds a new role. Role name cannot be empty.
RoleAdd(context.Context, *AuthRoleAddRequest) (*AuthRoleAddResponse, error)
// RoleGet gets detailed role information.
RoleGet(context.Context, *AuthRoleGetRequest) (*AuthRoleGetResponse, error)

View File

@ -264,7 +264,7 @@ service Auth {
};
}
// UserAdd adds a new user.
// UserAdd adds a new user. User name cannot be empty.
rpc UserAdd(AuthUserAddRequest) returns (AuthUserAddResponse) {
option (google.api.http) = {
post: "/v3/auth/user/add"
@ -320,7 +320,7 @@ service Auth {
};
}
// RoleAdd adds a new role.
// RoleAdd adds a new role. Role name cannot be empty.
rpc RoleAdd(AuthRoleAddRequest) returns (AuthRoleAddResponse) {
option (google.api.http) = {
post: "/v3/auth/role/add"