Skip to content

Commit 913af2c

Browse files
committed
Bug #34094706 : require_secure_transport causes abrupt server exit when checking
thd->active_vio Description: ------------ When require_secure_transport is enabled, the server checks the thd->active_vio pointer in acl_authenticate(). However, this check is not thread-safe as server can exit abruptly if the connection is killed before checking active_vio. Fix: ---- Provided mutex protection in the acl_authenticate() method to avoid this problem and added a nullptr check as well for thd->active_vio. Change-Id: I42c284dcaf3c988734010f9faf02f9c023dda90d
1 parent abd8f90 commit 913af2c

File tree

2 files changed

+41
-4
lines changed

2 files changed

+41
-4
lines changed

sql/auth/sql_authentication.cc

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include <mysql/plugin_validate_password.h> /* validate_password plugin */
3737
#include <mysql/service_my_plugin_log.h>
3838
#include "sys_vars.h"
39+
#include "debug_sync.h"
3940
#include <fstream> /* std::fstream */
4041
#include <string> /* std::string */
4142
#include <algorithm> /* for_each */
@@ -2195,6 +2196,14 @@ acl_authenticate(THD *thd, enum_server_command command)
21952196
compile_time_assert(MYSQL_USERNAME_LENGTH == USERNAME_LENGTH);
21962197
assert(command == COM_CONNECT || command == COM_CHANGE_USER);
21972198

2199+
mysql_mutex_lock(&thd->LOCK_thd_data);
2200+
2201+
DBUG_EXECUTE_IF("before_server_mpvio_initialize", {
2202+
DBUG_SET("+d,wait_until_thread_kill");
2203+
const char act[] = "now SIGNAL acl_auth_reached";
2204+
assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
2205+
});
2206+
21982207
server_mpvio_initialize(thd, &mpvio, &charset_adapter);
21992208
/*
22002209
Clear thd->db as it points to something, that will be freed when
@@ -2223,6 +2232,7 @@ acl_authenticate(THD *thd, enum_server_command command)
22232232
{
22242233
login_failed_error(&mpvio, mpvio.auth_info.password_used);
22252234
server_mpvio_update_thd(thd, &mpvio);
2235+
mysql_mutex_unlock(&thd->LOCK_thd_data);
22262236
DBUG_RETURN(1);
22272237
}
22282238

@@ -2265,11 +2275,19 @@ acl_authenticate(THD *thd, enum_server_command command)
22652275
}
22662276
}
22672277

2278+
DBUG_EXECUTE_IF("before_server_mpvio_update_thd", {
2279+
DBUG_SET("+d,wait_until_thread_kill");
2280+
const char act[] = "now SIGNAL acl_auth_reached";
2281+
assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
2282+
});
2283+
22682284
server_mpvio_update_thd(thd, &mpvio);
22692285
#ifdef HAVE_PSI_THREAD_INTERFACE
22702286
PSI_THREAD_CALL(set_connection_type)(thd->get_vio_type());
22712287
#endif /* HAVE_PSI_THREAD_INTERFACE */
22722288

2289+
mysql_mutex_unlock(&thd->LOCK_thd_data);
2290+
22732291
Security_context *sctx= thd->security_context();
22742292
const ACL_USER *acl_user= mpvio.acl_user;
22752293
bool proxy_check= check_proxy_users && !*mpvio.auth_info.authenticated_as;
@@ -2451,13 +2469,20 @@ acl_authenticate(THD *thd, enum_server_command command)
24512469
DBUG_RETURN(1);
24522470
}
24532471

2454-
if (opt_require_secure_transport &&
2455-
!is_secure_transport(thd->active_vio->type))
2456-
{
2472+
mysql_mutex_lock(&thd->LOCK_thd_data);
2473+
DBUG_EXECUTE_IF("before_secure_transport_check", {
2474+
DBUG_SET("+d,wait_until_thread_kill");
2475+
const char act[] = "now SIGNAL acl_auth_reached";
2476+
assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
2477+
});
2478+
2479+
if (opt_require_secure_transport && thd->active_vio != NULL &&
2480+
!is_secure_transport(thd->active_vio->type)) {
2481+
mysql_mutex_unlock(&thd->LOCK_thd_data);
24572482
my_error(ER_SECURE_TRANSPORT_REQUIRED, MYF(0));
24582483
DBUG_RETURN(1);
24592484
}
2460-
2485+
mysql_mutex_unlock(&thd->LOCK_thd_data);
24612486

24622487
/* checking password_time_expire for connecting user */
24632488
password_time_expired= check_password_lifetime(thd, mpvio.acl_user);
@@ -2542,6 +2567,11 @@ acl_authenticate(THD *thd, enum_server_command command)
25422567
#endif // !EMBEDDED_LIBRARY
25432568
}
25442569

2570+
DBUG_EXECUTE_IF("wait_until_thread_kill", {
2571+
DBUG_SET("-d,wait_until_thread_kill");
2572+
const char act[] = "now WAIT_FOR killed";
2573+
assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
2574+
});
25452575
/*
25462576
This is the default access rights for the current database. It's
25472577
set to 0 here because we don't have an active database yet (and we
@@ -2928,6 +2958,12 @@ static int sha256_password_authenticate(MYSQL_PLUGIN_VIO *vio,
29282958

29292959
DBUG_ENTER("sha256_password_authenticate");
29302960

2961+
DBUG_EXECUTE_IF("in_sha256_password_authenticate", {
2962+
DBUG_SET("+d,wait_until_thread_kill");
2963+
const char act[] = "now SIGNAL acl_auth_reached";
2964+
assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
2965+
});
2966+
29312967
generate_user_salt(scramble, SCRAMBLE_LENGTH + 1);
29322968

29332969
/*

sql/sql_parse.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1936,6 +1936,7 @@ bool dispatch_command(THD *thd, const COM_DATA *com_data,
19361936
thd->proc_info= 0;
19371937
thd->lex->sql_command= SQLCOM_END;
19381938

1939+
DEBUG_SYNC(thd, "processlist_wait");
19391940
/* Performance Schema Interface instrumentation, end */
19401941
MYSQL_END_STATEMENT(thd->m_statement_psi, thd->get_stmt_da());
19411942
thd->m_statement_psi= NULL;

0 commit comments

Comments
 (0)