From 77e27f66fe1e4743cfb9ca509a2666794715ce28 Mon Sep 17 00:00:00 2001 From: Jack Wu Date: Wed, 20 Oct 2021 18:24:21 -0400 Subject: [PATCH] sql: with grant option/grant option for Release note (sql change): If the WITH GRANT OPTION flag is present when granting privileges to a user, then that user is able to grant those same privileges to subsequent users; otherwise, they cannot. If the GRANT OPTION FOR flag is present when revoking privileges from a user, then only the ability the grant those privileges is revoked from that user, not the privileges themselves --- .../settings/settings-for-tenants.txt | 2 +- docs/generated/settings/settings.html | 2 +- docs/generated/sql/bnf/grant_stmt.bnf | 6 +- docs/generated/sql/bnf/revoke_stmt.bnf | 3 + docs/generated/sql/bnf/stmt_block.bnf | 20 +- pkg/ccl/importccl/import_table_creation.go | 1 + pkg/clusterversion/cockroach_versions.go | 8 + pkg/cmd/roachtest/cluster.go | 16 + .../roachtest/cluster/cluster_interface.go | 1 + pkg/sql/alter_default_privileges.go | 6 +- .../catalog/catprivilege/default_privilege.go | 52 +- pkg/sql/catalog/descpb/privilege.go | 146 ++++- pkg/sql/catalog/descpb/privilege.pb.go | 129 ++-- pkg/sql/catalog/descpb/privilege.proto | 1 + pkg/sql/catalog/descpb/privilege_test.go | 250 +++++-- pkg/sql/grant_revoke.go | 15 +- ...alter_default_privileges_with_grant_option | 611 ++++++++++++++++++ .../logic_test/grant_revoke_with_grant_option | 292 +++++++++ ...g_catalog_pg_default_acl_with_grant_option | 164 +++++ pkg/sql/parser/sql.y | 52 +- pkg/sql/pg_catalog.go | 15 +- pkg/sql/privilege/privilege.go | 47 +- pkg/sql/sem/tree/grant.go | 7 +- pkg/sql/sem/tree/revoke.go | 7 +- 24 files changed, 1681 insertions(+), 172 deletions(-) create mode 100644 pkg/sql/logictest/testdata/logic_test/alter_default_privileges_with_grant_option create mode 100644 pkg/sql/logictest/testdata/logic_test/grant_revoke_with_grant_option create mode 100644 pkg/sql/logictest/testdata/logic_test/pg_catalog_pg_default_acl_with_grant_option diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index 2574ca609b7d..ecaaac1d9f74 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -167,4 +167,4 @@ trace.debug.enable boolean false if set, traces for recent requests can be seen trace.jaeger.agent string the address of a Jaeger agent to receive traces using the Jaeger UDP Thrift protocol, as :. If no port is specified, 6381 will be used. trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.zipkin.collector string the address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -version version 21.2-4 set the active cluster version in the format '.' +version version 21.2-6 set the active cluster version in the format '.' diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index f79e16ccfbf4..8d74d4e4a5fe 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -172,6 +172,6 @@ trace.jaeger.agentstringthe address of a Jaeger agent to receive traces using the Jaeger UDP Thrift protocol, as :. If no port is specified, 6381 will be used. trace.opentelemetry.collectorstringaddress of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.zipkin.collectorstringthe address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -versionversion21.2-4set the active cluster version in the format '.' +versionversion21.2-6set the active cluster version in the format '.' diff --git a/docs/generated/sql/bnf/grant_stmt.bnf b/docs/generated/sql/bnf/grant_stmt.bnf index c532da5452f9..dc69eb2944ec 100644 --- a/docs/generated/sql/bnf/grant_stmt.bnf +++ b/docs/generated/sql/bnf/grant_stmt.bnf @@ -1,6 +1,6 @@ grant_stmt ::= - 'GRANT' 'ALL' 'PRIVILEGES' 'ON' targets 'TO' role_spec_list - | 'GRANT' 'ALL' 'ON' targets 'TO' role_spec_list - | 'GRANT' privilege_list 'ON' targets 'TO' role_spec_list + 'GRANT' 'ALL' 'PRIVILEGES' 'ON' targets 'TO' role_spec_list opt_with_grant_option + | 'GRANT' 'ALL' 'ON' targets 'TO' role_spec_list opt_with_grant_option + | 'GRANT' privilege_list 'ON' targets 'TO' role_spec_list opt_with_grant_option | 'GRANT' privilege_list 'TO' role_spec_list | 'GRANT' privilege_list 'TO' role_spec_list 'WITH' 'ADMIN' 'OPTION' diff --git a/docs/generated/sql/bnf/revoke_stmt.bnf b/docs/generated/sql/bnf/revoke_stmt.bnf index 4f23347ed806..a24e96e31624 100644 --- a/docs/generated/sql/bnf/revoke_stmt.bnf +++ b/docs/generated/sql/bnf/revoke_stmt.bnf @@ -2,5 +2,8 @@ revoke_stmt ::= 'REVOKE' 'ALL' 'PRIVILEGES' 'ON' targets 'FROM' role_spec_list | 'REVOKE' 'ALL' 'ON' targets 'FROM' role_spec_list | 'REVOKE' privilege_list 'ON' targets 'FROM' role_spec_list + | 'REVOKE' 'GRANT' 'OPTION' 'FOR' 'ALL' 'PRIVILEGES' 'ON' targets 'FROM' role_spec_list + | 'REVOKE' 'GRANT' 'OPTION' 'FOR' 'ALL' 'ON' targets 'FROM' role_spec_list + | 'REVOKE' 'GRANT' 'OPTION' 'FOR' privilege_list 'ON' targets 'FROM' role_spec_list | 'REVOKE' privilege_list 'FROM' role_spec_list | 'REVOKE' 'ADMIN' 'OPTION' 'FOR' privilege_list 'FROM' role_spec_list diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index 6c846178024b..e40659bdc2d6 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -74,23 +74,27 @@ discard_stmt ::= 'DISCARD' 'ALL' grant_stmt ::= - 'GRANT' privileges 'ON' targets 'TO' role_spec_list + 'GRANT' privileges 'ON' targets 'TO' role_spec_list opt_with_grant_option | 'GRANT' privilege_list 'TO' role_spec_list | 'GRANT' privilege_list 'TO' role_spec_list 'WITH' 'ADMIN' 'OPTION' - | 'GRANT' privileges 'ON' 'TYPE' target_types 'TO' role_spec_list - | 'GRANT' privileges 'ON' 'SCHEMA' schema_name_list 'TO' role_spec_list - | 'GRANT' privileges 'ON' 'ALL' 'TABLES' 'IN' 'SCHEMA' schema_name_list 'TO' role_spec_list + | 'GRANT' privileges 'ON' 'TYPE' target_types 'TO' role_spec_list opt_with_grant_option + | 'GRANT' privileges 'ON' 'SCHEMA' schema_name_list 'TO' role_spec_list opt_with_grant_option + | 'GRANT' privileges 'ON' 'ALL' 'TABLES' 'IN' 'SCHEMA' schema_name_list 'TO' role_spec_list opt_with_grant_option prepare_stmt ::= 'PREPARE' table_alias_name prep_type_clause 'AS' preparable_stmt revoke_stmt ::= 'REVOKE' privileges 'ON' targets 'FROM' role_spec_list + | 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' targets 'FROM' role_spec_list | 'REVOKE' privilege_list 'FROM' role_spec_list | 'REVOKE' 'ADMIN' 'OPTION' 'FOR' privilege_list 'FROM' role_spec_list | 'REVOKE' privileges 'ON' 'TYPE' target_types 'FROM' role_spec_list + | 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' 'TYPE' target_types 'FROM' role_spec_list | 'REVOKE' privileges 'ON' 'SCHEMA' schema_name_list 'FROM' role_spec_list + | 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' 'SCHEMA' schema_name_list 'FROM' role_spec_list | 'REVOKE' privileges 'ON' 'ALL' 'TABLES' 'IN' 'SCHEMA' schema_name_list 'FROM' role_spec_list + | 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' 'ALL' 'TABLES' 'IN' 'SCHEMA' schema_name_list 'FROM' role_spec_list savepoint_stmt ::= 'SAVEPOINT' name @@ -321,6 +325,10 @@ targets ::= role_spec_list ::= ( role_spec ) ( ( ',' role_spec ) )* +opt_with_grant_option ::= + 'WITH' 'GRANT' 'OPTION' + | + privilege_list ::= ( privilege ) ( ( ',' privilege ) )* @@ -2312,10 +2320,6 @@ alter_default_privileges_target_object ::= | 'TYPES' | 'SCHEMAS' -opt_with_grant_option ::= - 'WITH' 'GRANT' 'OPTION' - | - role_option ::= 'CREATEROLE' | 'NOCREATEROLE' diff --git a/pkg/ccl/importccl/import_table_creation.go b/pkg/ccl/importccl/import_table_creation.go index f5e661b5f523..58ad0f005e11 100644 --- a/pkg/ccl/importccl/import_table_creation.go +++ b/pkg/ccl/importccl/import_table_creation.go @@ -79,6 +79,7 @@ func MakeTestingSimpleTableDescriptor( Privileges: descpb.NewPrivilegeDescriptor( security.PublicRoleName(), privilege.SchemaPrivileges, + privilege.List{}, security.RootUserName(), ), }).BuildCreatedMutableSchema() diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index 46dedc598e14..e84d45ad5976 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -271,6 +271,10 @@ const ( // limits when splitting requests. TargetBytesAvoidExcess + // ValidateGrantOption checks whether the current user granting privileges to + // another user holds the grant option for those privileges + ValidateGrantOption + // ************************************************* // Step (1): Add new versions here. // Do not add new versions to a patch release. @@ -459,6 +463,10 @@ var versionsSingleton = keyedVersions{ Key: TargetBytesAvoidExcess, Version: roachpb.Version{Major: 21, Minor: 2, Internal: 4}, }, + { + Key: ValidateGrantOption, + Version: roachpb.Version{Major: 21, Minor: 2, Internal: 6}, + }, // ************************************************* // Step (2): Add new versions here. diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go index 49d3c8d834fa..71343eb75cc0 100644 --- a/pkg/cmd/roachtest/cluster.go +++ b/pkg/cmd/roachtest/cluster.go @@ -2308,6 +2308,22 @@ func (c *clusterImpl) ConnE(ctx context.Context, node int) (*gosql.DB, error) { return db, nil } +// ConnEUser returns a SQL connection to the specified node as a specific user +func (c *clusterImpl) ConnEUser(ctx context.Context, node int, user string) (*gosql.DB, error) { + urls, err := c.ExternalPGUrl(ctx, c.Node(node)) + if err != nil { + return nil, err + } + //dataSourceName := fmt.Sprintf("%s?user=%s?password=%s", urls[0], user, password) + dataSourceName := strings.Replace(urls[0], "root", user, 1) + fmt.Println("datasourcename is: ", dataSourceName) + db, err := gosql.Open("postgres", dataSourceName) + if err != nil { + return nil, err + } + return db, nil +} + func (c *clusterImpl) MakeNodes(opts ...option.Option) string { var r option.NodeListOption for _, o := range opts { diff --git a/pkg/cmd/roachtest/cluster/cluster_interface.go b/pkg/cmd/roachtest/cluster/cluster_interface.go index 1ec97c76d0a8..fe379db5da79 100644 --- a/pkg/cmd/roachtest/cluster/cluster_interface.go +++ b/pkg/cmd/roachtest/cluster/cluster_interface.go @@ -71,6 +71,7 @@ type Cluster interface { Conn(ctx context.Context, node int) *gosql.DB ConnE(ctx context.Context, node int) (*gosql.DB, error) + ConnEUser(ctx context.Context, node int, user string) (*gosql.DB, error) // URLs for the Admin UI. diff --git a/pkg/sql/alter_default_privileges.go b/pkg/sql/alter_default_privileges.go index b6b739855cce..875534dc3f6c 100644 --- a/pkg/sql/alter_default_privileges.go +++ b/pkg/sql/alter_default_privileges.go @@ -93,10 +93,12 @@ func (n *alterDefaultPrivilegesNode) startExec(params runParams) error { privileges := n.n.Grant.Privileges grantees := n.n.Grant.Grantees objectType := n.n.Grant.Target + grantOption := n.n.Grant.WithGrantOption if !n.n.IsGrant { privileges = n.n.Revoke.Privileges grantees = n.n.Revoke.Grantees objectType = n.n.Revoke.Target + grantOption = n.n.Revoke.GrantOptionFor } granteeSQLUsernames, err := grantees.ToSQLUsernames(params.p.SessionData(), security.UsernameValidation) @@ -165,11 +167,11 @@ func (n *alterDefaultPrivilegesNode) startExec(params runParams) error { for _, role := range roles { if n.n.IsGrant { defaultPrivs.GrantDefaultPrivileges( - role, privileges, granteeSQLUsernames, objectType, + role, privileges, granteeSQLUsernames, objectType, grantOption, ) } else { defaultPrivs.RevokeDefaultPrivileges( - role, privileges, granteeSQLUsernames, objectType, + role, privileges, granteeSQLUsernames, objectType, grantOption, ) } diff --git a/pkg/sql/catalog/catprivilege/default_privilege.go b/pkg/sql/catalog/catprivilege/default_privilege.go index a2ba632600ab..86fc44a9de40 100644 --- a/pkg/sql/catalog/catprivilege/default_privilege.go +++ b/pkg/sql/catalog/catprivilege/default_privilege.go @@ -71,6 +71,7 @@ func (d *Mutable) GrantDefaultPrivileges( privileges privilege.List, grantees []security.SQLUsername, targetObject tree.AlterDefaultPrivilegesTargetObject, + withGrantOption bool, ) { defaultPrivilegesForRole := d.defaultPrivilegeDescriptor.FindOrCreateUser(role) for _, grantee := range grantees { @@ -79,7 +80,7 @@ func (d *Mutable) GrantDefaultPrivileges( // special privilege cases into real privileges on the PrivilegeDescriptor. // foldPrivileges converts the real privileges back into flags. expandPrivileges(defaultPrivilegesForRole, role, &defaultPrivileges, targetObject) - defaultPrivileges.Grant(grantee, privileges) + defaultPrivileges.Grant(grantee, privileges, withGrantOption) foldPrivileges(defaultPrivilegesForRole, role, &defaultPrivileges, targetObject) defaultPrivilegesForRole.DefaultPrivilegesPerObject[targetObject] = defaultPrivileges } @@ -91,6 +92,7 @@ func (d *Mutable) RevokeDefaultPrivileges( privileges privilege.List, grantees []security.SQLUsername, targetObject tree.AlterDefaultPrivilegesTargetObject, + grantOptionFor bool, ) { defaultPrivilegesForRole := d.defaultPrivilegeDescriptor.FindOrCreateUser(role) for _, grantee := range grantees { @@ -99,7 +101,7 @@ func (d *Mutable) RevokeDefaultPrivileges( // special privilege cases into real privileges on the PrivilegeDescriptor. // foldPrivileges converts the real privileges back into flags. expandPrivileges(defaultPrivilegesForRole, role, &defaultPrivileges, targetObject) - defaultPrivileges.Revoke(grantee, privileges, targetObject.ToPrivilegeObjectType()) + defaultPrivileges.Revoke(grantee, privileges, targetObject.ToPrivilegeObjectType(), grantOptionFor) foldPrivileges(defaultPrivilegesForRole, role, &defaultPrivileges, targetObject) defaultPrivilegesForRole.DefaultPrivilegesPerObject[targetObject] = defaultPrivileges @@ -151,18 +153,20 @@ func (d *immutable) CreatePrivilegesFromDefaultPrivileges( // it as the case where the user has all privileges. defaultPrivilegesForCreatorRole := descpb.InitDefaultPrivilegesForRole(role) for _, user := range GetUserPrivilegesForObject(defaultPrivilegesForCreatorRole, targetObject) { - newPrivs.Grant( + newPrivs.GranularGrant( user.UserProto.Decode(), privilege.ListFromBitField(user.Privileges, targetObject.ToPrivilegeObjectType()), + privilege.ListFromBitField(user.WithGrantOption, targetObject.ToPrivilegeObjectType()), ) } } else { // If default privileges were defined for the role, we create privileges // using the default privileges. for _, user := range GetUserPrivilegesForObject(*defaultPrivilegesForRole, targetObject) { - newPrivs.Grant( + newPrivs.GranularGrant( user.UserProto.Decode(), privilege.ListFromBitField(user.Privileges, targetObject.ToPrivilegeObjectType()), + privilege.ListFromBitField(user.WithGrantOption, targetObject.ToPrivilegeObjectType()), ) } } @@ -173,9 +177,10 @@ func (d *immutable) CreatePrivilegesFromDefaultPrivileges( defaultPrivilegesForAllRoles, found := d.GetDefaultPrivilegesForRole(descpb.DefaultPrivilegesRole{ForAllRoles: true}) if found { for _, user := range GetUserPrivilegesForObject(*defaultPrivilegesForAllRoles, targetObject) { - newPrivs.Grant( + newPrivs.GranularGrant( user.UserProto.Decode(), privilege.ListFromBitField(user.Privileges, targetObject.ToPrivilegeObjectType()), + privilege.ListFromBitField(user.WithGrantOption, targetObject.ToPrivilegeObjectType()), ) } } @@ -186,12 +191,26 @@ func (d *immutable) CreatePrivilegesFromDefaultPrivileges( // For backwards compatibility, also "inherit" privileges from the dbDesc. // Issue #67378. if targetObject == tree.Tables || targetObject == tree.Sequences { + //fmt.Println("tables or sequences loop") for _, u := range databasePrivileges.Users { - newPrivs.Grant(u.UserProto.Decode(), privilege.ListFromBitField(u.Privileges, privilege.Table)) + //fmt.Println(u.UserProto.Decode(), u.Privileges, u.WithGrantOption) + // newPrivs.Grant(u.UserProto.Decode(), privilege.ListFromBitField(u.Privileges, privilege.Table), false) + newPrivs.GranularGrant( + u.UserProto.Decode(), + privilege.ListFromBitField(u.Privileges, privilege.Table), + privilege.ListFromBitField(u.WithGrantOption, privilege.Table), + ) } } else if targetObject == tree.Schemas { + //fmt.Println("schemas loop") for _, u := range databasePrivileges.Users { - newPrivs.Grant(u.UserProto.Decode(), privilege.ListFromBitField(u.Privileges, privilege.Schema)) + //fmt.Println(u.UserProto.Decode(), u.Privileges, u.WithGrantOption) + //newPrivs.Grant(u.UserProto.Decode(), privilege.ListFromBitField(u.Privileges, privilege.Schema), false) + newPrivs.GranularGrant( + u.UserProto.Decode(), + privilege.ListFromBitField(u.Privileges, privilege.Schema), + privilege.ListFromBitField(u.WithGrantOption, privilege.Schema), + ) } } return newPrivs @@ -249,6 +268,7 @@ func foldPrivileges( security.PublicRoleName(), privilege.List{privilege.USAGE}, privilege.Type, + false, ) } // ForAllRoles cannot be a grantee, nothing left to do. @@ -256,8 +276,16 @@ func foldPrivileges( return } if privileges.HasAllPrivileges(role.Role, targetObject.ToPrivilegeObjectType()) { - setRoleHasAllOnTargetObject(defaultPrivilegesForRole, true, targetObject) - privileges.RemoveUser(role.Role) + // if user.grantoption == 0, then do this next line CHECK THIS LATER + //user := privileges.FindOrCreateUser(role.Role) + //if user.WithGrantOption == 0 { + //setRoleHasAllOnTargetObject(defaultPrivilegesForRole, true, targetObject) + //} + user := privileges.FindOrCreateUser(role.Role) // + if user.WithGrantOption == 0 { // + setRoleHasAllOnTargetObject(defaultPrivilegesForRole, true, targetObject) + privileges.RemoveUser(role.Role) + } // } } @@ -273,7 +301,7 @@ func expandPrivileges( targetObject tree.AlterDefaultPrivilegesTargetObject, ) { if targetObject == tree.Types && GetPublicHasUsageOnTypes(defaultPrivilegesForRole) { - privileges.Grant(security.PublicRoleName(), privilege.List{privilege.USAGE}) + privileges.Grant(security.PublicRoleName(), privilege.List{privilege.USAGE}, false) setPublicHasUsageOnTypes(defaultPrivilegesForRole, false) } // ForAllRoles cannot be a grantee, nothing left to do. @@ -281,7 +309,7 @@ func expandPrivileges( return } if GetRoleHasAllPrivilegesOnTargetObject(defaultPrivilegesForRole, targetObject) { - privileges.Grant(defaultPrivilegesForRole.GetExplicitRole().UserProto.Decode(), privilege.List{privilege.ALL}) + privileges.Grant(defaultPrivilegesForRole.GetExplicitRole().UserProto.Decode(), privilege.List{privilege.ALL}, false) setRoleHasAllOnTargetObject(defaultPrivilegesForRole, false, targetObject) } } @@ -299,6 +327,7 @@ func GetUserPrivilegesForObject( userPrivileges = append(userPrivileges, descpb.UserPrivileges{ UserProto: security.PublicRoleName().EncodeProto(), Privileges: privilege.USAGE.Mask(), + //GrantOption: privilege.USAGE.Mask(), // CHECK THIS LATER }) } // If ForAllRoles is specified, we can return early. @@ -312,6 +341,7 @@ func GetUserPrivilegesForObject( return append(userPrivileges, descpb.UserPrivileges{ UserProto: userProto, Privileges: privilege.ALL.Mask(), + //GrantOption: privilege.ALL.Mask(), // CHECK THIS LATER }) } return userPrivileges diff --git a/pkg/sql/catalog/descpb/privilege.go b/pkg/sql/catalog/descpb/privilege.go index 912e7e9fa130..eb624a210ded 100644 --- a/pkg/sql/catalog/descpb/privilege.go +++ b/pkg/sql/catalog/descpb/privilege.go @@ -18,6 +18,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/errors" ) @@ -111,12 +113,14 @@ func NewCustomSuperuserPrivilegeDescriptor( OwnerProto: owner.EncodeProto(), Users: []UserPrivileges{ { - UserProto: security.AdminRoleName().EncodeProto(), - Privileges: priv.ToBitField(), + UserProto: security.AdminRoleName().EncodeProto(), + Privileges: priv.ToBitField(), + WithGrantOption: priv.ToBitField(), }, { - UserProto: security.RootUserName().EncodeProto(), - Privileges: priv.ToBitField(), + UserProto: security.RootUserName().EncodeProto(), + Privileges: priv.ToBitField(), + WithGrantOption: priv.ToBitField(), }, }, Version: Version21_2, @@ -128,21 +132,25 @@ func NewCustomSuperuserPrivilegeDescriptor( // used for virtual tables. func NewPublicSelectPrivilegeDescriptor() *PrivilegeDescriptor { return NewPrivilegeDescriptor( - security.PublicRoleName(), privilege.List{privilege.SELECT}, security.NodeUserName(), + security.PublicRoleName(), privilege.List{privilege.SELECT}, privilege.List{}, security.NodeUserName(), ) } // NewPrivilegeDescriptor returns a privilege descriptor for the given // user with the specified list of privileges. func NewPrivilegeDescriptor( - user security.SQLUsername, priv privilege.List, owner security.SQLUsername, + user security.SQLUsername, + priv privilege.List, + grantOption privilege.List, + owner security.SQLUsername, ) *PrivilegeDescriptor { return &PrivilegeDescriptor{ OwnerProto: owner.EncodeProto(), Users: []UserPrivileges{ { - UserProto: user.EncodeProto(), - Privileges: priv.ToBitField(), + UserProto: user.EncodeProto(), + Privileges: priv.ToBitField(), + WithGrantOption: grantOption.ToBitField(), }, }, Version: Version21_2, @@ -159,28 +167,107 @@ func NewDefaultPrivilegeDescriptor(owner security.SQLUsername) *PrivilegeDescrip return NewCustomSuperuserPrivilegeDescriptor(DefaultSuperuserPrivileges, owner) } +// ValidateGrantPrivileges returns an error if the current user tries to grant a privilege that +// it does not possess or was not granted WITH GRANT OPTION +func (p *PrivilegeDescriptor) ValidateGrantPrivileges( + user security.SQLUsername, privList privilege.List, +) error { + userPriv, _ := p.FindUser(user) + + // User has ALL WITH GRANT OPTION so they can grant anything. + if privilege.ALL.IsSetIn(userPriv.WithGrantOption) { + return nil + } + + for _, priv := range privList { + if userPriv.WithGrantOption&priv.Mask() == 0 { + return pgerror.Newf(pgcode.InvalidGrantOperation, + "missing WITH GRANT OPTION privilege type %s", priv.String()) + } + } + + return nil +} + +// GranularGrant adds new privileges to this descriptor and new grant options which +// could be different from the privileges. Unlike the normal grant, the privileges +// and the grant options being granted could be different +func (p *PrivilegeDescriptor) GranularGrant( + user security.SQLUsername, privList privilege.List, grantOptionList privilege.List, +) { + userPriv := p.FindOrCreateUser(user) + if privilege.ALL.IsSetIn(userPriv.WithGrantOption) { + // User already has 'ALL' privilege: no-op. + // If userPriv.WithGrantOption has ALL, then userPriv.Privileges must also have ALL. + // It is possible however for userPriv.Privileges to have ALL but userPriv.WithGrantOption to not have ALL + return + } + + privBits := privList.ToBitField() + grantBits := grantOptionList.ToBitField() + if privilege.ALL.IsSetIn(privBits) { + userPriv.Privileges = privilege.ALL.Mask() + } else { + if !privilege.ALL.IsSetIn(userPriv.Privileges) { + userPriv.Privileges |= privBits + } + } + + if privilege.ALL.IsSetIn(grantBits) { + userPriv.WithGrantOption = privilege.ALL.Mask() + } else { + if !privilege.ALL.IsSetIn(userPriv.WithGrantOption) { + userPriv.WithGrantOption |= grantBits + } + } +} + // Grant adds new privileges to this descriptor for a given list of users. -func (p *PrivilegeDescriptor) Grant(user security.SQLUsername, privList privilege.List) { +func (p *PrivilegeDescriptor) Grant( + user security.SQLUsername, privList privilege.List, withGrantOption bool, +) { userPriv := p.FindOrCreateUser(user) - if privilege.ALL.IsSetIn(userPriv.Privileges) { + if privilege.ALL.IsSetIn(userPriv.WithGrantOption) { // User already has 'ALL' privilege: no-op. + // If userPriv.WithGrantOption has ALL, then userPriv.Privileges must also have ALL. + // It is possible however for userPriv.Privileges to have ALL but userPriv.WithGrantOption to not have ALL + return + } + + if privilege.ALL.IsSetIn(userPriv.Privileges) && !withGrantOption { + // A user can hold all privileges but not all grant options. + // If a user holds all privileges but withGrantOption is False, + // there is nothing left to be done return } + // We only have to modify grant option. bits := privList.ToBitField() if privilege.ALL.IsSetIn(bits) { // Granting 'ALL' privilege: overwrite. // TODO(marc): the grammar does not allow it, but we should // check if other privileges are being specified and error out. userPriv.Privileges = privilege.ALL.Mask() + if withGrantOption { + userPriv.WithGrantOption = privilege.ALL.Mask() + } return } - userPriv.Privileges |= bits + + if withGrantOption { + userPriv.WithGrantOption |= bits + } + if !privilege.ALL.IsSetIn(userPriv.Privileges) { + userPriv.Privileges |= bits + } } // Revoke removes privileges from this descriptor for a given list of users. func (p *PrivilegeDescriptor) Revoke( - user security.SQLUsername, privList privilege.List, objectType privilege.ObjectType, + user security.SQLUsername, + privList privilege.List, + objectType privilege.ObjectType, + grantOptionFor bool, ) { userPriv, ok := p.FindUser(user) if !ok || userPriv.Privileges == 0 { @@ -190,14 +277,17 @@ func (p *PrivilegeDescriptor) Revoke( bits := privList.ToBitField() if privilege.ALL.IsSetIn(bits) { - // Revoking 'ALL' privilege: remove user. - // TODO(marc): the grammar does not allow it, but we should - // check if other privileges are being specified and error out. - p.RemoveUser(user) + userPriv.WithGrantOption = 0 + if !grantOptionFor { + // Revoking 'ALL' privilege: remove user. + // TODO(marc): the grammar does not allow it, but we should + // check if other privileges are being specified and error out. + p.RemoveUser(user) + } return } - if privilege.ALL.IsSetIn(userPriv.Privileges) { + if privilege.ALL.IsSetIn(userPriv.Privileges) && !grantOptionFor { // User has 'ALL' privilege. Remove it and set // all other privileges one. validPrivs := privilege.GetValidPrivilegesForObject(objectType) @@ -209,12 +299,28 @@ func (p *PrivilegeDescriptor) Revoke( } } + if privilege.ALL.IsSetIn(userPriv.WithGrantOption) { + // User has 'ALL' grant option. Remove it and set + // all other grant options to one. + validPrivs := privilege.GetValidPrivilegesForObject(objectType) + userPriv.WithGrantOption = 0 + for _, v := range validPrivs { + if v != privilege.ALL { + userPriv.WithGrantOption |= v.Mask() + } + } + } + // One doesn't see "AND NOT" very often. - userPriv.Privileges &^= bits + userPriv.WithGrantOption &^= bits + if !grantOptionFor { + userPriv.Privileges &^= bits - if userPriv.Privileges == 0 { - p.RemoveUser(user) + if userPriv.Privileges == 0 { + p.RemoveUser(user) + } } + } // ValidateSuperuserPrivileges ensures that superusers have exactly the maximum diff --git a/pkg/sql/catalog/descpb/privilege.pb.go b/pkg/sql/catalog/descpb/privilege.pb.go index 819e558c5975..8faa36db5050 100644 --- a/pkg/sql/catalog/descpb/privilege.pb.go +++ b/pkg/sql/catalog/descpb/privilege.pb.go @@ -30,7 +30,8 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package type UserPrivileges struct { UserProto github_com_cockroachdb_cockroach_pkg_security.SQLUsernameProto `protobuf:"bytes,1,opt,name=user_proto,json=userProto,casttype=github.com/cockroachdb/cockroach/pkg/security.SQLUsernameProto" json:"user_proto"` // privileges is a bitfield of 1<= 64 { + return ErrIntOverflowPrivilege + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + m.WithGrantOption |= uint32(b&0x7F) << shift + if b < 0x80 { + break + } + } default: iNdEx = preIndex skippy, err := skipPrivilege(dAtA[iNdEx:]) diff --git a/pkg/sql/catalog/descpb/privilege.proto b/pkg/sql/catalog/descpb/privilege.proto index a5ac53137f29..538b88dc1686 100644 --- a/pkg/sql/catalog/descpb/privilege.proto +++ b/pkg/sql/catalog/descpb/privilege.proto @@ -21,6 +21,7 @@ message UserPrivileges { (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/security.SQLUsernameProto"]; // privileges is a bitfield of 1< ACL character +var privToACL = map[Kind]string{ + CREATE: "C", + SELECT: "r", + INSERT: "a", + DELETE: "d", + UPDATE: "w", + USAGE: "U", + CONNECT: "c", +} + +// orderedPrivs is the list of privileges sorted in alphanumeric order based on the ACL character -> CUacdrw +var orderedPrivs = List{CREATE, USAGE, INSERT, CONNECT, DELETE, SELECT, UPDATE} + // ListToACL converts a list of privileges to a list of Postgres // ACL items. // See: https://www.postgresql.org/docs/13/ddl-priv.html#PRIVILEGE-ABBREVS-TABLE // for privileges and their ACL abbreviations. -func (pl List) ListToACL(objectType ObjectType) string { +func (pl List) ListToACL(grantOptions List, objectType ObjectType) string { privileges := pl // If ALL is present, explode ALL into the underlying privileges. if pl.Contains(ALL) { privileges = GetValidPrivilegesForObject(objectType) + if grantOptions.Contains(ALL) { + grantOptions = GetValidPrivilegesForObject(objectType) + } } chars := make([]string, len(privileges)) - for _, privilege := range privileges { - switch privilege { - case CREATE: - chars = append(chars, "C") - case SELECT: - chars = append(chars, "r") - case INSERT: - chars = append(chars, "a") - case DELETE: - chars = append(chars, "d") - case UPDATE: - chars = append(chars, "w") - case USAGE: - chars = append(chars, "U") - case CONNECT: - chars = append(chars, "c") + for _, privilege := range orderedPrivs { + if _, ok := privToACL[privilege]; !ok { + continue + } + if privileges.Contains(privilege) { + chars = append(chars, privToACL[privilege]) + } + if grantOptions.Contains(privilege) { + chars = append(chars, "*") } } - sort.Strings(chars) + return strings.Join(chars, "") + } diff --git a/pkg/sql/sem/tree/grant.go b/pkg/sql/sem/tree/grant.go index 88ef29dc4367..085629e9883a 100644 --- a/pkg/sql/sem/tree/grant.go +++ b/pkg/sql/sem/tree/grant.go @@ -28,9 +28,10 @@ import ( // Grant represents a GRANT statement. type Grant struct { - Privileges privilege.List - Targets TargetList - Grantees RoleSpecList + Privileges privilege.List + Targets TargetList + Grantees RoleSpecList + WithGrantOption bool } // TargetList represents a list of targets. diff --git a/pkg/sql/sem/tree/revoke.go b/pkg/sql/sem/tree/revoke.go index bac0a86dd1c5..69bf49983216 100644 --- a/pkg/sql/sem/tree/revoke.go +++ b/pkg/sql/sem/tree/revoke.go @@ -24,9 +24,10 @@ import "github.com/cockroachdb/cockroach/pkg/sql/privilege" // Revoke represents a REVOKE statement. // PrivilegeList and TargetList are defined in grant.go type Revoke struct { - Privileges privilege.List - Targets TargetList - Grantees RoleSpecList + Privileges privilege.List + Targets TargetList + Grantees RoleSpecList + GrantOptionFor bool } // Format implements the NodeFormatter interface.