- developers - lists.mariadb.org

Re: [Maria-developers] f9f290b: MDEV-9851: CREATE USER w/o IDENTIFIED BY clause causes crash when using cracklib plugin
by Sergei Golubchik 29 Apr '16

29 Apr '16

Hi, Nirbhay! On Mar 31, Nirbhay Choubey wrote: > revision-id: f9f290b6828eeb57cba611d006d2a9301dc52244 (mariadb-10.1.13-3-gf9f290b) > parent(s): f4d5fe277599da4549c97c660f324c88cf9a2542 > author: Nirbhay Choubey > committer: Nirbhay Choubey > timestamp: 2016-03-31 18:03:44 -0400 > message: > > MDEV-9851: CREATE USER w/o IDENTIFIED BY clause causes crash when using cracklib plugin > > Add a check for NULL password. > > diff --git a/plugin/cracklib_password_check/cracklib_password_check.c b/plugin/cracklib_password_check/cracklib_password_check.c > index c593173..c192cdf 100644 > --- a/plugin/cracklib_password_check/cracklib_password_check.c > +++ b/plugin/cracklib_password_check/cracklib_password_check.c > @@ -33,7 +33,8 @@ static int crackme(MYSQL_LEX_STRING *username, MYSQL_LEX_STRING *password) > if ((host= strchr(user, '@'))) > *host++= 0; > > - if ((res= FascistCheckUser(password->str, dictionary, user, host))) > + if ((password->str == NULL) || // No password > + (res= FascistCheckUser(password->str, dictionary, user, host))) > { > my_printf_error(ER_NOT_VALID_PASSWORD, "cracklib: %s", > MYF(ME_JUST_WARNING), res); You forgot to fix the simple_password_check plugin. And if all plugins need to do the same check - it's a strong indication that this should've been done in the server. So, please, fix this in sql_acl.cc instead. Like this: - struct validation_data data= { &user->user, &user->pwtext }; + struct validation_data data= { &user->user, user->pwtext.str ? &user->pwtext : &empy_lex_str }; Ok to push with this fix and your test case. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

Re: [Maria-developers] 95e1c46: MDEV-9898 SET ROLE NONE can crash mysqld.
by Sergei Golubchik 29 Apr '16

29 Apr '16

Hi, Alexey! On Apr 28, Alexey Botchkov wrote: > revision-id: 95e1c46c1709e7244332a025a527cacceab7da60 (mariadb-10.1.13-14-g95e1c46) > parent(s): 646c4cea58afbb369021a3d7b5ccbbf6aed708d4 > committer: Alexey Botchkov > timestamp: 2016-04-28 13:36:36 +0400 > message: > > MDEV-9898 SET ROLE NONE can crash mysqld. > The check_user_can_set_role() used find_user_exact() to get the > permissions for the SET ROLE NONE command. > Which returned NULL too often, for instance when user > authenticated as 'user'@'%'. > Now we use find_user_or_anon() instead. No, this is wrong. SET ROLE NONE should use the same rule as SET ROLE name. And the latter uses acl_user->wild_eq(user, host, ip). Thus SET ROLE NONE should find the user with ACL_USER::wild_eq, that is, it should use find_user_wild(). Ok to push with find_user_wild(). Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] MariaDB packages for Alpine Linux
by Guillaume Lefranc 29 Apr '16

29 Apr '16

Hello, there is an outstanding request in docker-library/mariadb for Alpine support: https://github.com/docker-library/mariadb/issues/53 What I know is that we currently don't build packages for Alpine. Alpine is going to be the platform of choice for Docker containers in the future (you can read about the various advantages of Alpine here: https://www.brianchristner.io/docker-is-moving-to-alpine-linux/) Federico Razzoli has managed to make MariaDB 10.1 work in Alpine with some caveats (no tokudb, no oqgraph, no galera). You can look at his work here https://hub.docker.com/r/santec/mariadb-alpine/. I admit I don't know about portability of Galera lib on Alpine - should probably talk to Codership about that. Is there any way we can make this happen?

