maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #09550
Re: [Commits] c204e9b: MDEV-9610 Trigger on normal table can't insert into CONNECT engine table - Access Denied
Hi Sergei,
On 04/25/2016 08:42 PM, serg@xxxxxxxxxxx wrote:
revision-id: c204e9bd63e7f6ce40e83b1f7fc0b4607a131009 (mariadb-10.0.24-43-gc204e9b)
parent(s): ce38adddfa91949b30956abd0e4cab201effcdef
author: Sergei Golubchik
committer: Sergei Golubchik
timestamp: 2016-04-25 18:41:31 +0200
message:
MDEV-9610 Trigger on normal table can't insert into CONNECT engine table - Access Denied
in case of prelocking, don't check table->grant.privilege
in handler::external_lock(), do it in
handler::start_stmt().
---
storage/connect/ha_connect.cc | 58 ++++++++++++++++++++--
storage/connect/ha_connect.h | 1 +
storage/connect/mysql-test/connect/r/grant3.result | 5 ++
storage/connect/mysql-test/connect/t/grant3.test | 11 ++++
4 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/storage/connect/ha_connect.cc b/storage/connect/ha_connect.cc
index 645d000..1032567 100644
--- a/storage/connect/ha_connect.cc
+++ b/storage/connect/ha_connect.cc
@@ -4054,6 +4054,14 @@ int ha_connect::delete_all_rows()
} // end of delete_all_rows
+static void issue_access_denied_error(THD *thd)
+{
+ status_var_increment(thd->status_var.access_denied_errors);
+ my_error(access_denied_error_code(thd->password), MYF(0),
+ thd->security_ctx->priv_user, thd->security_ctx->priv_host,
+ (thd->password ? ER(ER_YES) : ER(ER_NO)));
+}
+
bool ha_connect::check_privileges(THD *thd, PTOS options, char *dbn)
{
const char *db= (dbn && *dbn) ? dbn : NULL;
@@ -4124,12 +4132,9 @@ bool ha_connect::check_privileges(THD *thd, PTOS options, char *dbn)
*/
if (!table || !table->mdl_ticket || table->mdl_ticket->get_type() == MDL_EXCLUSIVE)
return check_access(thd, FILE_ACL, db, NULL, NULL, 0, 0);
- if (table->grant.privilege & FILE_ACL)
+ if (thd->lex->requires_prelocking() || table->grant.privilege & FILE_ACL)
return false;
- status_var_increment(thd->status_var.access_denied_errors);
- my_error(access_denied_error_code(thd->password), MYF(0),
- thd->security_ctx->priv_user, thd->security_ctx->priv_host,
- (thd->password ? ER(ER_YES) : ER(ER_NO)));
+ issue_access_denied_error(thd);
return true;
// This is temporary until a solution is found
@@ -4146,6 +4151,46 @@ bool ha_connect::check_privileges(THD *thd, PTOS options, char *dbn)
return true;
} // end of check_privileges
+bool ha_connect::check_cached_privileges(THD *thd)
+{
+ PTOS options= GetTableOptionStruct();
+ TABTYPE type=GetRealType(options);
+
+#ifdef NO_EMBEDDED_ACCESS_CHECKS
+ return false;
+#endif
+
+ switch (type) {
+ case TAB_DOS:
+ case TAB_FIX:
+ case TAB_BIN:
+ case TAB_CSV:
+ case TAB_FMT:
+ case TAB_DBF:
+ case TAB_XML:
+ case TAB_INI:
+ case TAB_VEC:
+ case TAB_JSON:
+ if (!options->filename || !*options->filename)
+ return false;
+ /* Fall through to check FILE_ACL */
+ case TAB_ODBC:
+ case TAB_MYSQL:
+ case TAB_DIR:
+ case TAB_MAC:
+ case TAB_WMI:
+ case TAB_OEM:
+ if (table->grant.privilege & FILE_ACL)
+ return false;
+ issue_access_denied_error(thd);
+ return true;
+ default:
The patch looks fine.
I'd suggest two things.
1. Don't use "default"
Can you please list all possible type value instead of "default"?
Otherwise, when a new table type is added the next time, it will be
easier to forget to update check_cached_privileges() accordingly.
2. Try to avoid duplicate code.
Btw, I'd probably try to remove the duplicate switch,
by adding a new method like this:
enum Extra_acl
{
CONNECT_SECURE_FILE_PATH= 1,
CONNECT_FILE_ACL= 1<<1;
}
int ha_connect::extra_acl_needed(THD *thd, PTOS options)
{
...
switch (GetRealType(options)) {
...
case TAB_DOS:
case TAB_FIX:
case TAB_BIN:
case TAB_CSV:
case TAB_FMT:
case TAB_DBF:
case TAB_XML:
case TAB_INI:
case TAB_VEC:
case TAB_JSON:
return (options->filename && *options->filename) ?
CONNECT_SECURE_FILE_PATH : 0;
case TAB_ODBC:
case TAB_MYSQL:
case TAB_DIR:
case TAB_MAC:
case TAB_WMI:
case TAB_OEM:
#ifdef NO_EMBEDDED_ACCESS_CHECKS
return 0;
#endif
return CONNECT_FILE_ACL;
...
}
...
}
And then reuse it in both ha_connect::check_privilege()
and ha_connect::check_cached_privileges().
The latter would look about like this:
bool ha_connect::check_cached_privileges(THD *thd)
{
PTOS options= GetTableOptionStruct();
if (!(extra_acl_needed(options) & CONNECT_FILE_ACL) ||
(table->grant.privilege & FILE_ACL))
return false;
issue_access_denied_error(thd);
return true;
}
+ return false;
+ }
+ return false;
+
+} // end of check_cached_privileges
+
// Check that two indexes are equivalent
bool ha_connect::IsSameIndex(PIXDEF xp1, PIXDEF xp2)
{
@@ -4308,6 +4353,9 @@ int ha_connect::start_stmt(THD *thd, thr_lock_type lock_type)
PGLOBAL g= GetPlug(thd, xp);
DBUG_ENTER("ha_connect::start_stmt");
+ if (check_cached_privileges(thd))
+ DBUG_RETURN(HA_ERR_INTERNAL_ERROR);
+
// Action will depend on lock_type
switch (lock_type) {
case TL_WRITE_ALLOW_WRITE:
diff --git a/storage/connect/ha_connect.h b/storage/connect/ha_connect.h
index 05cc872..fae804b 100644
--- a/storage/connect/ha_connect.h
+++ b/storage/connect/ha_connect.h
@@ -537,6 +537,7 @@ int index_prev(uchar *buf);
protected:
bool check_privileges(THD *thd, PTOS options, char *dbn);
+ bool check_cached_privileges(THD *thd);
It looks like the number of leading spaces is wrong here.
MODE CheckMode(PGLOBAL g, THD *thd, MODE newmode, bool *chk, bool *cras);
char *GetDBfromName(const char *name);
diff --git a/storage/connect/mysql-test/connect/r/grant3.result b/storage/connect/mysql-test/connect/r/grant3.result
new file mode 100644
index 0000000..2f9d37b
--- /dev/null
+++ b/storage/connect/mysql-test/connect/r/grant3.result
@@ -0,0 +1,5 @@
+create table tcon (i int) engine=Connect table_type=DOS file_name='tcon.dos';
+create table tin (i int);
+create trigger tr after insert on tin for each row insert into tcon values (new.i);
+insert into tin values (1);
+drop table tin,tcon;
diff --git a/storage/connect/mysql-test/connect/t/grant3.test b/storage/connect/mysql-test/connect/t/grant3.test
new file mode 100644
index 0000000..9f05ca7
--- /dev/null
+++ b/storage/connect/mysql-test/connect/t/grant3.test
@@ -0,0 +1,11 @@
+#
+# MDEV-9610 Trigger on normal table can't insert into CONNECT engine table - Access Denied
+#
+create table tcon (i int) engine=Connect table_type=DOS file_name='tcon.dos';
+create table tin (i int);
+create trigger tr after insert on tin for each row insert into tcon values (new.i);
+insert into tin values (1);
+drop table tin,tcon;
+
+let datadir=`select @@datadir`;
+remove_file $datadir/test/tcon.dos;
_______________________________________________
commits mailing list
commits@xxxxxxxxxxx
https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
Follow ups
