sql: ensures that users can't access the DB after DROP USER #20718
Comments
|
@vivek for triage. Users can't be dropped when they still have privileges. And privilege checks should fail for all queries (transaction, time-travelling queries, etc...) (if not, that's a bug). |
|
There are queries that could consume a lot of server resources that don't touch tables, like It would be helpful for this user to have sessions for deleted users get terminated. |
|
I agree this is wanted. I'm just saying turning error deletion into a session termination is not a trivial thing. But then again, I don't know anything about how we hook sessions into things. |
|
There's an RFC for session cancellation: #17252. |
|
Session cancellation is marked for 2.0 in Airtable. @vivekmenezes mentioning punting on the decision on how would implement it until Jan. |
|
pinged the RFC |
knz
added
A-sql-privileges
|
cc @aaron-crl |
|
Looks like this would fall onto SQL Features team, so I'll move the issue in their project. |
|
although this seems like a reasonable idea, I tested with Postgres and observed that neither DROP USER nor ALTER USER NOLOGIN cause an existing session for that user to be terminated. so i'll consider this "blocked" until we hear from product or our customers that this issue is important to address. i'm worried about accidentally breaking compatibility if we implement this without being sure. |
knz
added
A-authentication
knz
changed the title
exalate-issue-sync
bot
added
T-sql-foundations
111662: opt: add lock operator r=DrewKimball a=michae2 **execbuilder: change error message to match PostgreSQL** Release note (sql change): Change the following error message: ``` statement error cannot execute FOR UPDATE in a read-only transaction ``` to this to match PostgreSQL: ``` statement error cannot execute SELECT FOR UPDATE in a read-only transaction ``` **opt: add lock operator** Add a new implementation of `SELECT FOR UPDATE` and `SELECT FOR SHARE` statements. Instead of locking during the initial row fetch, this new implementation constructs a `Lock` operator on the top of the query plan which performs the locking phase using a locking semi-join lookup. During optbuilder we build plans with both `Lock` operators and initial-row-fetch locking. During execbuilder we decide which implementation to use based on the isolation level and whether `optimizer_use_lock_op_for_serializable` is set. If the new implementation is chosen, `Lock` operators become locking semi-LookupJoins. In some cases these new plans will have superfluous lookup joins. A future PR will optimize away some of these superfluous lookup joins. Fixes: #57031, #75457 Epic: CRDB-25322 Release note (sql change): Add a new session variable, `optimizer_use_lock_op_for_serializable`, which when set enables a new implementation of `SELECT FOR UPDATE`. This new implementation of `SELECT FOR UPDATE` acquires row locks *after* any joins and filtering, and always acquires row locks on the primary index of the table being locked. This more closely matches `SELECT FOR UPDATE` behavior in PostgreSQL, but at the cost of more round trips from gateway node to replica leaseholder. Under read committed isolation (and other isolation levels weaker than serializable) we will always use this new implementation of `SELECT FOR UPDATE` regardless of the value of `optimizer_use_lock_op_for_serializable` to ensure correctness. 111934: sql: cache role existence checks to fix perf regression r=rafiss a=rafiss In 12ec26f, we started to check for the existence of a role whenever a privilege for the public role was checked. This can hapen multiple times during some pg_catalog queries, so it introduced a performance regression. Now, the role existence is cached so we avoid the penalty. The existence of a role is cached for the duration of the entire transaction, and also gets inherited by any internal executor used to implement a command run by the user's transaction. No release note since this is new in 23.2. informs #20718 fixes #112102 Release note: None Co-authored-by: Michael Erickson <[email protected]> Co-authored-by: Rafi Shamim <[email protected]>
In both secure and insecure modes, a connected user is permitted to stay connected and run queries even after the user has been
DROPped.This is counterintuitive and possibly a security risk.
Edit 2023-03-29 from @rafiss:
Mitigation
a deactivated/deleted user can be "ejected" as follows:
CANCEL SESSIONSwith a suitable SELECT clause that selects only that user's session.system.web_sessions.Jira issue: CRDB-5913
Epic CRDB-26131
The text was updated successfully, but these errors were encountered: