Skip to content

Commit

Permalink
MYSQL-266: CVE-2012-5627: Reuse of salt during change_user
Browse files Browse the repository at this point in the history
This is a port of Percona fix for the vulnerability.
  • Loading branch information
inaam-rana committed Oct 29, 2013
1 parent 9d4e93f commit 4f5727e
Show file tree
Hide file tree
Showing 11 changed files with 269 additions and 83 deletions.
5 changes: 4 additions & 1 deletion client/mysqltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4087,7 +4087,10 @@ void do_change_user(struct st_command *command)
cur_con->name, ds_user.str, ds_passwd.str, ds_db.str));

if (mysql_change_user(mysql, ds_user.str, ds_passwd.str, ds_db.str))
die("change user failed: %s", mysql_error(mysql));
handle_error(command, mysql_errno(mysql), mysql_error(mysql),
mysql_sqlstate(mysql), &ds_res);
else
handle_no_error(command);

dynstr_free(&ds_user);
dynstr_free(&ds_passwd);
Expand Down
5 changes: 5 additions & 0 deletions mysql-test/r/change_user_notembedded.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ERROR 28000: Access denied for user 'foo'@'localhost' (using password: YES)
ERROR 28000: Access denied for user 'foo'@'localhost' (using password: NO)
ERROR 28000: Access denied for user 'foo'@'localhost' (using password: YES)
ERROR 08S01: Unknown command
ERROR 08S01: Unknown command
20 changes: 20 additions & 0 deletions mysql-test/r/failed_auth_3909.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
optimize table mysql.user;
Table Op Msg_type Msg_text
mysql.user optimize status OK
insert mysql.user (user,plugin) values ('foo','bar'),('bar','bar'),('baz','bar');
Warnings:
Warning 1364 Field 'ssl_cipher' doesn't have a default value
Warning 1364 Field 'x509_issuer' doesn't have a default value
Warning 1364 Field 'x509_subject' doesn't have a default value
flush privileges;
connect(localhost,u1,,test,MASTER_PORT,MASTER_SOCKET);
ERROR HY000: Plugin 'bar' is not loaded
connect(localhost,u2,,test,MASTER_PORT,MASTER_SOCKET);
ERROR 28000: Access denied for user 'u2'@'localhost' (using password: NO)
connect(localhost,u2,password,test,MASTER_PORT,MASTER_SOCKET);
ERROR 28000: Access denied for user 'u2'@'localhost' (using password: YES)
ERROR HY000: Plugin 'bar' is not loaded
ERROR 28000: Access denied for user 'u2'@'localhost' (using password: NO)
ERROR 28000: Access denied for user 'u2'@'localhost' (using password: YES)
delete from mysql.user where plugin = 'bar';
flush privileges;
6 changes: 3 additions & 3 deletions mysql-test/r/mysqltest.result
Original file line number Diff line number Diff line change
Expand Up @@ -922,9 +922,9 @@ a int(11) YES NULL
b varchar(255) YES NULL
c datetime YES NULL
drop table t1;
mysqltest: At line 1: change user failed: Unknown database 'inexistent'
mysqltest: At line 1: change user failed: Access denied for user 'inexistent'@'localhost' (using password: NO)
mysqltest: At line 1: change user failed: Access denied for user 'root'@'localhost' (using password: YES)
mysqltest: At line 1: query 'change_user root,,inexistent' failed: 1049: Unknown database 'inexistent'
mysqltest: At line 1: query 'change_user inexistent,,test' failed: 1045: Access denied for user 'inexistent'@'localhost' (using password: NO)
mysqltest: At line 1: query 'change_user root,inexistent,test' failed: 1045: Access denied for user 'root'@'localhost' (using password: YES)
REPLACED_FILE1.txt
file1.txt
file2.txt
Expand Down
24 changes: 24 additions & 0 deletions mysql-test/t/change_user_notembedded.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
source include/not_embedded.inc;

