Skip to content

Commit

Permalink
sql: introduce the new role option CREATELOGIN
Browse files Browse the repository at this point in the history
This commit introduces a new SQL-level role option (alongside LOGIN
etc) called `CREATELOGIN`.

This is used to control access to:

- the `WITH PASSWORD` clause for `CREATE/ALTER USER/ROLE`:
  initializing, changing or removing password on users.

- the `VALID UNTIL` clause for `CREATE/ALTER USER/ROLE`:
  changing the expiration date for a password.

- `ALTER USER/ROLE CREATELOGIN/NOCREATELOGIN`:
  granting or removing the option for a user or role.

- `ALTER USER/ROLE LOGIN/NOLOGIN`:
  enabling a user/role to log in using either passwords, certs or
  another valid auth method

This feature enables a site operator to separate the responsibility of
administrating users and roles, which requires the CREATEROLE
privilege, from that of administrating authentication principals,
which now requires CREATELOGIN.

For example, it can be used to ensure that no SQL client can choose
their own passwords for users/roles, even if they are privileged
enough to create new users or roles.

Release note (security update): Defining or changing authentication
principals or their credentials now requires the new `CREATELOGIN`
option to be set for the requesting user or one of its roles. This
includes setting/removing the LOGIN option (wether the principal can
log in); initializing or changing the password of a SQL user, as well
as setting the expiration date for a password.  Previously, only the
`CREATEROLE` option was sufficient to perform these changes. The
pseudo-option `NOCREATELOGIN` can be used to revoke `CREATELOGIN`.

The two predefined `root` and `admin` roles have the option
`CREATELOGIN` set by default.

Release note (security update): Only a user which already has
option `CREATELOGIN` (either itself or one of its roles) can grant
this option or use `NOCREATELOGIN`.
  • Loading branch information
knz committed Jul 17, 2020
1 parent 5149293 commit 0e24be5
Show file tree
Hide file tree
Showing 16 changed files with 418 additions and 158 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,6 @@
<tr><td><code>trace.debug.enable</code></td><td>boolean</td><td><code>false</code></td><td>if set, traces for recent requests can be seen in the /debug page</td></tr>
<tr><td><code>trace.lightstep.token</code></td><td>string</td><td><code></code></td><td>if set, traces go to Lightstep using this token</td></tr>
<tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>if set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'); ignored if trace.lightstep.token is set</td></tr>
<tr><td><code>version</code></td><td>custom validation</td><td><code>20.1-13</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
<tr><td><code>version</code></td><td>custom validation</td><td><code>20.1-14</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
</tbody>
</table>
4 changes: 4 additions & 0 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,7 @@ unreserved_keyword ::=
| 'CONVERSION'
| 'COPY'
| 'COVERING'
| 'CREATELOGIN'
| 'CREATEROLE'
| 'CUBE'
| 'CURRENT'
Expand Down Expand Up @@ -774,6 +775,7 @@ unreserved_keyword ::=
| 'NO'
| 'NORMAL'
| 'NO_INDEX_JOIN'
| 'NOCREATELOGIN'
| 'NOCREATEROLE'
| 'NOLOGIN'
| 'NOWAIT'
Expand Down Expand Up @@ -1859,6 +1861,8 @@ role_option ::=
| 'NOCREATEROLE'
| 'LOGIN'
| 'NOLOGIN'
| 'CREATELOGIN'
| 'NOCREATELOGIN'
| password_clause
| valid_until_clause

Expand Down
10 changes: 10 additions & 0 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const (
VersionNodeMembershipStatus
VersionRangeStatsRespHasDesc
VersionMinPasswordLength
VersionCreateLoginPrivilege

// Add new versions here (step one of two).
)
Expand Down Expand Up @@ -551,6 +552,15 @@ var versionsSingleton = keyedVersions([]keyedVersion{
Key: VersionMinPasswordLength,
Version: roachpb.Version{Major: 20, Minor: 1, Unstable: 13},
},
{
// VersionCreateLoginPrivilege is when CREATELOGIN/NOCREATELOGIN
// are introduced.
//
// It represents adding authn principal management via CREATELOGIN
// role option.
Key: VersionCreateLoginPrivilege,
Version: roachpb.Version{Major: 20, Minor: 1, Unstable: 14},
},

// Add new versions here (step two of two).

Expand Down
5 changes: 3 additions & 2 deletions pkg/clusterversion/versionkey_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions pkg/sql/alter_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"context"
"fmt"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
Expand Down Expand Up @@ -64,6 +65,12 @@ func (p *planner) AlterRoleNode(
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, false /* newUser */); err != nil {
return nil, err
}

ua, err := p.getUserAuthInfo(ctx, nameE, opName)
if err != nil {
return nil, err
Expand All @@ -77,6 +84,35 @@ func (p *planner) AlterRoleNode(
}, nil
}

func (p *planner) checkPasswordOptionConstraints(
ctx context.Context, roleOptions roleoption.List, newUser bool,
) error {
if !p.EvalContext().Settings.Version.IsActive(ctx, clusterversion.VersionCreateLoginPrivilege) {
// TODO(knz): Remove this condition in 21.1.
if roleOptions.Contains(roleoption.CREATELOGIN) || roleOptions.Contains(roleoption.NOCREATELOGIN) {
return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
`granting CREATELOGIN or NOCREATELOGIN requires all nodes to be upgraded to %s`,
clusterversion.VersionByKey(clusterversion.VersionCreateLoginPrivilege))
}
}

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) {
// 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
// LOGIN privilege; even if they have CREATEROLE privilege.
if err := p.HasRoleOption(ctx, roleoption.CREATELOGIN); err != nil {
return err
}
}
return nil
}

