Below is the list of changes that have just been committed into a local
5.1 repository of thek. When thek 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, 2008-01-10 19:29:56+01:00, thek@adventure.(none) +4 -0
Bug#27145 EXTRA_ACL troubles
** PREVIEW **
The flag EXTRA_ACL is used in conjugation with our access checks, yet it is
not clear what impact this flag has.
This is a code clean up which removes the use of EXTRA_ACL and also fixes
the privilege checks for:
CREATE TABLE .. LIKE .. => Requires SELECT privileges on the table
SHOW CREATE TABLE => Requires any privileges on any column combination
CHECKSUM TABLE => Requires SELECT on the table
SHOW COLUMNS => Requires any privileges on any column combination on the table.
SHOW INDEX => Requires any privileges on any column combination on the table.
sql/sql_acl.cc@stripped, 2008-01-10 19:29:52+01:00, thek@adventure.(none) +44 -27
* Converted function documentation to doxygen and clarified some behaviors.
* Changed value from uint to bool to better reflect its meaning.
* Removed pointless variable orig_want_access
sql/sql_acl.h@stripped, 2008-01-10 19:29:52+01:00, thek@adventure.(none) +1 -1
* Converted function parameter form uint to bool to better reflect its meaning.
sql/sql_parse.cc@stripped, 2008-01-10 19:29:52+01:00, thek@adventure.(none) +174 -77
* Rewrote function documentation in doxygen comments for: check_access,
check_table_acces, check_grant.
* Removed EXTRA_ACL flag as it doesn't hold any meaningful purpose anymore.
* Fixed privilege check for SHOW COLUMNS and SHOW INDEX
* Modified check_table_access to gain clarity in what EXTRA_ACL actually does.
* Modified check_access to gain clarity in what EXTRA_ACL actually does.
* Fixed privilege check for CREATE TABLE .. LIKE .. ; It now requires SELECT
privileges on the table.
* Fixed privilege check for SHOW CREATE TABLE ..; It now requires any privilege
on any column combination.
sql/sql_show.cc@stripped, 2008-01-10 19:29:53+01:00, thek@adventure.(none) +1 -1
* Removed pointless EXTRA_ACL flag.
diff -Nrup a/sql/sql_acl.cc b/sql/sql_acl.cc
--- a/sql/sql_acl.cc 2007-11-28 15:34:10 +01:00
+++ b/sql/sql_acl.cc 2008-01-10 19:29:52 +01:00
@@ -3814,40 +3814,50 @@ end:
DBUG_RETURN(return_val);
}
-/****************************************************************************
- Check table level grants
+/**
+ @brief Check table level grants
- SYNOPSIS
- bool check_grant()
- thd Thread handler
- want_access Bits of privileges user needs to have
- tables List of tables to check. The user should have 'want_access'
- to all tables in list.
- show_table <> 0 if we are in show table. In this case it's enough to have
- any privilege for the table
- number Check at most this number of tables.
- no_errors If 0 then we write an error. The error is sent directly to
- the client
-
- RETURN
- 0 ok
- 1 Error: User did not have the requested privileges
+ @param thd Thread handler
+ @param want_access Bits of privileges user needs to have
+ @param tables List of tables to check. The user should have 'want_access'
+ to all tables in list.
+ @param show_table TRUE if it's enough to have any privilege for the table
+ @param number Check at most this number of tables.
+ @param no_errors TRUE if no error should be sent directly to the client
+
+ If table->grant.want_privilege != 0 then the requested privileges where
+ in the set of COL_ACLS but access was not granted on the table level. As
+ a consequence an extra check of column privileges is required.
+
+ Specifically if this function returns FALSE the user has some kind of
+ privilege on a combination of columns in each table.
- NOTE
- This functions assumes that either number of tables to be inspected
+ This function is usually preceeded by check_access which establish the
+ User-, Db- and Host access rights.
+
+ @see check_access
+ @see check_table_access
+
+ @note This functions assumes that either number of tables to be inspected
by it is limited explicitly (i.e. is is not UINT_MAX) or table list
used and thd->lex->query_tables_own_last value correspond to each
other (the latter should be either 0 or point to next_global member
of one of elements of this table list).
-****************************************************************************/
+
+ @return Access status
+ @retval FALSE Access granted; But column privileges might need to be
+ checked.
+ @retval TRUE The user did not have the requested privileges on any of the
+ tables.
+
+*/
bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables,
- uint show_table, uint number, bool no_errors)
+ bool show_table, uint number, bool no_errors)
{
TABLE_LIST *table, *first_not_own_table= thd->lex->first_not_own_table();
Security_context *sctx= thd->security_ctx;
uint i;
- ulong orig_want_access= want_access;
DBUG_ENTER("check_grant");
DBUG_ASSERT(number > 0);
@@ -3865,7 +3875,10 @@ bool check_grant(THD *thd, ulong want_ac
table != first_not_own_table && i < number;
table= table->next_global, i++)
{
- /* Remove SHOW_VIEW_ACL, because it will be checked during making view */
+ /*
+ Save a copy of the privileges without the SHOW_VIEW_ACL attribute.
+ It will be checked during making view.
+ */
table->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL);
}
@@ -3878,7 +3891,6 @@ bool check_grant(THD *thd, ulong want_ac
sctx = test(table->security_ctx) ?
table->security_ctx : thd->security_ctx;
- want_access= orig_want_access;
want_access&= ~sctx->master_access;
if (!want_access)
continue; // ok
@@ -3908,8 +3920,13 @@ bool check_grant(THD *thd, ulong want_ac
want_access &= ~table->grant.privilege;
goto err; // No grants
}
+
+ /*
+ For SHOW TABLE, SHOW COLUMNS, SHOW INDEX it is enough to have some
+ privileges on any column combination on the table.
+ */
if (show_table)
- continue; // We have some priv on this
+ continue;
table->grant.grant_table=grant_table; // Remember for column test
table->grant.version=grant_version;
@@ -3927,7 +3944,7 @@ bool check_grant(THD *thd, ulong want_ac
}
}
rw_unlock(&LOCK_grant);
- DBUG_RETURN(0);
+ DBUG_RETURN(FALSE);
err:
rw_unlock(&LOCK_grant);
@@ -3941,7 +3958,7 @@ err:
sctx->host_or_ip,
table ? table->table_name : "unknown");
}
- DBUG_RETURN(1);
+ DBUG_RETURN(TRUE);
}
diff -Nrup a/sql/sql_acl.h b/sql/sql_acl.h
--- a/sql/sql_acl.h 2007-09-27 11:25:12 +02:00
+++ b/sql/sql_acl.h 2008-01-10 19:29:52 +01:00
@@ -238,7 +238,7 @@ my_bool grant_init();
void grant_free(void);
my_bool grant_reload(THD *thd);
bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables,
- uint show_command, uint number, bool dont_print_error);
+ bool show_command, uint number, bool dont_print_error);
bool check_grant_column (THD *thd, GRANT_INFO *grant,
const char *db_name, const char *table_name,
const char *name, uint length, Security_context *sctx);
diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc
--- a/sql/sql_parse.cc 2007-12-10 16:43:23 +01:00
+++ b/sql/sql_parse.cc 2008-01-10 19:29:52 +01:00
@@ -2672,7 +2672,7 @@ end_with_restore_list:
if (lex->only_view)
first_table->skip_temporary= 1;
if (check_show_create_table_access(thd, first_table))
- goto error;
+ goto error;
res= mysqld_show_create(thd, first_table);
break;
}
@@ -2680,7 +2680,7 @@ end_with_restore_list:
case SQLCOM_CHECKSUM:
{
DBUG_ASSERT(first_table == all_tables && first_table != 0);
- if (check_table_access(thd, SELECT_ACL | EXTRA_ACL, all_tables, 0))
+ if (check_table_access(thd, SELECT_ACL, all_tables, 0))
goto error; /* purecov: inspected */
res = mysql_checksum_table(thd, first_table, &lex->check_opt);
break;
@@ -4795,24 +4795,30 @@ bool check_one_table_access(THD *thd, ul
}
-/****************************************************************************
- Get the user (global) and database privileges for all used tables
-
- NOTES
- The idea of EXTRA_ACL is that one will be granted access to the table if
- one has the asked privilege on any column combination of the table; For
- example to be able to check a table one needs to have SELECT privilege on
- any column of the table.
-
- RETURN
- 0 ok
- 1 If we can't get the privileges and we don't use table/column grants.
+/**
+ @brief Compare requested privileges with the privileges acquired from the
+ User- and Db-tables.
- save_priv In this we store global and db level grants for the table
- Note that we don't store db level grants if the global grants
- is enough to satisfy the request and the global grants contains
- a SELECT grant.
-****************************************************************************/
+ @param thd Thread handler
+ @param want_access The requested access privileges.
+ @param db A pointer to the Db name.
+ @param[out] save_priv A pointer to the granted privileges will be stored.
+ @param dont_check_global_grants True if no global grants are checked.
+ @param no_error True if no errors should be sent to the client.
+ @param schema_db True if the db specified belongs to the meta data tables.
+
+ 'save_priv' is used to save the User-table (global) and Db-table grants for
+ the supplied db name. Note that we don't store db level grants if the global
+ grants is enough to satisfy the request AND the global grants contains a
+ SELECT grant.
+
+ @see check_grant
+
+ @return Status of denial of access by exclusive ACLs.
+ @retval FALSE Access can't exclusively be denied by Db- and User-table access
+ unless Column- and Table-grants are checked too.
+ @retval TRUE Access denied.
+*/
bool
check_access(THD *thd, ulong want_access, const char *db, ulong *save_priv,
@@ -4851,10 +4857,13 @@ check_access(THD *thd, ulong want_access
DBUG_RETURN(TRUE); /* purecov: tested */
}
+ /*
+ Meta data inspection of meta data.
+ */
if (schema_db)
{
if (!(sctx->master_access & FILE_ACL) && (want_access & FILE_ACL) ||
- (want_access & ~(SELECT_ACL | EXTRA_ACL | FILE_ACL)))
+ (want_access & ~(SELECT_ACL | FILE_ACL)))
{
if (!no_errors)
{
@@ -4862,10 +4871,16 @@ check_access(THD *thd, ulong want_access
my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),
sctx->priv_user, sctx->priv_host, db_name);
}
+
+ /*
+ Access denied;
+ [out] *save_privileges= 0
+ */
DBUG_RETURN(TRUE);
}
else
{
+ /* Access granted */
*save_priv= SELECT_ACL;
DBUG_RETURN(FALSE);
}
@@ -4876,20 +4891,27 @@ check_access(THD *thd, ulong want_access
#else
if ((sctx->master_access & want_access) == want_access)
{
+ /* get access for current db */
+ db_access= sctx->db_access;
/*
- If we don't have a global SELECT privilege, we have to get the database
+ 1. If we don't have a global SELECT privilege, we have to get the database
specific access rights to be able to handle queries of type
UPDATE t1 SET a=1 WHERE b > 0
+ 2. Change db access if it isn't current db which is being addressed
*/
- db_access= sctx->db_access;
if (!(sctx->master_access & SELECT_ACL) &&
(db && (!thd->db || db_is_pattern || strcmp(db,thd->db))))
db_access=acl_get(sctx->host, sctx->ip, sctx->priv_user, db,
db_is_pattern);
+
+ /*
+ The effective privileges are the union of the global privileges
+ and the the intersection of db- and host-privileges.
+ */
*save_priv=sctx->master_access | db_access;
DBUG_RETURN(FALSE);
}
- if (((want_access & ~sctx->master_access) & ~(DB_ACLS | EXTRA_ACL)) ||
+ if (((want_access & ~sctx->master_access) & ~DB_ACLS) ||
! db && dont_check_global_grants)
{ // We can never grant this
DBUG_PRINT("error",("No possible access"));
@@ -4904,24 +4926,48 @@ check_access(THD *thd, ulong want_access
}
if (db == any_db)
- DBUG_RETURN(FALSE); // Allow select on anything
+ {
+ /*
+ Access granted; Allow select on *any* db.
+ [out] *save_privileges= 0 (or possibly SELECT_ACL ?)
+ */
+ DBUG_RETURN(FALSE);
+ }
if (db && (!thd->db || db_is_pattern || strcmp(db,thd->db)))
db_access= acl_get(sctx->host, sctx->ip, sctx->priv_user, db,
db_is_pattern);
else
db_access= sctx->db_access;
- DBUG_PRINT("info",("db_access: %lu", db_access));
- /* Remove SHOW attribute and access rights we already have */
- want_access &= ~(sctx->master_access | EXTRA_ACL);
DBUG_PRINT("info",("db_access: %lu want_access: %lu",
db_access, want_access));
- db_access= ((*save_priv=(db_access | sctx->master_access)) & want_access);
- if (db_access == want_access ||
+ /* Save the union of User- and Db-table privileges. */
+ *save_priv= (db_access | sctx->master_access);
+
+ /*
+ We need to investigate column- and table access if the requested privileges
+ belongs to the bit set (TABLE_ACLS | PROC_ACLS)
+ */
+ bool need_table_or_column_check=
+ (want_access & (TABLE_ACLS | PROC_ACLS)) > 0;
+
+ /*
+ Grant access if the requested access is in the intersection of
+ host- and db-privileges (as retrieved from the acl cache),
+ also grant access if the requested privileges are in the union of
+ TABLES_ACLS and PROC_ACLS; see check_grant.
+ */
+ if ( (db_access & want_access) == want_access ||
(!dont_check_global_grants &&
- !(want_access & ~(db_access | TABLE_ACLS | PROC_ACLS))))
- DBUG_RETURN(FALSE); /* Ok */
+ need_table_or_column_check))
+ {
+ /*
+ Ok; but need to check table- and column privileges.
+ [out] *save_privileges is (User-priv | (Db-priv & Host-priv))
+ */
+ DBUG_RETURN(FALSE);
+ }
DBUG_PRINT("error",("Access denied"));
if (!no_errors)
@@ -4930,7 +4976,13 @@ check_access(THD *thd, ulong want_access
(db ? db : (thd->db ?
thd->db :
"unknown"))); /* purecov: tested */
- DBUG_RETURN(TRUE); /* purecov: tested */
+ {
+ /*
+ Access is denied;
+ [out] *save_privileges is (User-priv | (Db-priv & Host-priv))
+ */
+ DBUG_RETURN(TRUE); /* purecov: tested */
+ }
#endif /* NO_EMBEDDED_ACCESS_CHECKS */
}
@@ -5011,14 +5063,22 @@ static bool check_show_access(THD *thd,
DBUG_ASSERT(dst_table);
- if (check_access(thd, SELECT_ACL | EXTRA_ACL,
- dst_table->db,
- &dst_table->grant.privilege,
- FALSE, FALSE,
- test(dst_table->schema_table)))
- return FALSE;
+ if (check_access(thd, SELECT_ACL,
+ dst_table->db,
+ &dst_table->grant.privilege,
+ FALSE, FALSE,
+ test(dst_table->schema_table)))
+ return TRUE; /* Access denied */
+
+ /*
+ Check_grant will grant access if there is any column privileges on
+ all of the tables thanks to the fourth parameter (bool show_table).
+ */
+ if (check_grant(thd, SELECT_ACL, dst_table, TRUE, UINT_MAX, FALSE))
+ return TRUE; /* Access denied */
- return (check_grant(thd, SELECT_ACL, dst_table, 2, UINT_MAX, FALSE));
+ /* Access granted */
+ return FALSE;
}
default:
break;
@@ -5028,27 +5088,41 @@ static bool check_show_access(THD *thd,
}
-/*
- Check the privilege for all used tables.
-
- SYNOPSYS
- check_table_access()
- thd Thread context
- want_access Privileges requested
- tables List of tables to be checked
- no_errors FALSE/TRUE - report/don't report error to
- the client (using my_error() call).
-
- NOTES
- Table privileges are cached in the table list for GRANT checking.
- This functions assumes that table list used and
- thd->lex->query_tables_own_last value correspond to each other
- (the latter should be either 0 or point to next_global member
- of one of elements of this table list).
+/**
+ @brief Check if the requested privileges exists in either User-, Host- or
+ Db-tables.
- RETURN VALUE
- FALSE - OK
- TRUE - Access denied
+ @param thd Thread context
+ @param want_access Privileges requested
+ @param tables List of tables to be compared against
+ @param no_errors Don't report error to the client (using my_error() call).
+
+ The suppled table list contains cached privileges. This functions calls the
+ help functions check_access and check_grant to verify the first three steps
+ in the privileges check queue:
+ 1. Global privileges
+ 2. OR (db privileges AND host privileges)
+ 3. OR table privileges
+ 4. OR column privileges (not checked by this function!)
+ 5. OR routine privileges (not checked by this function!)
+
+ The idea of EXTRA_ACL is that one will be granted access to the table if
+ one has the asked privilege on any column combination of the table; For
+ example to be able to check a table one needs to have SELECT privilege on
+ any column of the table.
+
+ @see check_access
+ @see check_grant
+
+ @note This functions assumes that table list used and
+ thd->lex->query_tables_own_last value correspond to each other
+ (the latter should be either 0 or point to next_global member
+ of one of elements of this table list).
+
+ @return
+ @retval FALSE OK
+ @retval TRUE Access denied; But column or routine privileges might need to
+ be checked also.
*/
bool
@@ -5058,6 +5132,8 @@ check_table_access(THD *thd, ulong want_
#ifndef NO_EMBEDDED_ACCESS_CHECKS
TABLE_LIST *org_tables= tables;
#endif
+ bool have_extra_acl= (want_access & EXTRA_ACL) != 0;
+ want_access= want_access & ~EXTRA_ACL;
TABLE_LIST *first_not_own_table= thd->lex->first_not_own_table();
Security_context *sctx= thd->security_ctx, *backup_ctx= thd->security_ctx;
/*
@@ -5073,7 +5149,7 @@ check_table_access(THD *thd, ulong want_
sctx= backup_ctx;
if (tables->schema_table &&
- (want_access & ~(SELECT_ACL | EXTRA_ACL | FILE_ACL)))
+ (want_access & ~(SELECT_ACL | FILE_ACL)))
{
if (!no_errors)
my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),
@@ -5091,7 +5167,6 @@ check_table_access(THD *thd, ulong want_
{
if (check_show_access(thd, tables))
goto deny;
-
continue;
}
@@ -5100,22 +5175,15 @@ check_table_access(THD *thd, ulong want_
continue;
thd->security_ctx= sctx;
if ((sctx->master_access & want_access) ==
- (want_access & ~EXTRA_ACL) &&
- thd->db)
+ want_access && thd->db)
tables->grant.privilege= want_access;
- else if (tables->db && thd->db && strcmp(tables->db, thd->db) == 0)
- {
- if (check_access(thd,want_access,tables->db,&tables->grant.privilege,
- 0, no_errors, test(tables->schema_table)))
- goto deny; // Access denied
- }
else if (check_access(thd,want_access,tables->db,&tables->grant.privilege,
- 0, no_errors, test(tables->schema_table)))
+ 0, no_errors, test(tables->schema_table)))
goto deny;
}
thd->security_ctx= backup_ctx;
- return check_grant(thd,want_access & ~EXTRA_ACL,org_tables,
- test(want_access & EXTRA_ACL), UINT_MAX, no_errors);
+ return check_grant(thd,want_access,org_tables, have_extra_acl, UINT_MAX,
+ no_errors);
deny:
thd->security_ctx= backup_ctx;
return TRUE;
@@ -7047,6 +7115,8 @@ bool insert_precheck(THD *thd, TABLE_LIS
/**
@brief Check privileges for SHOW CREATE TABLE statement.
+ The required privileges are: Any privileges on any column combination.
+
@param thd Thread context
@param table Target table
@@ -7056,10 +7126,37 @@ bool insert_precheck(THD *thd, TABLE_LIS
static bool check_show_create_table_access(THD *thd, TABLE_LIST *table)
{
- return check_access(thd, SELECT_ACL | EXTRA_ACL, table->db,
- &table->grant.privilege, 0, 0,
- test(table->schema_table)) ||
- check_grant(thd, SELECT_ACL, table, 2, UINT_MAX, 0);
+ DBUG_ENTER("check_show_create_table_access");
+
+ DBUG_ASSERT(table);
+
+ if (check_access(thd, SELECT_ACL,
+ table->db,
+ &table->grant.privilege,
+ FALSE, FALSE,
+ test(table->schema_table)))
+ DBUG_RETURN(TRUE); /* Access denied */
+
+ /*
+ Check_grant will grant access if there is any column privileges on
+ all of the tables thanks to the fourth parameter (bool show_table).
+ */
+ if (check_grant(thd, SELECT_ACL, table, TRUE, UINT_MAX, FALSE))
+ DBUG_RETURN(TRUE); /* Access denied */
+
+ /*
+ If dst_table->grant_privilege is != 0; then there exists column
+ privileges on the table.
+ */
+ //if (dst_table->grant.privilege)
+ // return FALSE;
+
+ /* Access granted */
+ DBUG_RETURN(FALSE);
+ //DBUG_RETURN(check_access(thd, SELECT_ACL, table->db,
+ // &table->grant.privilege, 0, 0,
+ // test(table->schema_table)) ||
+ // check_grant(thd, SELECT_ACL, table, FALSE, UINT_MAX, 0));
}
@@ -7129,7 +7226,7 @@ bool create_table_precheck(THD *thd, TAB
}
else if (lex->create_info.options & HA_LEX_CREATE_TABLE_LIKE)
{
- if (check_show_create_table_access(thd, tables))
+ if (check_table_access(thd, SELECT_ACL, tables, 0))
goto err;
}
error= FALSE;
diff -Nrup a/sql/sql_show.cc b/sql/sql_show.cc
--- a/sql/sql_show.cc 2007-11-21 21:09:25 +01:00
+++ b/sql/sql_show.cc 2008-01-10 19:29:53 +01:00
@@ -3658,7 +3658,7 @@ static int get_schema_column_record(THD
#ifndef NO_EMBEDDED_ACCESS_CHECKS
uint col_access;
- check_access(thd,SELECT_ACL | EXTRA_ACL, db_name->str,
+ check_access(thd,SELECT_ACL, db_name->str,
&tables->grant.privilege, 0, 0, test(tables->schema_table));
col_access= get_column_grant(thd, &tables->grant,
db_name->str, table_name->str,
| Thread |
|---|
| • bk commit into 5.1 tree (thek:1.2681) BUG#27145 | kpettersson | 10 Jan |