#
# MDEV-3915 COM_CHANGE_USER allows fast password brute-forcing
#
# only three failed change_user per connection.
# successful change_user do NOT reset the counter
#
connect (test,localhost,root,,);
connection test;
--error 1045
change_user foo,bar;
--error 1045
change_user foo;
change_user;
--error 1045
change_user foo,bar;
--error 1047
change_user foo,bar;
--error 1047
change_user;
disconnect test;
connection default;

37 changes: 37 additions & 0 deletions mysql-test/t/failed_auth_3909.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
source include/not_embedded.inc;

#
# MDEV-3909 remote user enumeration
#
# verify that for some failed login attemps (with wrong user names)
# the server requests a plugin
#
optimize table mysql.user;
insert mysql.user (user,plugin) values ('foo','bar'),('bar','bar'),('baz','bar');
flush privileges;

--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
--error ER_PLUGIN_IS_NOT_LOADED
connect (fail,localhost,u1);

--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
--error ER_ACCESS_DENIED_ERROR
connect (fail,localhost,u2);

--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
--error ER_ACCESS_DENIED_ERROR
connect (fail,localhost,u2,password);

--error ER_PLUGIN_IS_NOT_LOADED
change_user u1;

--error ER_ACCESS_DENIED_ERROR
change_user u2;

--error ER_ACCESS_DENIED_ERROR
change_user u2,password;

delete from mysql.user where plugin = 'bar';
flush privileges;


54 changes: 48 additions & 6 deletions sql/sql_acl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8024,6 +8024,7 @@ struct MPVIO_EXT :public MYSQL_PLUGIN_VIO
} cached_server_packet;
int packets_read, packets_written; ///< counters for send/received packets
uint connect_errors; ///< if there were connect errors for this host
bool make_it_fail;
/** when plugin returns a failure this tells us what really happened */
enum { SUCCESS, FAILURE, RESTART } status;

