Skip to content

Commit

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

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 SETPASSWORD/NOSETPASSWORD`:
  granting or removing the option for a user or role.

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

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): Initializing or changing the password
of a SQL user, as well as setting the expiration date for a password,
now requires the new `SETPASSWORD` option to be set for the requesting
user or one of its roles. Previously, only the `CREATEROLE` option was
sufficient to perform these changes. The pseudo-option `NOSETPASSWORD`
can be used to revoke `SETPASSWORD`.

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

Release note (security update): Only a user which already has
option `SETPASSWORD` (either itself or one of its roles) can grant
this option or use `NOSETPASSWORD`.
  • Loading branch information
knz committed Jun 24, 2020
1 parent 12494b0 commit c959dd1
Show file tree
Hide file tree
Showing 16 changed files with 346 additions and 145 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,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-7</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-8</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 @@ -760,6 +760,7 @@ unreserved_keyword ::=
| 'NO_INDEX_JOIN'
| 'NOCREATEROLE'
| 'NOLOGIN'
| 'NOSETPASSWORD'
| 'NOWAIT'
| 'NULLS'
| 'IGNORE_FOREIGN_KEYS'
Expand Down Expand Up @@ -813,6 +814,7 @@ unreserved_keyword ::=
| 'ROLLUP'
| 'ROWS'
| 'RULE'
| 'SETPASSWORD'
| 'SETTING'
| 'SETTINGS'
| 'STATUS'
Expand Down Expand Up @@ -1805,6 +1807,8 @@ role_option ::=
| 'NOCREATEROLE'
| 'LOGIN'
| 'NOLOGIN'
| 'SETPASSWORD'
| 'NOSETPASSWORD'
| password_clause
| valid_until_clause

Expand Down
8 changes: 8 additions & 0 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const (
VersionAlterColumnTypeGeneral
VersionAlterSystemJobsAddCreatedByColumns
VersionAddScheduledJobsTable
VersionSetPasswordPrivilege

// Add new versions here (step one of two).
)
Expand Down Expand Up @@ -511,6 +512,13 @@ var versionsSingleton = keyedVersions([]keyedVersion{
Key: VersionAddScheduledJobsTable,
Version: roachpb.Version{Major: 20, Minor: 1, Unstable: 7},
},
{
// VersionSetPasswordPrivilege is when SETPASSWORD/NOSETPASSWORD are introduced.
//
// It represents adding password management via SETPASSWORD role option.
Key: VersionSetPasswordPrivilege,
Version: roachpb.Version{Major: 20, Minor: 1, Unstable: 8},
},

// 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.

34 changes: 34 additions & 0 deletions pkg/sql/alter_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ func (p *planner) AlterRoleNode(
return nil, err
}

// Check that the requested combination of
// PASSWORD/SETPASSWORD/NOSETPASSWORD is compatible with the user's
// own SETPASSWORD privilege.
if err := p.checkPasswordOptionConstraints(ctx, roleOptions); err != nil {
return nil, err
}

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

func (p *planner) checkPasswordOptionConstraints(
ctx context.Context, roleOptions roleoption.List,
) error {
if !p.EvalContext().Settings.Version.IsActive(ctx, clusterversion.VersionSetPasswordPrivilege) {
// TODO(knz): Remove this condition in 21.1.
if roleOptions.Contains(roleoption.SETPASSWORD) || roleOptions.Contains(roleoption.NOSETPASSWORD) {
return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
`granting SETPASSWORD or NOSETPASSWORD requires all nodes to be upgraded to %s`,
clusterversion.VersionByKey(clusterversion.VersionSetPasswordPrivilege))
}
} else {
if roleOptions.Contains(roleoption.SETPASSWORD) ||
roleOptions.Contains(roleoption.NOSETPASSWORD) ||
roleOptions.Contains(roleoption.PASSWORD) ||
roleOptions.Contains(roleoption.VALIDUNTIL) {
// Only a role who has SETPASSWORD itself can grant SETPASSWORD
// or NOSETPASSWORD to another role, or set up a password for
// authentication, or set up password validity, even if they
// have CREATEROLE privilege.
if err := p.HasRoleOption(ctx, roleoption.SETPASSWORD); err != nil {
return err
}
}
}
return nil
}

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

// Using CREATE ROLE syntax enables NOLOGIN by default.
if isRole && !roleOptions.Contains(roleoption.LOGIN) &&
Expand All @@ -74,11 +77,14 @@ func (p *planner) CreateRoleNode(
roleoption.RoleOption{Option: roleoption.NOLOGIN, HasValue: false})
}

if err != nil {
if err := roleOptions.CheckRoleOptionConflicts(); err != nil {
return nil, err
}

if err := roleOptions.CheckRoleOptionConflicts(); err != nil {
// Check that the requested combination of
// PASSWORD/SETPASSWORD/NOSETPASSWORD is compatible with the user's
// own SETPASSWORD privilege.
if err := p.checkPasswordOptionConstraints(ctx, roleOptions); err != nil {
return nil, err
}

Expand Down Expand Up @@ -112,6 +118,10 @@ func (n *CreateRoleNode) startExec(params runParams) error {

var hashedPassword []byte
if n.roleOptions.Contains(roleoption.PASSWORD) {
if err := params.p.HasRoleOption(params.ctx, roleoption.SETPASSWORD); err != nil {
return err
}

hashedPassword, err = n.roleOptions.GetHashedPassword()
if err != nil {
return 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 CREATEROLE, SETPASSWORD {}
root CREATEROLE, SETPASSWORD {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 CREATEROLE, SETPASSWORD {}
root CREATEROLE, SETPASSWORD {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 CREATEROLE, SETPASSWORD {}
root CREATEROLE, SETPASSWORD {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 CREATEROLE, SETPASSWORD {}
root CREATEROLE, SETPASSWORD {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 CREATEROLE, SETPASSWORD {}
root CREATEROLE, SETPASSWORD {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 CREATEROLE, SETPASSWORD {}
root CREATEROLE, SETPASSWORD {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 CREATEROLE, SETPASSWORD {}
root CREATEROLE, SETPASSWORD {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 CREATEROLE, SETPASSWORD {}
root CREATEROLE, SETPASSWORD {admin}
testuser · {}

user testuser

Expand Down
Loading

0 comments on commit c959dd1

Please sign in to comment.