October 2024 - developers - lists.mariadb.org

Community request: audit the PARSEC plugin
by Andrew Hutchings 16 Oct '24

16 Oct '24

Hi all, There is a new authentication plugin called PARSEC that is set to become the default authentication plugin in MariaDB soon (MDEV-32618 and MDEV-12320). The current documentation for it can be found here: https://mariadb.com/kb/en/authentication-plugin-parsec/ I personally think that using a single salt and sending it to the client could be a problem. I do not work in the security industry and the documentation has a few errors in it and pieces missing, so I'm unclear how certain things work. I think the ext-salt requirements will likely add complications to load balancing scenarios and replication. This is a request for anyone in the community to please audit this plugin before it becomes the default. Once it is the default, it will be difficult to change it. Kind Regards -- Andrew (LinuxJedi) Hutchings Chief Contributions Officer MariaDB Foundation

2 2

Re: 778c67f7af0: MDEV-18151 Skipped error returning for GRANT/SET PASSWORD
by Sergei Golubchik 09 Oct '24

09 Oct '24

Hi, Oleksandr, On Oct 09, Oleksandr Byelkin wrote: > revision-id: 778c67f7af0 (mariadb-10.5.26-154-g778c67f7af0) > parent(s): e9c999caf48 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2024-10-04 09:28:46 +0200 > message: > > MDEV-18151 Skipped error returning for GRANT/SET PASSWORD > > Make error issueing for GRANT and SET PASSWORD the same. typo. "issuing" > Report errors wich were skipped before. > > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -2370,6 +2369,21 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user, > > res= 0; > end: > + switch(res) > + { > + case ER_OUTOFMEMORY: // should be reported by my_malloc > + case ER_NOT_VALID_PASSWORD: // should be reported by plugin > + case 0: > + break; > + case ER_SET_PASSWORD_AUTH_PLUGIN: > + my_error(res, MYF(0), plugin_name); > + break; > + case ER_PASSWD_LENGTH: > + my_error(ER_PASSWD_LENGTH, MYF(0), SCRAMBLED_PASSWORD_CHAR_LENGTH); this is also reported by the plugin. The caller doesn't know what the correct length is, it's not SCRAMBLED_PASSWORD_CHAR_LENGTH, this value is different for different plugins. > + break; > + default: > + DBUG_ASSERT(0); > + } > if (unlock_plugin) > plugin_unlock(thd, plugin); > return res; > Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

Re: e8021aaf28e: auth: refactor MYSQL_AUTHENTICATION_INTERFACE_VERSION naming
by Sergei Golubchik 07 Oct '24

07 Oct '24

Hi, Nikita, This looks very confusing, this is not how versions work. What are you trying to achieve by that? On Oct 07, Nikita Malyavin wrote: > revision-id: e8021aaf28e (mariadb-11.6.1-9-ge8021aaf28e) > parent(s): a5b80531fbb > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2024-10-04 00:16:17 +0200 > message: > > auth: refactor MYSQL_AUTHENTICATION_INTERFACE_VERSION naming > > Rename MYSQL_AUTHENTICATION_INTERFACE_VERSION to > MYSQL_AUTH_INTERFACE_VERSION_2_02. > This is the current latest version, and it will be fixed to all auth plugins > for current implementation. > MYSQL_AUTHENTICATION_INTERFACE_VERSION will stay for compatibility with 3-rd > party auth plugins and will be fixed to V. 2.02. > Add MYSQL_AUTHENTICATION_INTERFACE_VERSION_LAST that will be updated as needed. > Add MYSQL_AUTH_INTERFACE_MAJOR_MASK to identify major version. > Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 4

Re: b17308e254a: MDEV-34854 Parsec sends garbage when using an empty password
by Sergei Golubchik 07 Oct '24

07 Oct '24

Hi, Nikita, On Oct 07, Nikita Malyavin wrote: > revision-id: b17308e254a (mariadb-11.6.1-10-gb17308e254a) > parent(s): e8021aaf28e > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2024-10-04 01:33:47 +0200 > message: > > MDEV-34854 Parsec sends garbage when using an empty password > > When an empty password is set, the server doesn't call > st_mysql_auth::hash_password and leaves > MYSQL_SERVER_AUTH_INFO::auth_string empty. > > Fix: > generate hashes for empty passwords as well. This breaks some auth > plugins, so we increment interface version and do it only from Auth V. > 2.03. > > Some empty passwords could be already stored with no though. The user "with no though" ? > will have to call SET PASSWORD once again, anyway the authentication > wouldn't have worked for such password. ok, I presume you mean ed25519 only, because mysql_native_password worked with an empty password and it generates an empty hash for it. > diff --git a/mysql-test/suite/plugins/r/parsec.result b/mysql-test/suite/plugins/r/parsec.result > index 512c066e2d7..b7e3537af29 100644 > --- a/mysql-test/suite/plugins/r/parsec.result > +++ b/mysql-test/suite/plugins/r/parsec.result ... let's add ed25519 test too, for completeness. > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 2722ea2ea19..ba05a5656c5 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -2402,7 +2402,10 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user, > res= ER_NOT_VALID_PASSWORD; > goto end; > } > - if (pwtext.length) > + > + // Starting from version 2.03 we also generate hash for empty passwords. > + if ((info->interface_version >= MYSQL_AUTH_INTERFACE_VERSION_2_03 I don't understand this MYSQL_AUTH_INTERFACE_VERSION_2_03 thing. First, again, that's not how a version is supposed to work. Second, this empty-password change isn't a change in the API. You can simply start calling hash_password() for empty passwords and it'll just work. I've tried :) > + && pwtext.str) || pwtext.length) > { > if (info->hash_password) > { Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 2

PRs with one approval pending the second
by Otto Kekäläinen 04 Oct '24

04 Oct '24

Hi *all* developers, Hopefully someone else than Andrew or Sergei replies as a project of the size of MariaDB should have multiple developers who are capable of reviewing PRs, and not overload a few. These PRs already have one approval, but would need a second to be merged. If you feel you are knowledgeable in the domain of the PRs, please help out by reviewing them. Thanks! https://github.com/MariaDB/server/pull/3225: MDEV-29955: add ${PCRE_LIBRARY_PATH} to link directories https://github.com/MariaDB/server/pull/3540: Add MTRs to further test coverage around TLS functionality

1 0

PRs with two approvals but not merged
by Otto Kekäläinen 04 Oct '24

04 Oct '24

Hi, Here are two PRs that have already two approvals, yet they have not been merged for many weeks. Can you please state explicitly on the PR as a comment what the submitter needs to do, or if there is nothing to do, just merge it? * https://github.com/MariaDB/server/pull/3208 * https://github.com/MariaDB/server/pull/2556 Thanks

3 3

More extensive major version upgrade testing
by Otto Kekäläinen 04 Oct '24

04 Oct '24

Hi, If any dev here is interested in the topic of ensuring the major version upgrades (and essentially running mariadb-upgrade) continues to work flawlessly and regressions are less likely to surface unnoticed, please review and consider adding your approvals to https://github.com/MariaDB/server/pull/3503 and share feedback on WIP https://github.com/MariaDB/server/pull/3563. Thanks!

2 1

Re: [PATCH] MDEV-28413 System versioning is incorrect in Spider
by Yuchen Pei 03 Oct '24

03 Oct '24

Hi Aleksey, Thanks for the patch. Please see below for some initial review comments. There are some todos for myself noted below which I will look into tomorrow. > From b4465fc6b51dfc4e645569201e6026ec6405367b Mon Sep 17 00:00:00 2001 > From: Aleksey Midenkov <midenok(a)gmail.com> > Date: Mon, 30 Sep 2024 22:26:51 +0300 > Subject: [PATCH] MDEV-28413 System versioning is incorrect in Spider > Basic functioning of system versioning in Spider: UPDATE, DELETE, > DELETE HISTORY, AS OF, FROM .. TO, BETWEEN .. AND. > More testing should be done for: > LOAD DATA, joins, derived tables, multi-update/delete and what else? > Note that we are still using HA_CAN_DIRECT_UPDATE_AND_DELETE, but not > for periods in DELETE (fwiw there is condition > !table_list->has_period(), but it is missing in UDPATE). > --- > .../mysql-test/spider/r/versioning.result | 142 ++++++++++++++++ > .../mysql-test/spider/t/versioning.test | 151 ++++++++++++++++++ > storage/spider/spd_db_conn.cc | 19 ++- > storage/spider/spd_db_include.h | 1 + > storage/spider/spd_db_mysql.cc | 25 ++- > 5 files changed, 335 insertions(+), 3 deletions(-) > create mode 100644 storage/spider/mysql-test/spider/r/versioning.result > create mode 100644 storage/spider/mysql-test/spider/t/versioning.test > [... 149 lines elided] > diff --git a/storage/spider/mysql-test/spider/t/versioning.test b/storage/spider/mysql-test/spider/t/versioning.test > new file mode 100644 > index 00000000000..6093169bff4 > --- /dev/null > +++ b/storage/spider/mysql-test/spider/t/versioning.test > @@ -0,0 +1,151 @@ > +--source include/have_innodb.inc > +--source suite/versioning/common.inc > + > +install plugin spider soname 'ha_spider'; For init the spider plugin in spider tests we commonly do --disable_query_log --disable_result_log --source ../t/test_init.inc --enable_result_log --enable_query_log > +set spider_same_server_link= on; > + > +--replace_result $MASTER_1_MYPORT MASTER_1_MYPORT > +eval create server s foreign data wrapper mysql options > +(host '127.0.0.1', database 'test', user 'root', port $MASTER_1_MYPORT); Use evalp instead of eval and you can save the --replace_result line > + > + > +create table t1 ( > + x int, y int > +) with system versioning; > + > +create table t2 ( > + x int, y int, > + row_start timestamp(6) as row start invisible, > + row_end timestamp(6) as row end invisible, > + period for system_time (row_start, row_end) > +) with system versioning; > + > +create or replace table t1_sp ( > + x int, y int) > +engine=spider comment='wrapper "mysql", srv "s", table "t1"' > +with system versioning; > + > +create or replace table t2_sp ( > + x int, y int, > + row_start timestamp(6) as row start invisible, > + row_end timestamp(6) as row end invisible, > + period for system_time (row_start, row_end)) > +engine=spider comment='wrapper "mysql", srv "s", table "t2"' > +with system versioning; > + > +--echo # Timestamps are not propagated (subject of MDEV-16546, but not for implicit system fields) > +set timestamp= unix_timestamp('2000-01-01 00:00:00'); I tried executing directly using select spider_direct_sql('set timestamp= unix_timestamp("2000-01-01 00:00:00");', '', 'srv "s"'); But then I get extra rows in subsequent select queries and I think it is caused by the timezones. It could be a separate task to fix this, but we should warn users about it. > +insert into t1_sp values (1, 1), (2, 2); It may be obvious to you, but I find the timestamps at times hard to keep track of, so I suggest adding a comment to each DML statement indicating the time, as well as the select row_start / row_end statements. For example, change the line above to insert into t1_sp values (1, 1), (2, 2); # T1 > +insert into t2_sp values (1, 1), (2, 2); and change this line to insert into t2_sp values (1, 1), (2, 2); # T2 > +--sleep 0.01 > + > +set timestamp= unix_timestamp('2000-01-01 00:11:11'); > +--echo # check dml generating history > +delete from t1_sp where x = 1; add # T3 > +--sleep 0.01 > +delete from t2_sp where x = 1; # T4 > +--sleep 0.01 > + > +select row_end into @t1 from t2 for system_time all where x = 1; And so we can annotate this line with the timestamp above, like so: select row_end into @t1 from t2 for system_time all where x = 1; # @t1 = T4 > + > +update t1_sp set y= y + 1; # T5 > +select row_start into @t2 from t1 where x = 2; # @t2 = T5 > +--sleep 0.01 > +update t2_sp set y= y + 1; # T6 With the annotations I suggested, results from the following block of selects becomes easier to understand. > + > +select * from t1_sp; > +select * from t2_sp; > +select * from t1_sp for system_time as of timestamp @t1; > +select * from t2_sp for system_time as of timestamp @t1; > +select * from t1_sp for system_time as of timestamp @t2; > +select * from t2_sp for system_time as of timestamp @t2; > +select * from t1_sp for system_time from timestamp @t1 to timestamp @t2; > +select * from t2_sp for system_time from timestamp @t1 to timestamp @t2; > +select * from t1_sp for system_time between timestamp @t1 and timestamp @t2; > +select * from t2_sp for system_time between timestamp @t1 and timestamp @t2; > + > +--echo # spider cannot call functions, it fails in spider_create_group_by_handler() > + # at: > + # > + # 1555 if (spider_db_print_item_type(item, null, spider, null, null, 0, > + # 1556 roop_count, true, fields_arg)) > + # 1557 { > + # 1558 dbug_print("info",("spider dbton_id=%d can't create select", roop_count)); > + # 1559 spider_clear_bit(dbton_bitmap, roop_count); > + # 1560 keep_going = false; > + # 1561 break; > + # 1562 } > + # This is fine. The Spider group-by-handler (GBH) is not meant to handle every situation. Either remove this block starting from the --echo, or make it more succinct by replacing it with something like (Note that GBH supports certain functions like trim): # The spider group-by-handler does not support the check_row function > +--echo # but somehow it doesn't fail the query and continues without FOR SYSTEM_TIME ALL clause > +select *, check_row(row_start, row_end) from t1_sp for system_time all order by row_end, y, x; > +select *, check_row(row_start, row_end) from t2_sp for system_time all order by row_end, y, x; I will check why the execution without the group-by-handler results in wrong results. > +--echo # SELECT from base table works as usual: > +select *, check_row(row_start, row_end) from t1 for system_time all order by row_end, y, x; > +select *, check_row(row_start, row_end) from t2 for system_time all order by row_end, y, x; > + > +--echo # here it works nice (append_table_list() has the chance to add FOR SYSTEM_TIME ALL) > + > +# Usage: SHOW_TIMESTAMPS=1 mtr versioning.spider Shouldn't the test name be spider.versioning? > +if ($SHOW_TIMESTAMPS) > +{ > + select @t1, @t2; > +} > +if (!$SHOW_TIMESTAMPS) > +{ > +--replace_regex /\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d.\d\d\d\d\d\d/TIMESTAMP/ > +} > +select *, row_start, row_end from t1_sp for system_time all order by row_end, y, x; > + > +if (!$SHOW_TIMESTAMPS) > +{ > +--replace_regex /\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d.\d\d\d\d\d\d/TIMESTAMP/ > +} > +select *, row_start, row_end from t2_sp for system_time all order by row_end, y, x; > + > +delete history from t1_sp before system_time @t1; > +delete history from t2_sp before system_time @t1; > + > +if (!$SHOW_TIMESTAMPS) > +{ > +--replace_regex /\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d.\d\d\d\d\d\d/TIMESTAMP/ > +} > +select *, row_start, row_end from t1_sp for system_time all order by row_end, y, x; > + > +if (!$SHOW_TIMESTAMPS) > +{ > +--replace_regex /\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d.\d\d\d\d\d\d/TIMESTAMP/ > +} > +select *, row_start, row_end from t2_sp for system_time all order by row_end, y, x; > + > +select * from t1 for system_time all; > +select * from t2 for system_time all; > + > +delete history from t1_sp; > +delete history from t2_sp; > + > +if (!$SHOW_TIMESTAMPS) > +{ > +--replace_regex /\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d.\d\d\d\d\d\d/TIMESTAMP/ > +} > +select *, row_start, row_end from t1_sp for system_time all order by row_end, y, x; > + > +if (!$SHOW_TIMESTAMPS) > +{ > +--replace_regex /\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d.\d\d\d\d\d\d/TIMESTAMP/ > +} > +select *, row_start, row_end from t2_sp for system_time all order by row_end, y, x; > + > +select * from t1 for system_time all; > +select * from t2 for system_time all; > + > +drop tables t1, t2, t1_sp, t2_sp; > +set timestamp= default; > +--source suite/versioning/common_finish.inc > + > +uninstall plugin spider; > +drop table mysql.spider_link_failed_log, mysql.spider_link_mon_servers, mysql.spider_tables, mysql.spider_table_crd, mysql.spider_table_position_for_recovery, mysql.spider_table_sts, mysql.spider_xa, mysql.spider_xa_failed_log, mysql.spider_xa_member; > +drop function spider_direct_sql; > +drop function spider_bg_direct_sql; > +drop function spider_ping_table; > +drop function spider_copy_tables; > +drop function spider_flush_table_mon_cache; For deinit the spider plugin in spider tests we commonly do --disable_query_log --disable_result_log --source ../t/test_deinit.inc --enable_result_log --enable_query_log Please replace the lines from "uninstall plugin spider;" to the end of the test with this block. > diff --git a/storage/spider/spd_db_conn.cc b/storage/spider/spd_db_conn.cc > index 8750e87237e..896fce8adf5 100644 > --- a/storage/spider/spd_db_conn.cc > +++ b/storage/spider/spd_db_conn.cc > @@ -1515,9 +1515,8 @@ int spider_db_append_key_where_internal( > ) { > SPIDER_RESULT_LIST *result_list = &spider->result_list; > SPIDER_SHARE *share = spider->share; > -#ifndef DBUG_OFF > TABLE *table = spider->get_table(); > -#endif Why remove the #ifndef and #endif? > [... 99 lines elided] Best, Yuchen

1 1

[Maria-developers] MariaDB // Temporal tables
by Robert Palm 02 Oct '24

02 Oct '24

2 2