| Bug #118046 | KILL (QUERY) should check thread ownership against the security context that initiated the current operation | ||
|---|---|---|---|
| Submitted: | 23 Apr 21:21 | Modified: | 22 May 10:33 |
| Reporter: | Troels Madsen | Email Updates: | |
| Status: | Verified | Impact on me: | |
| Category: | MySQL Server: Stored Routines | Severity: | S2 (Serious) |
| Version: | 8.0 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[19 May 14:44]
MySQL Verification Team
Hello Troels Madsen, Thank you for the report and feedback. My sincere apologises for the late response. I wanted to ensure I had all the necessary information before replying on this. IMHO this is not a bug but expected behaviour i.e. My understanding is that - if a stored routine is created with SQL SECURITY DEFINER and then invoked by an account other than the definer, the invoker cannot kill the thread using KILL because the definer has the privileges to kill the thread, but not the invoker. Of course, if the invoker has privileges to KILL then it can or There was a related bug was raised in the past - Bug #57445 Some references in this context - https://dev.mysql.com/doc/refman/8.0/en/stored-objects-security.html#stored-objects-securi... https://dev.mysql.com/doc/refman/8.0/en/create-procedure.html regards, Umesh
[22 May 9:55]
Troels Madsen
I respectfully disagree with the conclusion that the invoker shouldn't be allowed to kill a running stored procedure on its own thread if that procedure is created with SQL SECURITY DEFINER to another account. The purpose of SQL SECURITY DEFINER is to control under what credentials the stored procedure executes, and thus what the stored can access / update while running. The ability to run (/start) the stored procedure is however controlled by GRANT EXECUTE. And if you have given an account the right to start a stored procedure, that account should then IMHO also be allowed to stop / abort instances of that stored procedure running on its own threads. The reason this change is important is for (production) scenarios where a query (from a stored procedure with sql security definer) suddenly starts taking much longer time / using a lot more resources (e.g. due to data changes). In that scenarios the client libraries will typically about the stored procedure call after 30 second, and try to kill the running query. But in this scenario the KILL QUERY fails, and we end up with a situation where the client application is disconnected from the query, while the query continues running and using resources till its either killed by a DBA or it completes and delivers it data to nowhere.
[22 May 10:33]
MySQL Verification Team
Thank you for the feedback. regards, Umesh
Description: Presently the permission to kill a query / connection is done against the current security context. But when executing e.g. a stored procedure executing under a different user, this check will fail. This means you can't kill your own queries in this (typical) scenario. Most client side libraries has includes the ability to specify a query timeout, and if the query timeout is exceeded, the client side library will typically try to kill the long running query. But if the thread is executing a stored procedure running under a different user, the attempt to cleanup will fail. As a result the client side will have abandoned waiting for the result, but the long running query continues burning resources on the server! How to repeat: Create a long running (e.g. doing a sleep) stored procedure using security definer, with user B as definer. Grant user A execute right on the stored procedure. Start two connections as user A. Call the stored procedure from the first connection. Identify the running thread and try to kill it using KILL QUERY from the second connection. Suggested fix: In sql_parse.cc, method kill_one_thread, line 6467, change from if (sctx->check_access(SUPER_ACL) || sctx->has_global_grant(STRING_WITH_LEN("CONNECTION_ADMIN")).first || sctx->user_matches(tmp->security_context())) { to if (sctx->check_access(SUPER_ACL) || sctx->has_global_grant(STRING_WITH_LEN("CONNECTION_ADMIN")).first || sctx->user_matches(&(tmp->m_main_security_ctx))) { (( preferably with a new getter method for &(tmp->m_main_security_ctx) ))