Fix counter leak in nonsuper_connections

Summary: If a non-super user connects to mysqld with an invalid database name, nonsuper_connections is incremented, but never decremented. Test Plan: mtr Reviewers: luqun, mung, yoshinori, yzha Reviewed By: yzha Subscribers: butterflybot, vinaybhat, [email protected] Differential Revision: https://phabricator.intern.facebook.com/D15104315 Tasks: T43597145 Signature: 15104315:1556319471:4a89b4301f2dbf56bb3b57b59cdeb7bf8b4bc590
percona-ysorokin · Apr 26, 2019 · a9083cc · a9083cc
1 parent c87c2d6
commit a9083cc
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 4 deletions.
diff --git a/mysql-test/r/max_nonsuper_connections.result b/mysql-test/r/max_nonsuper_connections.result
@@ -50,6 +50,27 @@ disconnect con4;
 disconnect con3;
 disconnect con2;
 disconnect con1;
+Verifying nonsuper_connections is decremented when db is denied
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
+ERROR 42000: Access denied for user 'test_user'@'localhost' to database 'bogus_db'
 connection default;
 connect  con$i, localhost, test_user,,test;
 connect  con$i, localhost, test_user,,test;

diff --git a/mysql-test/t/max_nonsuper_connections.test b/mysql-test/t/max_nonsuper_connections.test
@@ -96,6 +96,18 @@ while ($i)
   dec $i;
 }
 
+# Verify counter leak is fixed when permission to a database is denied
+--echo Verifying nonsuper_connections is decremented when db is denied
+disable_query_log;
+let $i = 20;
+while ($i)
+{
+  --error ER_DBACCESS_DENIED_ERROR
+  connect (con_denied_$i, localhost, test_user,,bogus_db);
+  dec $i;
+}
+enable_query_log;
+
 # able to refill up max_nonsuper_connections
 connection default;
 let $i = 10;

diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
@@ -11445,9 +11445,13 @@ acl_authenticate(THD *thd, uint com_change_user_pkt_len)
   if (thd->is_admin_connection() &&
       !(thd->main_security_ctx.master_access & SUPER_ACL) &&
       !(thd->main_security_ctx.master_access & ADMIN_PORT_ACL)) {
+    USER_CONN *uc = const_cast<USER_CONN*>(thd->get_user_connect());
+    if (uc) {
+      uc->user_stats.errors_access_denied.inc();
+    }
     my_error(ER_SPECIFIC_ACCESS_DENIED_ERROR, MYF(0), "SUPER, ADMIN_PORT");
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
-    fix_user_conn(thd, ADMIN_PORT); // Undo work by
+    fix_user_conn(thd, OTHER_ACCESS); // Undo work by
     // get_or_create_user_conn
 #endif
     DBUG_RETURN (1);
@@ -11510,7 +11514,11 @@ acl_authenticate(THD *thd, uint com_change_user_pkt_len)
       USER_CONN* uc = const_cast<USER_CONN*>(thd->get_user_connect());
       if (uc)
       {
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+        fix_user_conn(thd, OTHER_ACCESS);
+#else
         decrease_user_connections(uc);
+#endif
         uc->user_stats.errors_access_denied.inc();
       }
       Host_errors errors;
@@ -11544,8 +11552,12 @@ acl_authenticate(THD *thd, uint com_change_user_pkt_len)
       USER_CONN* uc = const_cast<USER_CONN*>(thd->get_user_connect());
       if (uc)
       {
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+        fix_user_conn(thd, MAX_GLOBAL);
+#else
         decrease_user_connections(uc);
-        uc->user_stats.errors_access_denied.inc();
+#endif
+        uc->user_stats.connections_denied_max_global.inc();
       }
       Host_errors errors;
       errors.m_connect= 1;

diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc
@@ -93,7 +93,7 @@ void fix_user_conn(THD *thd, enum conn_denied_reason reason)
     case MAX_GLOBAL:
       us->connections_denied_max_global.inc();
       break;
-    case ADMIN_PORT:
+    case OTHER_ACCESS:
       break;
   }
 

diff --git a/sql/sql_connect.h b/sql/sql_connect.h
@@ -47,7 +47,7 @@ int check_user(THD *thd, enum enum_server_command command,
 bool login_connection(THD *thd);
 void prepare_new_connection_state(THD* thd);
 void end_connection(THD *thd);
-enum conn_denied_reason { MAX_USER, MAX_GLOBAL, ADMIN_PORT};
+enum conn_denied_reason { MAX_USER, MAX_GLOBAL, OTHER_ACCESS };
 void fix_user_conn(THD *thd, enum conn_denied_reason reason);
 int get_or_create_user_conn(THD *thd, const char *user,
                             const char *host, const USER_RESOURCES *mqh);