Expand Down Expand Up @@ -8371,14 +8372,14 @@ static bool send_plugin_request_packet(MPVIO_EXT *mpvio,
/**
Finds acl entry in user database for authentication purposes.
Finds a user and copies it into mpvio. Reports an authentication
failure if a user is not found.
Finds a user and copies it into mpvio. Creates a fake user
if no matching user account is found.
@note find_acl_user is not the same, because it doesn't take into
account the case when user is not empty, but acl_user->user is empty
@retval 0 found
@retval 1 not found
@retval 1 error
*/
static bool find_mpvio_user(MPVIO_EXT *mpvio)
{
Expand Down Expand Up @@ -8409,8 +8410,32 @@ static bool find_mpvio_user(MPVIO_EXT *mpvio)

if (!mpvio->acl_user)
{
login_failed_error(mpvio, mpvio->auth_info.password_used);
DBUG_RETURN (1);
/*
A matching user was not found. Fake it. Take any user, make the
authentication fail later.
This way we get a realistically looking failure, with occasional
"change auth plugin" requests even for nonexistent users. The ratio
of "change auth plugin" request will be the same for real and
nonexistent users.
Note, that we cannot pick any user at random, it must always be
the same user account for the incoming sctx->user name.
*/
ulong nr1=1, nr2=4;
CHARSET_INFO *cs= &my_charset_latin1;
cs->coll->hash_sort(cs, (uchar*) mpvio->auth_info.user_name,
mpvio->auth_info.user_name_length, &nr1, &nr2);

mysql_mutex_lock(&acl_cache->lock);
uint i= nr1 % acl_users.elements;
ACL_USER *acl_user_tmp= dynamic_element(&acl_users, i, ACL_USER*);
mpvio->acl_user= acl_user_tmp->copy(mpvio->mem_root);
make_lex_string_root(mpvio->mem_root,
&mpvio->acl_user_plugin,
acl_user_tmp->plugin.str,
acl_user_tmp->plugin.length, 0);
mysql_mutex_unlock(&acl_cache->lock);

mpvio->make_it_fail= true;
}

/* user account requires non-default plugin and the client is too old */
Expand Down Expand Up @@ -8481,6 +8506,9 @@ static bool parse_com_change_user_packet(MPVIO_EXT *mpvio, uint packet_length)
uint passwd_len= (mpvio->client_capabilities & CLIENT_SECURE_CONNECTION ?
(uchar) (*passwd++) : strlen(passwd));

if (passwd_len)
mpvio->auth_info.password_used= PASSWORD_USED_YES;

db+= passwd_len + 1;
/*
Database name is always NUL-terminated, so in case of empty database
Expand Down Expand Up @@ -9206,6 +9234,10 @@ static int server_mpvio_read_packet(MYSQL_PLUGIN_VIO *param, uchar **buf)
*buf= (uchar*) mpvio->cached_client_reply.pkt;
mpvio->cached_client_reply.pkt= 0;
mpvio->packets_read++;

if (mpvio->make_it_fail)
goto err;

DBUG_RETURN ((int) mpvio->cached_client_reply.pkt_len);
}

Expand Down Expand Up @@ -9248,13 +9280,22 @@ static int server_mpvio_read_packet(MYSQL_PLUGIN_VIO *param, uchar **buf)
else
*buf= mpvio->net->read_pos;

if (mpvio->make_it_fail)
goto err;

DBUG_RETURN((int)pkt_len);

err:
if (mpvio->status == MPVIO_EXT::FAILURE)
{
inc_host_errors(mpvio->ip);
my_error(ER_HANDSHAKE_ERROR, MYF(0));
if (!current_thd->is_error())
{
if (mpvio->make_it_fail)
login_failed_error(mpvio, mpvio->auth_info.password_used);
else
my_error(ER_HANDSHAKE_ERROR, MYF(0));
}
}
DBUG_RETURN(-1);
}
Expand Down Expand Up @@ -9446,6 +9487,7 @@ server_mpvio_initialize(THD *thd, MPVIO_EXT *mpvio, uint connect_errors,
mpvio->auth_info.user_name_length= 0;
mpvio->connect_errors= connect_errors;
mpvio->status= MPVIO_EXT::FAILURE;
mpvio->make_it_fail= false;

mpvio->client_capabilities= thd->client_capabilities;
mpvio->mem_root= thd->mem_root;
Expand Down
1 change: 1 addition & 0 deletions sql/sql_class.cc
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,7 @@ THD::THD()
first_successful_insert_id_in_prev_stmt_for_binlog(0),
first_successful_insert_id_in_cur_stmt(0),
stmt_depends_on_first_successful_insert_id_in_prev_stmt(FALSE),
failed_com_change_user(0),
examined_row_count(0),
warning_info(&main_warning_info),
stmt_da(&main_da),
Expand Down
1 change: 1 addition & 0 deletions sql/sql_class.h
Original file line number Diff line number Diff line change
Expand Up @@ -1982,6 +1982,7 @@ class THD :public Statement,
}

ha_rows cuted_fields;
uint8 failed_com_change_user;

/*
number of rows we actually sent to the client, including "synthetic"
Expand Down
19 changes: 18 additions & 1 deletion sql/sql_parse.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,22 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
CHARSET_INFO *save_character_set_results=
thd->variables.character_set_results;

rc= acl_authenticate(thd, 0, packet_length);
/* Ensure we don't free security_ctx->user in case we have to revert */
thd->security_ctx->user= 0;
thd->set_user_connect(0);

/*
to limit COM_CHANGE_USER ability to brute-force passwords,
we only allow three unsuccessful COM_CHANGE_USER per connection.
*/
if (thd->failed_com_change_user >= 3)
{
my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0));
rc= 1;
}
else
rc= acl_authenticate(thd, 0, packet_length);

MYSQL_AUDIT_NOTIFY_CONNECTION_CHANGE_USER(thd);
if (rc)
{
Expand All @@ -1083,6 +1098,8 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
thd->variables.collation_connection= save_collation_connection;
thd->variables.character_set_results= save_character_set_results;
thd->update_charset();
thd->failed_com_change_user++;
my_sleep(1000000);
}
else
{
Expand Down
Loading

0 comments on commit 4f5727e

Please sign in to comment.