1 0

[Maria-developers] Why reserved keywords: UTC_TIME, UTC_DATE, UTC_DATETIME
by Alexander Barkov 28 Apr '16

28 Apr '16

Hello, I noticed that UTC_TIME, UTC_DATE, UTC_DATETIME are reserved keywords: MariaDB [test]> SELECT * FROM utc_time; ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'utc_time' at line 1 I think they should not. - These keywords are not mentioned in the SQL standard - They are not involved in any complex grammar that would prevent them from being non-reserved keywords Looks like a bug... Any objections to make them non-reserved keywords in 10.2?

6 8

Re: [Maria-developers] GSoC 2016: Unique indexes for blobs
by Sergei Golubchik 27 Apr '16

27 Apr '16

Hi, Shubham! On Apr 24, Shubham Barai wrote: > Hi Sergei, > > I have created github repository with a new branch for my prototype. Great, what's the url? btw, please, use "Reply to all", so that the whole list could see your progress. Also google recommends you to blog regularly about what you're doing: http://write.flossmanuals.net/gsocstudentguide/time-management-for-students/ but if you don't like that, you can send weekly progress report emails instead. Whatever suits you best. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 6

[Maria-developers] GSoC Student Introduction
by Charles Muurmu 26 Apr '16

26 Apr '16

Hi everyone, I am Charles Muurmu also known as laserlight on IRC and I was selected for GSoC 2016 to work on migrating MariaDB Cassandra storage engine from the Thrift API to Datastax driver. I will be mentored by Spetrunia. My goals for this community bonding period will be get myself ready by accomplishing the following tasks: Fork and clone the server code. Try to build the code. Setup a blog on which I will post weekly reports on progress. Study the Datastax C++ driver API and documentation. Study the ha_cassandra.xx code and storage engines architecture. I believe these will prepare me for my first task when coding begins which will be to integrate the Datastax C++ driver into the MariaDB Server build system. I equally look forward to more instructions from my Mentor on how we will work on this task and lastly, I would love to congratulate the other students selected with MariaDB. Cheers, laserlight.

1 0

Re: [Maria-developers] [Commits] c204e9b: MDEV-9610 Trigger on normal table can't insert into CONNECT engine table - Access Denied
by Alexander Barkov 26 Apr '16

26 Apr '16

