Below is the list of changes that have just been committed into a local
5.0 repository of antony. When antony does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
ChangeSet@stripped, 2007-07-09 21:54:16-07:00, antony@stripped +4 -0
Bug#25679
"Federated Denial of Service"
Federated storage engine used to attempt to open connections within
the ::create() and ::open() methods which are invoked while LOCK_open
mutex is being held by mysqld. As a result, no other connection can
open tables during the network processing. Long DNS lookup times
would stall mysqld's operation and a rogue connection string which
connects to a remote server which simply stalls during handshake can
stall mysqld for much longer periods of time.
This patch moves the connection-opening much later to when LOCK_open
mutex is not being held.
mysql-test/r/federated.result@stripped, 2007-07-09 21:54:01-07:00, antony@stripped +5 -4
change of test/results due to bug25679
mysql-test/t/federated.test@stripped, 2007-07-09 21:54:02-07:00, antony@stripped +6 -3
change of test/results due to bug25679
sql/ha_federated.cc@stripped, 2007-07-09 21:54:03-07:00, antony@stripped +136 -156
bug25679
remove function check_foreign_data_source()
ha_federated::open() no longer opens the federated connection.
ha_federated::create() no longer opens and tests connection.
ha_federated::real_query() opens and tests presence of table.
sql/ha_federated.h@stripped, 2007-07-09 21:54:03-07:00, antony@stripped +1 -0
bug25679
new method real_query()
diff -Nrup a/mysql-test/r/federated.result b/mysql-test/r/federated.result
--- a/mysql-test/r/federated.result 2007-06-28 13:36:17 -07:00
+++ b/mysql-test/r/federated.result 2007-07-09 21:54:01 -07:00
@@ -40,17 +40,18 @@ CREATE TABLE federated.t1 (
)
ENGINE="FEDERATED" DEFAULT CHARSET=latin1
CONNECTION='mysql://root@stripped:SLAVE_PORT/federated/t3';
-ERROR HY000: Can't create federated table. Foreign data src error: error: 1146 'Table 'federated.t3' doesn't exist'
+SELECT * FROM federated.t1;
+ERROR HY000: The foreign data source you are trying to reference does not exist. Data source error: error: 1146 'Table 'federated.t3' doesn't exist'
+DROP TABLE federated.t1;
CREATE TABLE federated.t1 (
`id` int(20) NOT NULL,
`name` varchar(32) NOT NULL default ''
)
ENGINE="FEDERATED" DEFAULT CHARSET=latin1
CONNECTION='mysql://user:pass@stripped:SLAVE_PORT/federated/t1';
+SELECT * FROM federated.t1;
ERROR HY000: Unable to connect to foreign data source: database: 'federated' username: 'user' hostname: '127.0.0.1'
-DROP TABLE IF EXISTS federated.t1;
-Warnings:
-Note 1051 Unknown table 't1'
+DROP TABLE federated.t1;
CREATE TABLE federated.t1 (
`id` int(20) NOT NULL,
`name` varchar(32) NOT NULL default ''
diff -Nrup a/mysql-test/t/federated.test b/mysql-test/t/federated.test
--- a/mysql-test/t/federated.test 2007-06-28 13:36:17 -07:00
+++ b/mysql-test/t/federated.test 2007-07-09 21:54:02 -07:00
@@ -30,25 +30,28 @@ CREATE TABLE federated.t1 (
# test non-existant table
--replace_result $SLAVE_MYPORT SLAVE_PORT
---error 1434
eval CREATE TABLE federated.t1 (
`id` int(20) NOT NULL,
`name` varchar(32) NOT NULL default ''
)
ENGINE="FEDERATED" DEFAULT CHARSET=latin1
CONNECTION='mysql://root@stripped:$SLAVE_MYPORT/federated/t3';
+--error 1431
+SELECT * FROM federated.t1;
+DROP TABLE federated.t1;
# test bad user/password
--replace_result $SLAVE_MYPORT SLAVE_PORT
---error 1429
eval CREATE TABLE federated.t1 (
`id` int(20) NOT NULL,
`name` varchar(32) NOT NULL default ''
)
ENGINE="FEDERATED" DEFAULT CHARSET=latin1
CONNECTION='mysql://user:pass@stripped:$SLAVE_MYPORT/federated/t1';
+--error 1429
+SELECT * FROM federated.t1;
+DROP TABLE federated.t1;
-DROP TABLE IF EXISTS federated.t1;
# # correct connection, same named tables
--replace_result $SLAVE_MYPORT SLAVE_PORT
eval CREATE TABLE federated.t1 (
diff -Nrup a/sql/ha_federated.cc b/sql/ha_federated.cc
--- a/sql/ha_federated.cc 2007-06-29 15:14:00 -07:00
+++ b/sql/ha_federated.cc 2007-07-09 21:54:03 -07:00
@@ -497,105 +497,6 @@ err:
}
-/*
- Check (in create) whether the tables exists, and that it can be connected to
-
- SYNOPSIS
- check_foreign_data_source()
- share pointer to FEDERATED share
- table_create_flag tells us that ::create is the caller,
- therefore, return CANT_CREATE_FEDERATED_TABLE
-
- DESCRIPTION
- This method first checks that the connection information that parse url
- has populated into the share will be sufficient to connect to the foreign
- table, and if so, does the foreign table exist.
-*/
-
-static int check_foreign_data_source(FEDERATED_SHARE *share,
- bool table_create_flag)
-{
- char query_buffer[FEDERATED_QUERY_BUFFER_SIZE];
- char error_buffer[FEDERATED_QUERY_BUFFER_SIZE];
- uint error_code;
- String query(query_buffer, sizeof(query_buffer), &my_charset_bin);
- MYSQL *mysql;
- DBUG_ENTER("ha_federated::check_foreign_data_source");
-
- /* Zero the length, otherwise the string will have misc chars */
- query.length(0);
-
- /* error out if we can't alloc memory for mysql_init(NULL) (per Georg) */
- if (!(mysql= mysql_init(NULL)))
- DBUG_RETURN(HA_ERR_OUT_OF_MEM);
- /* check if we can connect */
- if (!mysql_real_connect(mysql,
- share->hostname,
- share->username,
- share->password,
- share->database,
- share->port,
- share->socket, 0))
- {
- /*
- we want the correct error message, but it to return
- ER_CANT_CREATE_FEDERATED_TABLE if called by ::create
- */
- error_code= (table_create_flag ?
- ER_CANT_CREATE_FEDERATED_TABLE :
- ER_CONNECT_TO_FOREIGN_DATA_SOURCE);
-
- my_sprintf(error_buffer,
- (error_buffer,
- "database: '%s' username: '%s' hostname: '%s'",
- share->database, share->username, share->hostname));
-
- my_error(ER_CONNECT_TO_FOREIGN_DATA_SOURCE, MYF(0), error_buffer);
- goto error;
- }
- else
- {
- /*
- Since we do not support transactions at this version, we can let the
- client API silently reconnect. For future versions, we will need more
- logic to deal with transactions
- */
- mysql->reconnect= 1;
- /*
- Note: I am not using INORMATION_SCHEMA because this needs to work with
- versions prior to 5.0
-
- if we can connect, then make sure the table exists
-
- the query will be: SELECT * FROM `tablename` WHERE 1=0
- */
- query.append(FEDERATED_SELECT);
- query.append(FEDERATED_STAR);
- query.append(FEDERATED_FROM);
- append_ident(&query, share->table_name, share->table_name_length,
- ident_quote_char);
- query.append(FEDERATED_WHERE);
- query.append(FEDERATED_FALSE);
-
- if (mysql_real_query(mysql, query.ptr(), query.length()))
- {
- error_code= table_create_flag ?
- ER_CANT_CREATE_FEDERATED_TABLE : ER_FOREIGN_DATA_SOURCE_DOESNT_EXIST;
- my_sprintf(error_buffer, (error_buffer, "error: %d '%s'",
- mysql_errno(mysql), mysql_error(mysql)));
-
- my_error(error_code, MYF(0), error_buffer);
- goto error;
- }
- }
- error_code=0;
-
-error:
- mysql_close(mysql);
- DBUG_RETURN(error_code);
-}
-
-
static int parse_url_error(FEDERATED_SHARE *share, TABLE *table, int error_num)
{
char buf[FEDERATED_QUERY_BUFFER_SIZE];
@@ -1478,36 +1379,7 @@ int ha_federated::open(const char *name,
DBUG_RETURN(1);
thr_lock_data_init(&share->lock, &lock, NULL);
- /* Connect to foreign database mysql_real_connect() */
- mysql= mysql_init(0);
-
- /*
- BUG# 17044 Federated Storage Engine is not UTF8 clean
- Add set names to whatever charset the table is at open
- of table
- */
- /* this sets the csname like 'set names utf8' */
- mysql_options(mysql,MYSQL_SET_CHARSET_NAME,
- this->table->s->table_charset->csname);
-
- if (!mysql || !mysql_real_connect(mysql,
- share->hostname,
- share->username,
- share->password,
- share->database,
- share->port,
- share->socket, 0))
- {
- free_share(share);
- DBUG_RETURN(stash_remote_error());
- }
- /*
- Since we do not support transactions at this version, we can let the client
- API silently reconnect. For future versions, we will need more logic to
- deal with transactions
- */
-
- mysql->reconnect= 1;
+ DBUG_ASSERT(mysql == NULL);
ref_length= (table->s->primary_key != MAX_KEY ?
table->key_info[table->s->primary_key].key_length :
@@ -1543,8 +1415,8 @@ int ha_federated::close(void)
stored_result= 0;
}
/* Disconnect from mysql */
- if (mysql) // QQ is this really needed
- mysql_close(mysql);
+ mysql_close(mysql);
+ mysql= NULL;
retval= free_share(share);
DBUG_RETURN(retval);
@@ -1767,6 +1639,14 @@ int ha_federated::write_row(byte *buf)
if (use_bulk_insert)
{
/*
+ Make sure we have an open connection so that we know the
+ maximum packet size. Calling ::real_query() with an empty
+ query opens the connection if it is not already open.
+ */
+ if (real_query(NULL,0))
+ goto err;
+
+ /*
Send the current bulk insert out if appending the current row would
cause the statement to overflow the packet size, otherwise set
auto_increment_update_required to FALSE as no query was executed.
@@ -1774,7 +1654,7 @@ int ha_federated::write_row(byte *buf)
if (bulk_insert.length + values_string.length() + bulk_padding >
mysql->net.max_packet_size && bulk_insert.length)
{
- error= mysql_real_query(mysql, bulk_insert.str, bulk_insert.length);
+ error= real_query(bulk_insert.str, bulk_insert.length);
bulk_insert.length= 0;
}
else
@@ -1798,10 +1678,10 @@ int ha_federated::write_row(byte *buf)
}
else
{
- error= mysql_real_query(mysql, values_string.ptr(),
- values_string.length());
+ error= real_query(values_string.ptr(), values_string.length());
}
-
+
+err:
if (error)
{
DBUG_RETURN(stash_remote_error());
@@ -1870,7 +1750,7 @@ int ha_federated::end_bulk_insert()
if (bulk_insert.str && bulk_insert.length)
{
- if (mysql_real_query(mysql, bulk_insert.str, bulk_insert.length))
+ if (real_query(bulk_insert.str, bulk_insert.length))
error= stash_remote_error();
else
if (table->next_number_field)
@@ -1915,7 +1795,7 @@ int ha_federated::optimize(THD* thd, HA_
append_ident(&query, share->table_name, share->table_name_length,
ident_quote_char);
- if (mysql_real_query(mysql, query.ptr(), query.length()))
+ if (real_query(query.ptr(), query.length()))
{
DBUG_RETURN(stash_remote_error());
}
@@ -1943,7 +1823,7 @@ int ha_federated::repair(THD* thd, HA_CH
if (check_opt->sql_flags & TT_USEFRM)
query.append(FEDERATED_USE_FRM);
- if (mysql_real_query(mysql, query.ptr(), query.length()))
+ if (real_query(query.ptr(), query.length()))
{
DBUG_RETURN(stash_remote_error());
}
@@ -2081,7 +1961,7 @@ int ha_federated::update_row(const byte
if (!has_a_primary_key)
update_string.append(FEDERATED_LIMIT1);
- if (mysql_real_query(mysql, update_string.ptr(), update_string.length()))
+ if (real_query(update_string.ptr(), update_string.length()))
{
DBUG_RETURN(stash_remote_error());
}
@@ -2146,7 +2026,7 @@ int ha_federated::delete_row(const byte
delete_string.append(FEDERATED_LIMIT1);
DBUG_PRINT("info",
("Delete sql: %s", delete_string.c_ptr_quick()));
- if (mysql_real_query(mysql, delete_string.ptr(), delete_string.length()))
+ if (real_query(delete_string.ptr(), delete_string.length()))
{
DBUG_RETURN(stash_remote_error());
}
@@ -2255,7 +2135,7 @@ int ha_federated::index_read_idx_with_re
NULL, 0);
sql_query.append(index_string);
- if (mysql_real_query(mysql, sql_query.ptr(), sql_query.length()))
+ if (real_query(sql_query.ptr(), sql_query.length()))
{
my_sprintf(error_buffer, (error_buffer, "error: %d '%s'",
mysql_errno(mysql), mysql_error(mysql)));
@@ -2321,7 +2201,7 @@ int ha_federated::read_range_first(const
mysql_free_result(stored_result);
stored_result= 0;
}
- if (mysql_real_query(mysql, sql_query.ptr(), sql_query.length()))
+ if (real_query(sql_query.ptr(), sql_query.length()))
{
retval= ER_QUERY_ON_FOREIGN_DATA_SOURCE;
goto error;
@@ -2421,9 +2301,7 @@ int ha_federated::rnd_init(bool scan)
stored_result= 0;
}
- if (mysql_real_query(mysql,
- share->select_query,
- strlen(share->select_query)))
+ if (real_query(share->select_query, strlen(share->select_query)))
goto error;
stored_result= mysql_store_result(mysql);
@@ -2640,8 +2518,7 @@ int ha_federated::info(uint flag)
append_ident(&status_query_string, share->table_name,
share->table_name_length, value_quote_char);
- if (mysql_real_query(mysql, status_query_string.ptr(),
- status_query_string.length()))
+ if (real_query(status_query_string.ptr(), status_query_string.length()))
goto error;
status_query_string.length(0);
@@ -2694,12 +2571,18 @@ int ha_federated::info(uint flag)
DBUG_RETURN(0);
error:
- if (result)
- mysql_free_result(result);
-
- my_sprintf(error_buffer, (error_buffer, ": %d : %s",
- mysql_errno(mysql), mysql_error(mysql)));
- my_error(error_code, MYF(0), error_buffer);
+ mysql_free_result(result);
+ if (mysql)
+ {
+ my_sprintf(error_buffer, (error_buffer, ": %d : %s",
+ mysql_errno(mysql), mysql_error(mysql)));
+ my_error(error_code, MYF(0), error_buffer);
+ }
+ else
+ {
+ error_code= remote_error_number;
+ my_error(error_code, MYF(0), ER(error_code));
+ }
DBUG_RETURN(error_code);
}
@@ -2777,7 +2660,7 @@ int ha_federated::delete_all_rows()
/*
TRUNCATE won't return anything in mysql_affected_rows
*/
- if (mysql_real_query(mysql, query.ptr(), query.length()))
+ if (real_query(query.ptr(), query.length()))
{
DBUG_RETURN(stash_remote_error());
}
@@ -2866,8 +2749,7 @@ int ha_federated::create(const char *nam
FEDERATED_SHARE tmp_share; // Only a temporary share, to test the url
DBUG_ENTER("ha_federated::create");
- if (!(retval= parse_url(&tmp_share, table_arg, 1)))
- retval= check_foreign_data_source(&tmp_share, 1);
+ retval= parse_url(&tmp_share, table_arg, 1);
my_free((gptr) tmp_share.scheme, MYF(MY_ALLOW_ZERO_PTR));
DBUG_RETURN(retval);
@@ -2875,9 +2757,107 @@ int ha_federated::create(const char *nam
}
+int ha_federated::real_query(const char *query, uint length)
+{
+ DBUG_ENTER("ha_federated::real_query");
+ if (!mysql)
+ {
+ char buffer[FEDERATED_QUERY_BUFFER_SIZE];
+ String sql_query(buffer, sizeof(buffer), &my_charset_bin);
+
+ /*
+ Bug#25679
+ Ensure that we do not hold the LOCK_open mutex while attempting
+ to establish Federated connection to guard against a trivial
+ Denial of Service scenerio.
+ */
+ safe_mutex_assert_not_owner(&LOCK_open);
+
+ if (!(mysql= mysql_init(NULL)))
+ {
+ remote_error_number= HA_ERR_OUT_OF_MEM;
+ DBUG_RETURN(-1);
+ }
+
+ /*
+ BUG# 17044 Federated Storage Engine is not UTF8 clean
+ Add set names to whatever charset the table is at open
+ of table
+ */
+ /* this sets the csname like 'set names utf8' */
+ mysql_options(mysql,MYSQL_SET_CHARSET_NAME,
+ this->table->s->table_charset->csname);
+
+ sql_query.length(0);
+
+ if (!mysql_real_connect(mysql,
+ share->hostname,
+ share->username,
+ share->password,
+ share->database,
+ share->port,
+ share->socket, 0))
+ {
+ mysql_close(mysql);
+ mysql= NULL;
+ sql_query.append("database: '");
+ sql_query.append(share->database);
+ sql_query.append("' username: '");
+ sql_query.append(share->username);
+ sql_query.append("' hostname: '");
+ sql_query.append(share->hostname);
+ sql_query.append("'");
+ my_error(ER_CONNECT_TO_FOREIGN_DATA_SOURCE, MYF(0), sql_query.ptr());
+ remote_error_number= -1;
+ DBUG_RETURN(-1);
+ }
+
+ /*
+ We have established a connection, lets try a simple dummy query just
+ to check that the table and expected columns are present.
+ */
+ sql_query.append(share->select_query);
+ sql_query.append(FEDERATED_WHERE);
+ sql_query.append(FEDERATED_FALSE);
+ if (mysql_real_query(mysql, sql_query.ptr(), sql_query.length()))
+ {
+ sql_query.length(0);
+ sql_query.append("error: ");
+ sql_query.qs_append(mysql_errno(mysql));
+ sql_query.append(" '");
+ sql_query.append(mysql_error(mysql));
+ sql_query.append("'");
+ mysql_close(mysql);
+ mysql= NULL;
+ my_error(ER_FOREIGN_DATA_SOURCE_DOESNT_EXIST, MYF(0), sql_query.ptr());
+ remote_error_number= -1;
+ DBUG_RETURN(-1);
+ }
+
+ /* Just throw away the result, no rows anyways but need to keep in sync */
+ mysql_free_result(mysql_store_result(mysql));
+
+ /*
+ Since we do not support transactions at this version, we can let the client
+ API silently reconnect. For future versions, we will need more logic to
+ deal with transactions
+ */
+
+ mysql->reconnect= 1;
+ }
+
+ if (!query || !length)
+ DBUG_RETURN(0);
+
+ DBUG_RETURN(mysql_real_query(mysql, query, length));
+}
+
+
int ha_federated::stash_remote_error()
{
DBUG_ENTER("ha_federated::stash_remote_error()");
+ if (!mysql)
+ DBUG_RETURN(remote_error_number);
remote_error_number= mysql_errno(mysql);
strmake(remote_error_buf, mysql_error(mysql), sizeof(remote_error_buf)-1);
if (remote_error_number == ER_DUP_ENTRY ||
diff -Nrup a/sql/ha_federated.h b/sql/ha_federated.h
--- a/sql/ha_federated.h 2007-06-28 16:02:58 -07:00
+++ b/sql/ha_federated.h 2007-07-09 21:54:03 -07:00
@@ -182,6 +182,7 @@ private:
uint key_len,
ha_rkey_function find_flag,
MYSQL_RES **result);
+ int real_query(const char *query, uint length);
public:
ha_federated(TABLE *table_arg);
~ha_federated()