func (n *alterRoleNode) startExec(params runParams) error {
var opName string
if n.isRole {
Expand Down
20 changes: 12 additions & 8 deletions pkg/sql/create_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,6 @@ func (p *planner) CreateRoleNode(
return p.TypeAsStringOrNull(ctx, e, op)
}
roleOptions, err := kvOptions.ToRoleOptions(asStringOrNull, opName)

// 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})
}

if err != nil {
return nil, err
}
Expand All @@ -83,6 +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})
}

ua, err := p.getUserAuthInfo(ctx, nameE, opName)
if err != nil {
return nil, err
Expand Down
84 changes: 42 additions & 42 deletions pkg/sql/logictest/testdata/logic_test/drop_user
Original file line number Diff line number Diff line change
Expand Up @@ -6,45 +6,45 @@ CREATE USER user1
query TTT colnames
SHOW USERS
----
username options member_of
admin CREATEROLE {}
root CREATEROLE {admin}
testuser · {}
user1 · {}
username options member_of
admin CREATELOGIN, CREATEROLE {}
root CREATELOGIN, CREATEROLE {admin}
testuser · {}
user1 · {}

statement ok
DROP USER user1

query TTT colnames
SHOW USERS
----
username options member_of
admin CREATEROLE {}
root CREATEROLE {admin}
testuser · {}
username options member_of
admin CREATELOGIN, CREATEROLE {}
root CREATELOGIN, CREATEROLE {admin}
testuser · {}

statement ok
CREATE USER user1

query TTT colnames
SHOW USERS
----
username options member_of
admin CREATEROLE {}
root CREATEROLE {admin}
testuser · {}
user1 · {}
username options member_of
admin CREATELOGIN, CREATEROLE {}
root CREATELOGIN, CREATEROLE {admin}
testuser · {}
user1 · {}

statement ok
DROP USER USEr1

query TTT colnames
SHOW USERS
----
username options member_of
admin CREATEROLE {}
root CREATEROLE {admin}
testuser · {}
username options member_of
admin CREATELOGIN, CREATEROLE {}
root CREATELOGIN, CREATEROLE {admin}
testuser · {}

statement error user user1 does not exist
DROP USER user1
Expand Down Expand Up @@ -76,40 +76,40 @@ CREATE USER user4
query TTT colnames
SHOW USERS
----
username options member_of
admin CREATEROLE {}
root CREATEROLE {admin}
testuser · {}
user1 · {}
user2 · {}
user3 · {}
user4 · {}
username options member_of
admin CREATELOGIN, CREATEROLE {}
root CREATELOGIN, CREATEROLE {admin}
testuser · {}
user1 · {}
user2 · {}
user3 · {}
user4 · {}

statement ok
DROP USER user1,user2

query TTT colnames
SHOW USERS
----
username options member_of
admin CREATEROLE {}
root CREATEROLE {admin}
testuser · {}
user3 · {}
user4 · {}
username options member_of
admin CREATELOGIN, CREATEROLE {}
root CREATELOGIN, CREATEROLE {admin}
testuser · {}
user3 · {}
user4 · {}

statement error user user1 does not exist
DROP USER user1,user3

query TTT colnames
SHOW USERS
----
username options member_of
admin CREATEROLE {}
root CREATEROLE {admin}
testuser · {}
user3 · {}
user4 · {}
username options member_of
admin CREATELOGIN, CREATEROLE {}
root CREATELOGIN, CREATEROLE {admin}
testuser · {}
user3 · {}
user4 · {}

statement ok
CREATE USER user1
Expand Down Expand Up @@ -137,10 +137,10 @@ PREPARE du AS DROP USER $1;
query TTT colnames
SHOW USERS
----
username options member_of
admin CREATEROLE {}
root CREATEROLE {admin}
testuser · {}
username options member_of
admin CREATELOGIN, CREATEROLE {}
root CREATELOGIN, CREATEROLE {admin}
testuser · {}

user testuser

Expand Down
Loading

0 comments on commit 0e24be5

Please sign in to comment.