Hi Sergei, On 04/25/2016 08:42 PM, serg(a)mariadb.org 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(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits >

2 1

Re: [Maria-developers] Deadlock with STOP SLAVE and LOCK_active_mi
by Michael Widenius 26 Apr '16

26 Apr '16

Hi! >>> Any suggestions for how this is supposed to work? Or is it just broken >>> by design, but saved because normally slave threads do not need to >>> access SHOW STATUS or system variables? <cut> > Hm. So I went through all of the code that references LOCK_active_mi to try > and understand this. It seems there are two main uses: > > 1. As you say, to serialise admin commands. I think the list is: end_slave() > (server shutdown); CHANGE MASTER; START/STOP SLAVE; START/STOP ALL SLAVES; > RESET SLAVE; FLUSH SLAVE. > > 2. To protect access to master_info_index, eg. to prevent a Master_info from > disappearing while it is accessed. When reading about this, I think it would be better to have a counter in Master_info if someone is using it and only delete if if the counter is zero. Would be trivial to add an increment of the counter in get_master_info(). The counter could either be protected by the LOCK_active_mi or one of the mutexes in Master_info like data_lock. > There is a comment in rpl_rli.h that LOCK_active_mi protects > Relay_log_info::inited, but I did not understand it - maybe it is wrong? Setting inited is indirectly protected by LOCK_active_mi, as init_all_master_info() which calls init_master_info(), is protected by LOCK_active_mi. Looking at the code, I didn't however see any need to protect inited as this is an internal flag that is always 1 after start. It's main usage is to avoid some re-initialization of relay logs on retries. > So the idea would be to make a new lock for (1), and keep LOCK_active_mi for > (2). > > Actually, using a lock for (1) seems a bad idea - STOP SLAVE can take a long > time to run, and a mutex makes it unkillable. A condition variable might be > better (to make STOP SLAVE killable). But I guess that is a separate issue, > it would not affect the possibility of deadlocks. Agree that a condition variable would be better, but not critical as one has already a problem if stop_slave doesn't work. > Both LOCK_active_mi and the new LOCK_serialize_replication_admin_commands > must by themselves prevent master_info_index from having elements added or > removed. This is needed for start_all_slaves() and stop_all_slaves() to work > correctly, I think. Why both? Isn't it enough to first take LOCK_serialize_replication_admin_commands and then take LOCK_active_mi if one needs to access master_info_index ? > Something will need to be done for remove_master_info(). Currently, it also > tries to stop slave threads (from within free_key_master_info()) while > holding LOCK_active_mi. It seems that threads should be stopped before > attempting to remove their mi from master_info_index, probably? To do that we would need to: - Add a flag in Master_info that it's not usable anymore and change this flag only under LOCK_active_mi. get_master_info() could wait (on a condition) if this flag is set. - Add a counter that Master_info is in use. - Add a function to release Master_info. - Call terminate_slave_threads() outside of free_key_master_info() > Actually, there would still be the deadlock problem after introducing the > new lock, because it is possible for a slave thread to run CHANGE MASTER! > (and I think START SLAVE / RESET SLAVE as well). But that is probably a bug, > does not seem good that this is possible. I guess these need a check to > throw an ER_LOCK_OR_ACTIVE_TRANSACTION error, like STOP SLAVE. Wouldn't the new lock LOCK_serialize_replication_admin_commands fix that? > I think I also found another lock problem while reading the code, for > MASTER_POS_WAIT() og SHOW BINLOG EVENTS. They both grab a Master_info under > LOCK_active_mi, but then proceed to access it after releasing the lock. I do > not see anything that prevents the Master_info to disappear under their feet > if RESET SLAVE ALL is run in another thread? But again, that's a separate > issue, I suppose. Probably there are some other tricky deadlock issues > lurking here. I checked the function mysql_show_binlog_events() but could not find any access to Master_info after mysql_mutex_unlock(&LOCK_active_mi) was released. > I don't know. It seems splitting into a new > LOCK_serialize_replication_admin_commands could solve the deadlock issue, > maybe, if the other problems mentioned above are addressed. It seems very > hard to convince oneself that this would not introduce new problems > somewhere, the locking in replication is really complex and feels very > fragile :-/ It doesn't really feel like something one would want to do in > 10.1 (certainly not 10.0), but maybe 10.2? > > It's kind of a very edge-case problem (and seems to have been there for a > _long_ time). Still, hanging the mysqld process hard, requiring kill -9, is > also not nice... Adding two new flags, one if master_info is in use and one if it's unusable, shouldn't be that hard to make reasonable safe in 10.1 I am now back in Finland and working. Feel free to call me to discuss this any time. Regards, Monty

2 2

[Maria-developers] Problem that some variables can be changed without stopping the slave IO thread
by Kristian Nielsen 25 Apr '16

25 Apr '16

Monty, This patch of yours introduced a change where before both slave threads must be stopped to change @@replicate_events_marked_for_skip, but after only the SQL thread is checked: ----------------------------------------------------------------------- commit 572560f38c248d5020f0e63aeb3f8905cd568208 Author: Michael Widenius <monty(a)askmonty.org> Date: Wed Oct 3 01:44:54 2012 +0300 Changed SHOW_FUNC variabels that don't return SHOW_ARRAY to SHOW_SIMPLE_FUNC. + Master_info_index::give_error_if_slave_running() + + @return + TRUE If some slave is running. An error is printed + FALSE No slave is running +*/ + +bool Master_info_index::give_error_if_slave_running() +{ + DBUG_ENTER("warn_if_slave_running"); + mysql_mutex_assert_owner(&LOCK_active_mi); + + for (uint i= 0; i< master_info_hash.records; ++i) + { + Master_info *mi; + mi= (Master_info *) my_hash_element(&master_info_hash, i); + if (mi->rli.slave_running != MYSQL_SLAVE_NOT_RUN) + { + my_error(ER_SLAVE_MUST_STOP, MYF(0), (int) mi->connection_name.length, + mi->connection_name.str); + DBUG_RETURN(TRUE); + } + } + DBUG_RETURN(FALSE); +} bool Sys_var_replicate_events_marked_for_skip::global_update(THD *thd, set_var *var) ... - /* Slave threads must be stopped to change the variable. */ + mysql_mutex_unlock(&LOCK_global_system_variables); mysql_mutex_lock(&LOCK_active_mi); - lock_slave_threads(active_mi); - init_thread_mask(&thread_mask, active_mi, 0 /*not inverse*/); - if (thread_mask) // We refuse if any slave thread is running - { - my_error(ER_SLAVE_MUST_STOP, MYF(0)); - result= true; - } - else + if (!master_info_index->give_error_if_slave_running()) ----------------------------------------------------------------------- So instead of checking both IO and SQL thread, now it checks only SQL thread (mi->rli.slave_running). Why was this change made? Was it by mistake? Unfortunately, I did not notice this before, so I subsequently used the same code (give_error_if_slave_running()) for the following variables: gtid_slave_pos slave_parallel_threads slave_domain_parallel_threads replicate_events_marked_for_skip gtid_ignore_duplicates This means that all of the above variables can be changed while the IO thread is still running. This was never the intention! There are some complex interactions between the master, the IO thread, and the SQL thread, particularly with respect to the slave's GTID position, and parts of the code are specifically written on the assumption that certain things cannot change without first stopping all replication threads. (I do not have the full overview of exactly what can break, I think that would require a careful review of large parts of the code, but I fear that it can corrupt not just the replication GTID position and possibly even internal server structures). So I am wondering what to do ... it is easy to fix give_error_if_slave_running() to check for both threads, but this is a change in behaviour (even if it should match documentation) that could easily break administration scripts, eg. after a 10.0 automatic security-fix upgrade. I also know that eg. Jean François has written about how he uses the feature of being able to not stop the IO thread in some cases. On the other hand, this can cause serious problems for users, as they have no real way of knowing not to attempt to change these variables without stopping IO thread first, when there is no error given. MDEV-9138 describes a complex scenario where slave position gets corrupted from this. Any suggestions? Thanks, - Kristian.

1 0

[Maria-developers] newest CVEs in mysql | mariadb vulnerable?
by Christian Rebischke 23 Apr '16

23 Apr '16

Hello, I just want to inform everybody about the newest mysql CVEs: Apr 21 CVE-2016-0666 Apr 21 CVE-2016-0663 Apr 21 CVE-2016-0665 Apr 21 CVE-2016-0661 Apr 21 CVE-2016-0662 Apr 21 CVE-2016-0659 Apr 21 CVE-2016-0658 Apr 21 CVE-2016-0657 Apr 21 CVE-2016-0656 Apr 21 CVE-2016-0654 Apr 21 CVE-2016-0655 Apr 21 CVE-2016-0653 Apr 21 CVE-2016-0652 Apr 21 CVE-2016-0651 Apr 21 CVE-2016-0650 Apr 21 CVE-2016-0649 Apr 21 CVE-2016-0648 Apr 21 CVE-2016-0647 Apr 21 CVE-2016-0646 Apr 21 CVE-2016-0644 Apr 21 CVE-2016-0643 Apr 21 CVE-2016-0642 Apr 21 CVE-2016-0641 Apr 21 CVE-2016-0639 Apr 21 CVE-2016-0640 Is there a list of current CVEs for mariadb there I can compare them? Best regards Chris

2 1