Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GRANT SYSTEM ALL removes login privilege from role #101292

Closed
hand-crdb opened this issue Apr 12, 2023 · 2 comments · Fixed by #104685
Closed

GRANT SYSTEM ALL removes login privilege from role #101292

hand-crdb opened this issue Apr 12, 2023 · 2 comments · Fixed by #104685
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@hand-crdb
Copy link
Contributor

hand-crdb commented Apr 12, 2023

Describe the problem

The GRANT SYSTEM ALL statement removes login permission from a role.

To Reproduce

To reproduce

  1. Create a user
  2. Verify that the user can log in
  3. Grant SYSTEM ALL privileges to the user
  4. Verify that the user can no longer log in

If possible, provide steps to reproduce the behavior:

▶ cockroach start-single-node --insecure --background
*
* WARNING: ALL SECURITY CONTROLS HAVE BEEN DISABLED!
* 
* This mode is intended for non-production testing only.
* 
* In this mode:
* - Your cluster is open to any client that can access any of your IP addresses.
* - Intruders with access to your machine or network can observe client-server traffic.
* - Intruders can log in without password and read or write any data in the cluster.
* - Intruders can consume all your server's resources and cause unavailability.
*
*
* INFO: To start a secure server without mandating TLS for clients,
* consider --accept-sql-without-tls instead. For other options, see:
* 
* - https://go.crdb.dev/issue-v/53404/v22.2
* - https://www.cockroachlabs.com/docs/v22.2/secure-a-cluster.html
*
*
* WARNING: Running a server without --sql-addr, with a combined RPC/SQL listener, is deprecated.
* This feature will be removed in the next version of CockroachDB.
*
*
* WARNING: neither --listen-addr nor --advertise-addr was specified.
* The server will advertise "crlMBP-NRWLD7WPC6MTc1.local" to other nodes, is this routable?
* 
* Consider using:
* - for local-only servers:  --listen-addr=localhost:36257 --sql-addr=localhost:26257
* - for multi-node clusters: --listen-addr=:36257 --sql-addr=:26257 --advertise-addr=<host/IP addr>
* 
*

▶ cockroach sql --insecure -e "show users";
  username | options | member_of
-----------+---------+------------
  admin    |         | {}
  root     |         | {admin}
(2 rows)


Time: 20ms


▶ cockroach sql --insecure -e "create user abc";
CREATE ROLE


Time: 114ms


▶ cockroach sql --insecure --user abc -e "show databases";
  database_name | owner | primary_region | secondary_region | regions | survival_goal
----------------+-------+----------------+------------------+---------+----------------
  defaultdb     | root  | NULL           | NULL             | {}      | NULL
  postgres      | root  | NULL           | NULL             | {}      | NULL
(2 rows)


Time: 2ms


▶ cockroach sql --insecure -e "grant system all to abc";  
GRANT


Time: 88ms


▶ cockroach sql --insecure --user abc -e "show databases";
ERROR: abc does not have login privilege
SQLSTATE: 28000
Failed running "sql"

Expected behavior

The user should not lose the login privilege when they are granted all system privileges.

Environment:

  • CockroachDB version: 22.2.7
  • Server OS: reproduced on Linux and MacOS
  • Client app: cockroach sql

Jira issue: CRDB-26895

Epic CRDB-27601

@hand-crdb hand-crdb added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Apr 12, 2023
@blathers-crl
Copy link

blathers-crl bot commented Apr 12, 2023

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I was unable to automatically find someone to ping.

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner labels Apr 12, 2023
@yuzefovich yuzefovich removed the X-blathers-untriaged blathers was unable to find an owner label Apr 12, 2023
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Apr 12, 2023
@rafiss
Copy link
Collaborator

rafiss commented Jun 9, 2023

Hm, the reason for this is that ALL includes the NOSQLLOGIN privilege. We may have to special case the privilege checking so that if a user has ALL, they are not considered to have NOSQLLOGIN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants