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-25 11:18:18-07:00, acurtis@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 client sessions
can open tables while Federated is attempting to open a connection.
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 a much longer period of
time.
This patch moves the opening of the connection much later, when the
federated actually issues queries, by which time the LOCK_open mutex is
no longer being held.
mysql-test/r/federated.result@stripped, 2007-07-25 11:18:07-07:00, acurtis@stripped +6 -5
change of test/results due to patch for bug25679
mysql-test/t/federated.test@stripped, 2007-07-25 11:18:07-07:00, acurtis@stripped +6 -3
change of test/results due to patch for bug25679
sql/ha_federated.cc@stripped, 2007-07-25 11:18:07-07:00, acurtis@stripped +144 -156
bug25679
remove function check_foreign_fata_source()
ha_federated::open() no longer opens the federated connection.
ha_federated::create() no longer opens and tests connection.
ha_federated::real_connect() opens connection and tests presence of table.
ha_federated::real_query() sends query, calling real_connect() if neccessary.
sql/ha_federated.h@stripped, 2007-07-25 11:18:08-07:00, acurtis@stripped +2 -0
bug25679
new methods real_query() and real_connect()
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-25 11:18:07 -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';
-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'
+SELECT * FROM federated.t1;
+ERROR HY000: Unable to connect to foreign data source: Access denied for user 'user'@'localhost' (using password: YES)
+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-25 11:18:07 -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-07-23 23:35:40 -07:00
+++ b/sql/ha_federated.cc 2007-07-25 11:18:07 -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);
@@ -1774,7 +1646,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 +1670,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());
@@ -1837,6 +1709,8 @@ void ha_federated::start_bulk_insert(ha_
DBUG_ENTER("ha_federated::start_bulk_insert");
dynstr_free(&bulk_insert);
+
+ DBUG_PRINT("info",("rows = %lld", (long long) rows));
/**
We don't bother with bulk-insert semantics when the estimated rows == 1
@@ -1847,6 +1721,13 @@ void ha_federated::start_bulk_insert(ha_
if (rows == 1)
DBUG_VOID_RETURN;
+ /*
+ Make sure we have an open connection so that we know the
+ maximum packet size.
+ */
+ if (!mysql && real_connect())
+ DBUG_VOID_RETURN;
+
page_size= (uint) my_getpagesize();
if (init_dynamic_string(&bulk_insert, NULL, page_size, page_size))
@@ -1875,7 +1756,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)
@@ -1921,7 +1802,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());
}
@@ -1949,7 +1830,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());
}
@@ -2087,7 +1968,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());
}
@@ -2152,7 +2033,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());
}
@@ -2261,7 +2142,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)));
@@ -2327,7 +2208,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;
@@ -2427,9 +2308,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);
@@ -2646,8 +2525,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);
@@ -2702,12 +2580,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);
}
@@ -2785,7 +2669,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());
}
@@ -2874,8 +2758,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);
@@ -2883,9 +2766,114 @@ int ha_federated::create(const char *nam
}
+int ha_federated::real_connect()
+{
+ char buffer[FEDERATED_QUERY_BUFFER_SIZE];
+ String sql_query(buffer, sizeof(buffer), &my_charset_bin);
+ DBUG_ENTER("ha_federated::real_connect");
+
+ /*
+ 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);
+
+ DBUG_ASSERT(mysql == NULL);
+
+ 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))
+ {
+ stash_remote_error();
+ mysql_close(mysql);
+ mysql= NULL;
+ my_error(ER_CONNECT_TO_FOREIGN_DATA_SOURCE, MYF(0), remote_error_buf);
+ 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;
+ DBUG_RETURN(0);
+}
+
+
+int ha_federated::real_query(const char *query, uint length)
+{
+ int rc;
+ DBUG_ENTER("ha_federated::real_query");
+
+ if (!mysql && (rc= real_connect()))
+ goto end;
+
+ if (!query || !length)
+ goto end;
+
+ rc= mysql_real_query(mysql, query, length);
+
+end:
+ DBUG_RETURN(rc);
+}
+
+
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-25 11:18:08 -07:00
@@ -182,6 +182,8 @@ private:
uint key_len,
ha_rkey_function find_flag,
MYSQL_RES **result);
+ int real_query(const char *query, uint length);
+ int real_connect();
public:
ha_federated(TABLE *table_arg);
~ha_federated()
| Thread |
|---|
| • bk commit into 5.0 tree (acurtis:1.2482) BUG#25679 | antony | 25 Jul |
