May 2020 - developers - lists.mariadb.org

[Maria-developers] OSX fix need on MariaDB-10.5
by Daniel Black 09 May '20

09 May '20

Mac lovers (who can use C/C++ and a debugger), As your MariaDB developers are hard at work getting 10.5 ready for you, unfortunately a bug has slipped into the 10.5 branch making it unusable on OSX (at least that's what it looks like on the CI infrastructure). The bug report is here: https://jira.mariadb.org/browse/MDEV-22173 As the time of developers is a little short as is the availability of OSX hardware, I'm hoping some of you are able to spend a few hours tackling this task to understand it better, and if possible, create patch. Steps: 1. build environment https://mariadb.com/kb/en/Build_Environment_Setup_for_Mac/ 2. clone the source git clone --branch 10.5 https://github.com/MariaDB/server.git mariadb-server cd mariadb-server git submodule update --recursive --init 3. build instructions https://mariadb.com/kb/en/generic-build-instructions/ (you don't need to install) 4. test (from build directory) mysql-test/mysql-test-run main.select might be useful here: --debugger=NAME or --gdb client-debugger=NAME or client-gdb To collaborative share your findings and to ask for further help MariaDB's community chat stream has been created for you. https://mariadb.zulipchat.com/#narrow/stream/118759-general/topic/Get.20OSX… To paraphrase J.F.Kennedy ask not what your database server community can do for you — ask what you can do for your database server community. Keep the community spirit going, Daniel Black -- Sent as a community member who noticed OSX failures and that everyone is busy

2 3

Re: [Maria-developers] 4312f382b6f: MDEV-22312: Bad error message for SET DEFAULT ROLE when user account is not granted the role
by Sergei Golubchik 08 May '20

08 May '20

Hi, Anel! On May 08, Anel Husakovic wrote: > revision-id: 4312f382b6f (mariadb-10.1.43-138-g4312f382b6f) > parent(s): 8c4b5261210 > author: Anel Husakovic <anel(a)mariadb.org> > committer: Anel Husakovic <anel(a)mariadb.org> > timestamp: 2020-05-07 16:49:33 +0200 > message: > > MDEV-22312: Bad error message for SET DEFAULT ROLE when user account is not granted the role > > diff --git a/mysql-test/suite/roles/set_role-recursive.test b/mysql-test/suite/roles/set_role-recursive.test > index 23d623e1966..dcf3dd5b6fc 100644 > --- a/mysql-test/suite/roles/set_role-recursive.test > +++ b/mysql-test/suite/roles/set_role-recursive.test > @@ -44,7 +44,7 @@ select * from mysql.roles_mapping; > > --sorted_result > show grants; > ---error ER_INVALID_ROLE > +--error ER_SET_DEFAULT_ROLE_FOR_USER > set role test_role2; > select current_user(), current_role(); > --sorted_result > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt > index c5e83062f0f..83ecd4735e8 100644 > --- a/sql/share/errmsg-utf8.txt > +++ b/sql/share/errmsg-utf8.txt > @@ -7144,3 +7144,5 @@ ER_WARN_AGGFUNC_DEPENDENCE > ukr "Агрегатна функція '%-.192s)' з SELECTу #%d належить до SELECTу #%d" > WARN_INNODB_PARTITION_OPTION_IGNORED > eng "<%-.64s> option ignored for InnoDB partition" > +ER_SET_DEFAULT_ROLE_FOR_USER > + eng "User '%s' has not been granted a role '%s'." Sorry, this is an absolute no-no. We cannot add new error messages to the old GA versions. Only to the very last RC/GA or to any alpha/beta. When you add an error message, all error messages added after it in newer versions will shift, change their numbers. And error numbers in RC/GA versions cannot change. Instead of a new error message and my_error(ER_SET_DEFAULT_ROLE_FOR_USER, MYF(0), c_usr.ptr(), rolename); you have to write my_printf_error(ER_INVALID_ROLE, "User %`s has not been granted a role %`s", MYF(0), c_usr.c_ptr(), rolename); note other changes: * %`s instead of '%s' for correct identifier quoting * no dot at the end * c_ptr() because printf expects a zero-terminated C string, and ptr() doesn't guarantee that. > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index b6a6f806e50..37141dd2390 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -2041,6 +2041,9 @@ static int check_user_can_set_role(const char *user, const char *host, > ACL_USER_BASE *acl_user_base; > ACL_USER *UNINIT_VAR(acl_user); > bool is_granted= FALSE; > + bool user_can_see_role= FALSE; > + char buf[1024]; > + String c_usr(buf, sizeof(buf), &my_charset_bin); use StringBuffer<1024> c_str; > int result= 0; > > /* clear role privileges */ > @@ -2083,14 +2086,39 @@ static int check_user_can_set_role(const char *user, const char *host, > is_granted= TRUE; > break; > } > + > + for (uint i=0; i < acl_user->role_grants.elements; i++) > + { > + ACL_ROLE *r= *(dynamic_element(&acl_user->role_grants, i, ACL_ROLE**)); > + if (!strcmp(safe_str(r->user.str), safe_str(rolename))) > + { > + size_t len= acl_user->user.length + acl_user->hostname_length + 2; > + c_usr.alloc(len); > + const char c= '@'; > + strmov(strmov(strmov(const_cast<char*>(c_usr.ptr()), > + safe_str(user)), &c), safe_str(host)); > + user_can_see_role= TRUE; > + } > + } No, you repeat this for() block for evert role grant of every role's grantee. And you only use it when there was an error, an uncommon case. Do not do the work that you aren't going to use. And the check doesn't seem correct. You want to use the same rule as for applicable_roles, you need to traverse the graph. Perhaps you can simply use check_role_is_granted() though. And also you need to check whether the current user has read access on mysql.user table - in that case it can see all roles, granted or not. > } > > /* According to SQL standard, the same error message must be presented */ this comment now belongs inside the if(), above ER_INVALID_ROLE > if (!is_granted) > { > - my_error(ER_INVALID_ROLE, MYF(0), rolename); > - result= 1; > - goto end; > + /* Handling of the bad error message - MDEV-22312 */ don't put MDEV numbers into the code, please. > + /* If the current user can see the role generate different type of error. */ > + if(user_can_see_role) > + { > + my_error(ER_SET_DEFAULT_ROLE_FOR_USER, MYF(0), c_usr.ptr(), rolename); > + result= 1; > + goto end; > + } > + else > + { > + my_error(ER_INVALID_ROLE, MYF(0), rolename); > + result= 1; > + goto end; > + } > } > > if (access) Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] Welcome to Google Summer of Code 2020 with MariaDB
by Vicențiu Ciorbaru 04 May '20

04 May '20

Hi! GSoC 2020 accepted proposals have just been announced and you are part of the successful applicants. This year we had a lot of very good proposals and so we have a one of the highest number of students (7) to mentor for GSoC. Congratulations on the effort to learn about the program, get involved and submit successful proposals! The fun part begins now! If you have not done so already, please reach out to your mentor and seek further advice about your project. Each one of your projects can be implemented in multiple ways, some more complex than others. I strongly advise you to create a concrete plan based off of your proposal with your mentor and aim to stick to it. The MariaDB Server codebase is a complex one with lots of different parts, however your mentor can guide you to ensure you have a successful GSoC. Remember that you are here to learn about Open Source, what it means to be part of an Open Source community, to interact with the community, all while becoming better programmers. Please take advantage of this opportunity and keep your technical discussions public, on Zulip https://mariadb.zulipchat.com, and on our developer mailing list https://launchpad.net/~maria-developers. By keeping your discussions public, you get the benefit of not only having your mentor see your questions but all of the MariaDB community. You never know when someone might lend a hand in case your mentor is not yet online. :) The mantra of our community should be that we are always welcoming to new people. Do not be afraid to ask questions. There are no bad or stupid or silly questions. The only key requirement is that you first make an effort to find the answer yourself and only afterwards seek outside help. If you can show a bit of involvement, the community will show you the same. I recommend that you agree with your mentor on some form of weekly progress reporting, via either a blog-post, an email, or something similar that can be publicly shared. I can not stress this enough: we are working on an open source project, our work must be done in the open and it is important to *show the world the good we are doing*. Hope all of you will have a great GSoC! Vicențiu MariaDB Foundation GSoC Admin

1 0

Re: [Maria-developers] d08860b28f3: MDEV-22374: VIEW with security definer require FILE privilege from definer not invoker in case of INTO OUTFILE
by Sergei Golubchik 04 May '20

04 May '20

Hi, Oleksandr! On May 04, Oleksandr Byelkin wrote: > revision-id: d08860b28f3 (mariadb-5.5.67-15-gd08860b28f3) > parent(s): ac2604f923f > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2020-04-28 09:16:33 +0200 > message: > > MDEV-22374: VIEW with security definer require FILE privilege from definer not invoker in case of INTO OUTFILE > > Check INTO OUTFILE clause always from invoker. > > --- > mysql-test/r/view_grant.result | 21 +++++++++++++++++++++ > mysql-test/t/view_grant.test | 33 +++++++++++++++++++++++++++++++++ > sql/sql_parse.cc | 18 +++++++++--------- > 3 files changed, 63 insertions(+), 9 deletions(-) > > diff --git a/mysql-test/r/view_grant.result b/mysql-test/r/view_grant.result > index b2d3a0b8ca4..37dc6adc978 100644 > --- a/mysql-test/r/view_grant.result > +++ b/mysql-test/r/view_grant.result > @@ -1587,3 +1587,24 @@ USE test; > DROP DATABASE mysqltest1; > DROP USER 'mysqluser1'@'%'; > DROP USER 'mysqluser2'@'%'; > +# > +# MDEV-22374: VIEW with security definer require FILE privilege from definer > +# not invoker in case of INTO OUTFILE > +# > +create user test@localhost; > +grant select on test.* to test@localhost; > +create table t1 (a int); > +create definer=test@localhost sql security definer view v1 as select * from t1; > +# check that ot works without view typo "it" > +select * INTO OUTFILE '../..//tmp/test_out_txt' from t1; > +# check that ot works without file same why "//" ? > +select * from v1; > +a > +# rights for file should be taken from current user not view > +select * INTO OUTFILE '../../tmp/test_out_txt' from (select count(*) from v1) as dv1; > +select * INTO OUTFILE '../..//tmp/test_out_txt' from (select * from v1) as dv1; > +select * INTO OUTFILE '../..//tmp/test_out_txt' from v1; same > +drop view v1; please add reverse tests too: +create sql security definer view v1 as select * from t1; +connect(con1, localhost, test); +error 1045; +select * INTO OUTFILE '../..//tmp/test_out_txt' from t1; +select * from v1; +error 1045; +select * INTO OUTFILE '../../tmp/test_out_txt' from (select count(*) from v1) as dv1; +error 1045; +select * INTO OUTFILE '../..//tmp/test_out_txt' from (select * from v1) as dv1; +error 1045; +select * INTO OUTFILE '../..//tmp/test_out_txt' from v1; +connection default; > +drop table t1; > +drop user test@localhost; > +# End of 5.5 tests > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index ae5a6b4cd35..515990c879f 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -2206,15 +2206,15 @@ mysql_execute_command(THD *thd) > lex->exchange != NULL implies SELECT .. INTO OUTFILE and this > requires FILE_ACL access. > */ > - ulong privileges_requested= lex->exchange ? SELECT_ACL | FILE_ACL : > - SELECT_ACL; > - > - if (all_tables) > - res= check_table_access(thd, > - privileges_requested, > - all_tables, FALSE, UINT_MAX, FALSE); > - else > - res= check_access(thd, privileges_requested, any_db, NULL, NULL, 0, 0); > + if (lex->exchange) > + res= check_access(thd, FILE_ACL, any_db, NULL, NULL, 0, 0); > + if (!res) > + { > + if (all_tables) > + res= check_table_access(thd, SELECT_ACL, all_tables, FALSE, UINT_MAX, FALSE); > + else > + res= check_access(thd, SELECT_ACL, any_db, NULL, NULL, 0, 0); > + } No, I think this special check for a special SELECT ... INTO OUTFILE case is wrong. Why does the server even checks global privileges via views? It should not do it, I think, not for FILE not for any other global or db level privilege. I'd try something like diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -4699,7 +4699,7 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST > Save a copy of the privileges without the SHOW_VIEW_ACL attribute. It will be checked during making view. */ - tl->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL); + tl->grant.orig_want_privilege= want_access & ~SHOW_VIEW_ACL & TABLE_ACLS; } mysql_rwlock_rdlock(&LOCK_grant); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -5204,7 +5204,8 @@ check_table_access(THD *thd, ulong requirements,TABLE_LIS> Register access for view underlying table. Remove SHOW_VIEW_ACL, because it will be checked during making view */ - table_ref->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL); + table_ref->grant.orig_want_privilege= + want_access & ~SHOW_VIEW_ACL & TABLE_ACLS; if (table_ref->schema_table_reformed) { Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 3ead2cea95c: MDEV-13266: Race condition in ANALYZE TABLE / statistics collection
by Sergey Petrunia 04 May '20

04 May '20

Hi Varun, On Sun, Apr 12, 2020 at 09:08:37PM +0530, Varun wrote: > revision-id: 3ead2cea95c32b7ceaf6e6ec81f7afbd9137cfe9 (mariadb-10.2.31-103-g3ead2cea95c) > parent(s): d565895bbd8e2a163be48b9bac51fccbf3949c80 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2020-04-12 21:05:36 +0530 > message: > > MDEV-13266: Race condition in ANALYZE TABLE / statistics collection > > Fixing a race condition while collecting the engine independent statistics. > The issue here was when the statistics was collected on specific indexes > then because of some race condition the statistics for indexes was not > collected. The TABLE::keys_in_use_for_query was set to 0 in such cases. > > This happens when the table is opened from TABLE_SHARE instead of the > table share stored in table_cache. It would be nice to write down the race condition that we've caught (in case we need this info later) Is my understanding correct: the error scenario is as follows: Thread1> start running "ANALYZE TABLE t PERISTENT FOR COLUMNS (..) INDEXES ($list)" Thread1> Walk through $list and save it in TABLE::keys_in_use_for_query Thread1> Close/re-open tables Thread2> Make some use of table t. This involves taking table t from Thread2> the table cache, and putting it back (with TABLE::keys_in_use_for_query reset to 0) Thread1> continue collecting EITS stats. Since TABLE::keys_in_use_for_query we will not collect statistics for indexes in $list. Please confirm (and if not, describe the race condition). The patch also introduces this behaviour: analyze table ... persistent for ... indexes(no_such_index); will now cause engine statistics to be still collected. Before the patch it exited with an error. But then, this is consistent with what happens for analyze table ... persistent for columns(no_such_column) ... So I guess this is ok. > --- > sql/sql_admin.cc | 46 +++++++++++++++++++++------------------------- > 1 file changed, 21 insertions(+), 25 deletions(-) > > diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc > index ab95fdc340c..0613495f202 100644 > --- a/sql/sql_admin.cc > +++ b/sql/sql_admin.cc > @@ -769,31 +769,6 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, > (table->table->s->table_category == TABLE_CATEGORY_USER && > (get_use_stat_tables_mode(thd) > NEVER || > lex->with_persistent_for_clause)); > - > - > - if (!lex->index_list) > - { > - tab->keys_in_use_for_query.init(tab->s->keys); > - } > - else > - { > - int pos; > - LEX_STRING *index_name; > - List_iterator_fast<LEX_STRING> it(*lex->index_list); > - > - tab->keys_in_use_for_query.clear_all(); > - while ((index_name= it++)) > - { > - if (tab->s->keynames.type_names == 0 || > - (pos= find_type(&tab->s->keynames, index_name->str, > - index_name->length, 1)) <= 0) > - { > - compl_result_code= result_code= HA_ADMIN_INVALID; > - break; > - } > - tab->keys_in_use_for_query.set_bit(--pos); > - } > - } > } > > if (result_code == HA_ADMIN_OK) > @@ -878,6 +853,27 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, > } > tab->file->column_bitmaps_signal(); > } > + if (!lex->index_list) > + tab->keys_in_use_for_query.init(tab->s->keys); > + else > + { > + int pos; > + LEX_STRING *index_name; > + List_iterator_fast<LEX_STRING> it(*lex->index_list); > + > + tab->keys_in_use_for_query.clear_all(); > + while ((index_name= it++)) > + { > + if (tab->s->keynames.type_names == 0 || > + (pos= find_type(&tab->s->keynames, index_name->str, > + index_name->length, 1)) <= 0) > + { > + compl_result_code= result_code= HA_ADMIN_INVALID; > + break; > + } > + tab->keys_in_use_for_query.set_bit(--pos); > + } > + } > if (!(compl_result_code= > alloc_statistics_for_table(thd, table->table)) && > !(compl_result_code= > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 2