From 087f61cd5c4832975099a6432afea7848145237e Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Fri, 9 Oct 2020 10:46:26 +0200 Subject: [PATCH] sql: ensure that CREATELOGIN is needed for CREATE USER Release note (bug fix): The `CREATE USER` statement without explicit NOLOGIN option implicitly grants LOGIN, and so requires the CREATELOGIN privilege. This was not checked properly, and is now enforced. This bug was introduced earlier in the v20.2 development cycle. --- pkg/sql/alter_role.go | 9 ++++++--- pkg/sql/create_role.go | 12 ++++++------ pkg/sql/logictest/testdata/logic_test/role | 18 ++++++++++++++---- pkg/sql/logictest/testdata/logic_test/user | 12 ++++++++++-- 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/pkg/sql/alter_role.go b/pkg/sql/alter_role.go index c735d0f964b9..8d3133c22b25 100644 --- a/pkg/sql/alter_role.go +++ b/pkg/sql/alter_role.go @@ -98,10 +98,13 @@ func (p *planner) checkPasswordOptionConstraints( if roleOptions.Contains(roleoption.CREATELOGIN) || roleOptions.Contains(roleoption.NOCREATELOGIN) || - roleOptions.Contains(roleoption.LOGIN) || - (roleOptions.Contains(roleoption.NOLOGIN) && !newUser) || // CREATE ROLE NOLOGIN is valid without CREATELOGIN. roleOptions.Contains(roleoption.PASSWORD) || - roleOptions.Contains(roleoption.VALIDUNTIL) { + roleOptions.Contains(roleoption.VALIDUNTIL) || + roleOptions.Contains(roleoption.LOGIN) || + // CREATE ROLE NOLOGIN is valid without CREATELOGIN. + (roleOptions.Contains(roleoption.NOLOGIN) && !newUser) || + // Disallow implicit LOGIN upon new user. + (newUser && !roleOptions.Contains(roleoption.NOLOGIN) && !roleOptions.Contains(roleoption.LOGIN)) { // Only a role who has CREATELOGIN itself can grant CREATELOGIN or // NOCREATELOGIN to another role, or set up a password for // authentication, or set up password validity, or enable/disable diff --git a/pkg/sql/create_role.go b/pkg/sql/create_role.go index 66e1033d6f9a..0f222f3628c2 100644 --- a/pkg/sql/create_role.go +++ b/pkg/sql/create_role.go @@ -75,18 +75,18 @@ func (p *planner) CreateRoleNode( return nil, err } - // Check that the requested combination of password options is - // compatible with the user's own CREATELOGIN privilege. - if err := p.checkPasswordOptionConstraints(ctx, roleOptions, true /* newUser */); err != nil { - return nil, err - } - // Using CREATE ROLE syntax enables NOLOGIN by default. if isRole && !roleOptions.Contains(roleoption.LOGIN) && !roleOptions.Contains(roleoption.NOLOGIN) { roleOptions = append(roleOptions, roleoption.RoleOption{Option: roleoption.NOLOGIN, HasValue: false}) } + // Check that the requested combination of password options is + // compatible with the user's own CREATELOGIN privilege. + if err := p.checkPasswordOptionConstraints(ctx, roleOptions, true /* newUser */); err != nil { + return nil, err + } + ua, err := p.getUserAuthInfo(ctx, nameE, opName) if err != nil { return nil, err diff --git a/pkg/sql/logictest/testdata/logic_test/role b/pkg/sql/logictest/testdata/logic_test/role index 44e73b97fb57..a640864a7a74 100644 --- a/pkg/sql/logictest/testdata/logic_test/role +++ b/pkg/sql/logictest/testdata/logic_test/role @@ -891,7 +891,7 @@ ALTER ROLE rolewithcreate WITH NOCREATEROLE user root statement ok -ALTER USER testuser CREATEROLE +ALTER USER testuser CREATEROLE CREATELOGIN user testuser @@ -1200,12 +1200,16 @@ CREATE USER testuser3 LOGIN statement error user testuser does not have CREATELOGIN privilege CREATE ROLE testrole3 LOGIN -# However it's possible to create a user/role with NOLOGIN. +statement error user testuser does not have CREATELOGIN privilege +CREATE USER testuser2 + +# CREATE ROLE implies NOLOGIN, which is OK statement ok -CREATE USER testuser4 NOLOGIN +CREATE ROLE testuser4 +# It's also possible to create a user/role with NOLOGIN. statement ok -CREATE USER testuser2 +CREATE USER testuser2 NOLOGIN statement error user testuser does not have CREATELOGIN privilege ALTER USER testuser2 WITH PASSWORD 'abc' @@ -1308,6 +1312,12 @@ ALTER USER testuser NOCREATELOGIN user testuser +statement error user testuser does not have CREATELOGIN privilege +CREATE USER testuser5 + +statement ok +CREATE ROLE testuser6 + statement error user testuser does not have CREATELOGIN privilege CREATE USER testuser5 WITH PASSWORD 'abc' diff --git a/pkg/sql/logictest/testdata/logic_test/user b/pkg/sql/logictest/testdata/logic_test/user index 49ff1d3e982f..3ddb39fa46ed 100644 --- a/pkg/sql/logictest/testdata/logic_test/user +++ b/pkg/sql/logictest/testdata/logic_test/user @@ -143,16 +143,24 @@ GRANT SELECT ON system.role_options to testuser user testuser statement ok -CREATE USER user4 CREATEROLE +CREATE ROLE user4 CREATEROLE + +statement ok +CREATE USER user5 NOLOGIN query TTT SELECT * FROM system.role_options ---- testuser CREATEROLE NULL user4 CREATEROLE NULL +user4 NOLOGIN NULL +user5 NOLOGIN NULL + +statement ok +DROP ROLE user4 statement ok -DROP USER user4 +DROP ROLE user5 subtest min_password_length