- developers - lists.mariadb.org

Re: [Maria-developers] MDEV-8615: Assertion `m_cpp_buf <= begin_ptr && begin_ptr <= m_cpp_buf + m_buf_length' failed in Lex_input_stream::body_utf8_start
by Sergei Golubchik 21 Jan '16

21 Jan '16

Hi, Sanja! On Dec 25, OleksandrByelkin wrote: > revision-id: 684bd68c33284f6106551188d9567f35b18593ed (mariadb-10.1.9-21-g684bd68) > parent(s): 59fcd7ff2315d007045eb987da5f21abbea6f6f1 > committer: Oleksandr Byelkin > timestamp: 2015-12-25 18:39:43 +0100 > message: > > MDEV-8615: Assertion `m_cpp_buf <= begin_ptr && begin_ptr <= m_cpp_buf + m_buf_length' failed in Lex_input_stream::body_utf8_start > > Nothing should be done before any keyword … [View More]

2 1

[Maria-developers] MDEV-8930 main.func_hybrid_type with --embedded --ps-protocol: Assertion `(longlong) thd->status_var.local_memory_used >= 0' failed
by Alexander Barkov 20 Jan '16

20 Jan '16

Hi Sergei, IIRC you fixed this bug during the meeting in Amsterdam. Can you please confirm? (or just close it, if so) Thanks.

1 0

[Maria-developers] Request for the project
by Ayush Bhargav 20 Jan '16

20 Jan '16

Hi. I am studying programming in college and I wanted to have some hand on experience on larger projects. I know c++ and mysql well. Please can you provide me a project on which I can start working. Thanks. - Ayush Bhargav

2 1

Re: [Maria-developers] [Commits] cae1945: MDEV-9117: Client Server capability negotiation for MariaDB specific functionality
by Sergei Golubchik 20 Jan '16

20 Jan '16

Hi, Sanja! On Nov 25, OleksandrByelkin wrote: > revision-id: cae19452eec1d3be47fa04759dd2dda6d061543e (mariadb-10.1.8-69-gcae1945) > parent(s): 55e67c3e344317c6f349f5391e5d117ec51ae062 > committer: Oleksandr Byelkin > timestamp: 2015-11-25 18:09:39 +0100 > message: > > MDEV-9117: Client Server capability negotiation for MariaDB specific functionality > > New capability flags space. > Removed old progress flag, added new one. > > diff --git a/include/… [View More]mysql_com.h b/include/mysql_com.h > index aa6ab0f..57092dc 100644 > --- a/include/mysql_com.h > +++ b/include/mysql_com.h > @@ -215,7 +216,8 @@ enum enum_server_command > /* Don't close the connection for a connection with expired password. */ > #define CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS (1UL << 22) > > -#define CLIENT_PROGRESS (1UL << 29) /* Client support progress indicator */ > +#define CLIENT_PROGRESS_OBSOLETE (1UL << 29) ok > +#define CLIENT_PROGRESS 0 why is that? > #define CLIENT_SSL_VERIFY_SERVER_CERT (1UL << 30) > /* > It used to be that if mysql_real_connect() failed, it would delete any > @@ -228,6 +230,13 @@ enum enum_server_command > */ > #define CLIENT_REMEMBER_OPTIONS (1UL << 31) > > +/* MariaDB extended capability flags */ > +#define MARIADB_CLIENT_FLAGS_MASK 0xffffffff00000000ULL > +/* Client support progress indicator */ > +#define MARIADB_CLIENT_PROGRESS (1ULL << 32) > +/* Client support mariaDB extended flags */ > +#define MARIADB_CLIENT_EXTENDED_FLAGS (1ULL << 63) how can this work, if client_flag and server_capabilities are only 32-bit variables? > + > #ifdef HAVE_COMPRESS > #define CAN_CLIENT_COMPRESS CLIENT_COMPRESS > #else > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 1240d5d..d07e0a4 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -11729,18 +11731,38 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio, > */ > DBUG_ASSERT(net->read_pos[pkt_len] == 0); > > - ulong client_capabilities= uint2korr(net->read_pos); > + ulonglong client_capabilities= uint2korr(net->read_pos); > + compile_time_assert(sizeof(client_capabilities) >= 8); > if (client_capabilities & CLIENT_PROTOCOL_41) > { > - if (pkt_len < 4) > + if (pkt_len < 32) > return packet_error; > client_capabilities|= ((ulong) uint2korr(net->read_pos+2)) << 16; > + if (!(client_capabilities & CLIENT_MYSQL)) > + { > + // it is client with mariadb extensions > + client_capabilities|= CLIENT_MYSQL; > + ulonglong ext_client_capabilities= > + (((ulonglong)uint4korr(net->read_pos + 28)) << 32); > + if (ext_client_capabilities & MARIADB_CLIENT_EXTENDED_FLAGS) > + client_capabilities|= ext_client_capabilities; > + else > + { > + DBUG_PRINT("error", ("CLIENT_PROTOCOL_41: on, " > + "CLIENT_LONG_PASSWORD/CLIENT_MYSQL off, " > + "but MARIADB_CLIENT_EXTENDED_FLAGS is off. " > + "flags: %llx ext flags %llx", > + client_capabilities, ext_client_capabilities)); > + return packet_error; Why do you need that (MARIADB_CLIENT_EXTENDED_FLAGS and the check)? > + } > + } here you need to check for old MariaDB clients - they will have CLIENT_MYSQL and might have CLIENT_PROGRESS_OBSOLETE - do something like else if (client_capabilities & CLIENT_PROGRESS_OBSOLETE) { client_capabilities |= MARIADB_CLIENT_PROGRESS; } > } > > diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c > index a1a52e8..31fca0f 100644 > --- a/tests/mysql_client_test.c > +++ b/tests/mysql_client_test.c > @@ -15336,7 +15336,7 @@ static void test_mysql_insert_id() > > myheader("test_mysql_insert_id"); > > - rc= mysql_query(mysql, "drop table if exists t1"); > + rc= mysql_query(mysql, "drop table if exists t1,t2"); separate commit, please > myquery(rc); > /* table without auto_increment column */ > rc= mysql_query(mysql, "create table t1 (f1 int, f2 varchar(255), key(f1))"); > @@ -18700,7 +18700,8 @@ static void test_progress_reporting() > myheader("test_progress_reporting"); > > conn= client_connect(CLIENT_PROGRESS, MYSQL_PROTOCOL_TCP, 0); > - DIE_UNLESS(conn->client_flag & CLIENT_PROGRESS); > + if (!(conn->client_flag & CLIENT_PROGRESS)) > + return; what are you testing here? Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org -- Vote for my Percona Live 2016 talks: https://www.percona.com/live/data-performance-conference-2016/sessions/mari… https://www.percona.com/live/data-performance-conference-2016/sessions/mari… [View Less]

2 8

[Maria-developers] Contribution in MariaDB
by Sanjana Garg 18 Jan '16

18 Jan '16

Hi, I am interested in contributing to the MariaDB project. Can someone guide me on how to get started. I am proficient in C, C++, python and SQL. Thanks Sanjana Garg

2 1

[Maria-developers] Projects to contribute
by Sonali Ellath 18 Jan '16

18 Jan '16

Hi Everyone, I was looking to contribute to MariaDb for GSoC 2016, I have successfully built MariaDb from source. What should be my next step? Should I pick a project and get it done? I am proficient in C++ programming, which ones would be suitable for me? Thanks! - Sonali

2 1

[Maria-developers] MDEV-8789: fully qualified field name resolving problem
by Oleksandr Byelkin 14 Jan '16

14 Jan '16

Hi! I decided to show my finding as soon as I have them found (to give time to explain/fix, also I think that name resolving fix could possibly cause some big rewriting) but if you prefer "all togather" do not hesitate to say it. So the test suite which demostrate the problem: --------- create table t1(a int, b int); insert into t1 values (1,1),(2,2),(10,10),(20,20); create table t2(c int, d int); insert into t2 values (10,1),(20,2); # mentioning "full path" to field works here select * … [View More]

1 3

Re: [Maria-developers] For review https://mariadb.atlassian.net/browse/MDEV-9058
by Oleksandr Byelkin 13 Jan '16

13 Jan '16

On 12.01.2016 13:09, Michael Widenius wrote: >>>>>> "Oleksandr" == Oleksandr Byelkin <sanja(a)montyprogram.com> writes: > Oleksandr> Hi! > Oleksandr> Here is full diff of the task (will be tested more when georg made it in > Oleksandr> the connectors tree) > > Oleksandr> Also it can be reviewed by parts in bb-sanja-10.2 branch on github. > > Oleksandr> Connectors part is on 10.2-georg in connectors tree. > > > diff --git a/… [View More]include/mysql_com.h b/include/mysql_com.h > index 57092dc..425559f 100644 > --- a/include/mysql_com.h > +++ b/include/mysql_com.h > @@ -102,7 +102,9 @@ enum enum_server_command > COM_STMT_PREPARE, COM_STMT_EXECUTE, COM_STMT_SEND_LONG_DATA, COM_STMT_CLOSE, > COM_STMT_RESET, COM_SET_OPTION, COM_STMT_FETCH, COM_DAEMON, > /* don't forget to update const char *command_name[] in sql_parse.cc */ > - > + COM_MDB_GAP_BEG, > + COM_MDB_GAP_END=254, > + COM_MULTI, > /* Must be last */ > COM_END > }; > > Change COM_MDB_GAP_END to be 253, to ensure that COM_END is 255. > (Better to keep everything in one byte) > > Also add an compiler error check that COM_END is 255 (to ensure that one > doesn't accidently add a new define without changing COM_DB_GAP_END. OK! > > @@ -234,6 +236,8 @@ enum enum_server_command > #define MARIADB_CLIENT_FLAGS_MASK 0xffffffff00000000ULL > > Haven't seen the above define before and it's not in the current 10.1 > code. ah, it was removed wit client code, so I'll remove it if it is not used. > > /* Client support progress indicator */ > #define MARIADB_CLIENT_PROGRESS (1ULL << 32) > +/* support COM_MULTI */ > +#define MARIADB_CLIENT_COM_MULTI (1ULL << 33) > /* Client support mariaDB extended flags */ > #define MARIADB_CLIENT_EXTENDED_FLAGS (1ULL << 63) > > @@ -268,7 +272,8 @@ enum enum_server_command > MARIADB_CLIENT_PROGRESS | \ > CLIENT_PLUGIN_AUTH | \ > CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA | \ > - CLIENT_CONNECT_ATTRS) > + CLIENT_CONNECT_ATTRS |\ > + MARIADB_CLIENT_COM_MULTI) > > You don't have MARIADB_CLIENT_COM_MULTI anywhere else in the patch. > Is that intententional ? > > If yes, you should add a comment that this is used in the LGPL C > driver code so that anyone examening the server code will not get > confused. > > As you have added flags bigger than 32, you should also change > THD::client_capabilities from ulong to ulonglong to reflect this! > > This patch doesn't include changes in sql_acl.cc to send the > MARIADB_CLIENT_COM_MULTI flag to the client. > Where are the necessary changes for this ? expanding in to 64bit was separate task (reviewed and pushed). Also we are not going to support this new features in old client, so we are signaling our connector only. I'll write a comment about it. > > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index b496e4c..54884c0 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -856,6 +856,7 @@ THD::THD(bool is_wsrep_applier) > /* statement id */ 0), > rli_fake(0), rgi_fake(0), rgi_slave(NULL), > protocol_text(this), protocol_binary(this), > + last_stmt(NULL), > in_sub_stmt(0), log_all_errors(0), > binlog_unsafe_warning_flags(0), > binlog_table_maps(0), > > You don't have to set last_stmt(NULL) in THD::THD() as this function > calls init() which sets this too. ok > > @@ -1424,6 +1425,7 @@ void THD::init(void) > bzero((char *) &org_status_var, sizeof(org_status_var)); > start_bytes_received= 0; > last_commit_gtid.seq_no= 0; > + last_stmt= NULL; > status_in_global= 0; > #ifdef WITH_WSREP > wsrep_exec_mode= wsrep_applier ? REPL_RECV : LOCAL_STATE; > > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > @@ -884,6 +1115,23 @@ void cleanup_items(Item *item) > DBUG_VOID_RETURN; > } > > +static enum enum_server_command fetch_command(THD *thd, char *packet) > +{ > + enum enum_server_command > + command= (enum enum_server_command) (uchar) packet[0]; > + NET *net= &thd->net; > + DBUG_ENTER("fetch_command"); > + > > If command is 256, then the following first test can never be true and > you may get a warning from gcc. Another reason to have COM_END as 255. yes > > + if (command >= COM_END || > + (command >= COM_MDB_GAP_BEG && command <= COM_MDB_GAP_END)) > + command= COM_END; // Wrong command > + > + DBUG_PRINT("info",("Command on %s = %d (%s)", > + vio_description(net->vio), command, > + command_name[command].str)); > + DBUG_RETURN(command); > +} > + > > /** > + check COM_MULTI packet > + > + @param thd thread handle > + @param packet pointer on the packet of commands > + @param packet_length length of this packet > + > + @retval TRUE - Error > + @retval FALSE - OK > +*/ > + > +bool maria_multi_check(THD *thd, char *packet, uint packet_length) > +{ > + DBUG_ENTER("maria_multi_check"); > + while (packet_length) > + { > > You need to check for packet_length >= 3 and give the > my_message(ER_UNKNOWN_COM_ERROR... error if packet_length > 1 and <= 3 > > > + // length of command + 3 bytes where that length was stored > + uint subpacket_length= (uint3korr(packet) + 3); > + DBUG_PRINT("info", ("sub-packet length: %d command: %x", > + subpacket_length, packet[3])); > + > + if (subpacket_length == 0 || > + subpacket_length > packet_length) > > Remove check of sub_packet_length == 0; It can never be 0 as you are > adding +3 above. ok > > + { > + my_message(ER_UNKNOWN_COM_ERROR, ER_THD(thd, ER_UNKNOWN_COM_ERROR), > + MYF(0)); > + DBUG_RETURN(TRUE); > + } > + > + packet+= subpacket_length; > + packet_length-= subpacket_length; > + } > > + DBUG_RETURN(FALSE); > +} > > > @@ -1901,6 +2200,69 @@ bool dispatch_command(enum enum_server_command command, THD *thd, > general_log_print(thd, command, NullS); > my_eof(thd); > break; > + case COM_MULTI: > + if (is_com_multi) > + { > + /* we do not allow deep recursion because it is not safe */ > + my_message(ER_UNKNOWN_COM_ERROR, ER_THD(thd, ER_UNKNOWN_COM_ERROR), > + MYF(0)); > + break; > + } > > Give another error as the above one is missleading. OK > > + if (!(thd->client_capabilities & CLIENT_MULTI_RESULTS)) > + { > + /* The client does not support multiple result sets being sent back */ > + my_error(ER_COMMULTI_BADCONTEXT, MYF(0)); > + break; > + } > > How can the above be possible ? > We should not allow the client to set COM_MULTI flag if they don't > have CLIENT_MULTI_RESULT. > Better to remove the above check and document when the client can set the > COM_MULTI flag. I am not of control of client, it can be some home-made client so better to check. > > + if (maria_multi_check(thd, packet, packet_length)) > + break; > + { > + /* We have to store next length because it will be destroyed by '\0' */ > + uint next_subpacket_length= uint3korr(packet); > + unsigned char *readbuff= net->buff; > + unsigned long readbuff_max_packet= net->max_packet; > + > > Remove the above one extra empty line. > Add also some documentation why we are changing net->buff here instead > of creating another buffer and using this instead. > > Actually it's much better to create a new buffer for packet becasue > of: > > - The buffer only need to be of size packet_length, not > net->max_packet which can be much longer. > - Clearer code as you don't have to manipulate net internal structures > or restore these. > - The disadvantage is that you can't allow COM_CHANGE_USER, but I > don't think one should be able to do that anyway as this may ask for > more information from the client. OK > > > + if (!(net->buff=(uchar*) my_malloc((size_t) net->max_packet+ > + NET_HEADER_SIZE + COMP_HEADER_SIZE +1, > + MYF(MY_WME)))) > + break; > + net->buff_end=net->buff + net->max_packet; > + net->write_pos=net->read_pos = net->buff; > + > + while(packet_length) > > space after while > > + { > + uint subpacket_length= next_subpacket_length; > > Why not do as in other code and make the rest of the code easier: > subpacket_length= next_subpacket_length +3 ; ok > > + if (subpacket_length + 3 < packet_length) > + next_subpacket_length= uint3korr(packet + subpacket_length + 3); > + /* safety like in do_command() */ > + packet[subpacket_length + 3]= '\0'; > + > + enum enum_server_command subcommand= fetch_command(thd, (packet + 3)); > + > > I would almost prefer that we check for sub_commend == COM_MULTI here > instead of at start of COM_COMMAND: This would be more logical as we > may not allow all commands here (like change user). OK > > + if (is_com_multi) > + { > + /* we do not allow deep recursion because it is not safe */ > + my_message(ER_UNKNOWN_COM_ERROR, ER_THD(thd, ER_UNKNOWN_COM_ERROR), > + MYF(0)); > + break; > + } > > > + if (dispatch_command(subcommand, thd, packet + 4, > + subpacket_length - 1, TRUE)) > > Add an assert that is_error() is set if we do a break. > (As otherwise you can miss to execute commands the client expects) OK > > + break; > + > + DBUG_ASSERT(subpacket_length + 3 <= packet_length); > + packet+= (subpacket_length + 3); > + packet_length-= (subpacket_length + 3); > > + } > > I don't think you need a net_flush here as the packet is flushed by > every command the sent a result. OK > > + net_flush(net); > + my_free(net->buff); > + net->buff= readbuff; > + net->max_packet= readbuff_max_packet; > + net->buff_end=net->buff + net->max_packet; > + net->write_pos=net->read_pos = net->buff; > + > > If we use an internal buffer, you can remoev all of the above (except > the free) OK > > + if (!thd->is_error()) > + { > + thd->get_stmt_da()->reset_diagnostics_area(); > + my_ok(thd); > + } > + } > + break; > > case COM_SLEEP: > case COM_CONNECT: // Impossible here > case COM_TIME: // Impossible from client > @@ -1940,6 +2302,8 @@ bool dispatch_command(enum enum_server_command command, THD *thd, > thd->protocol->end_statement(); > query_cache_end_of_result(thd); > } > + if (drop_more_results) > + thd->server_status&= ~SERVER_MORE_RESULTS_EXISTS; > > Another way, is to just remember the status of server_status and do: > > thd->server_status= ((thd->server_status & ~SERVER_MORE_RESULTS_EXISTS) | > (org_server_status & SERVER_MORE_RESULTS_EXISTS)); > > Easier to understand than drop_more_results I am not sure that execution can't legally change setver status (if not now then in future) > > --- a/sql/sql_prepare.cc > +++ b/sql/sql_prepare.cc > @@ -324,7 +324,9 @@ find_prepared_statement(THD *thd, ulong id) > prepared statements find() will return 0 if there is a named prepared > statement with such id. > */ > - Statement *stmt= thd->stmt_map.find(id); > + Statement *stmt= ((id == LAST_STMT_ID) ? > + thd->last_stmt : > + thd->stmt_map.find(id)); > > Please add a comment when id can be LAST_STMT_ID (clear to me but not > obivous for others) > > > if (stmt == 0 || stmt->type() != Query_arena::PREPARED_STATEMENT) > return NULL; > > --- a/storage/perfschema/pfs.cc > +++ b/storage/perfschema/pfs.cc > @@ -1444,19 +1444,21 @@ static void register_statement_v1(const char *category, > > for (; count>0; count--, info++) > { > - DBUG_ASSERT(info->m_name != NULL); > - len= strlen(info->m_name); > - full_length= prefix_length + len; > - if (likely(full_length <= PFS_MAX_INFO_NAME_LENGTH)) > + if(info->m_name != NULL) > > Add space after if > > Instead of doing an if, and cause a big diff, do instead: > > if (!info->m_name) > continue; > > This will remove all of the following in the diff: OK > > > { > - memcpy(formatted_name + prefix_length, info->m_name, len); > - info->m_key= register_statement_class(formatted_name, full_length, info->m_flags); > - } > - else > - { > - pfs_print_error("register_statement_v1: name too long <%s>\n", > - info->m_name); > - info->m_key= 0; > + len= strlen(info->m_name); > + full_length= prefix_length + len; > + if (likely(full_length <= PFS_MAX_INFO_NAME_LENGTH)) > + { > + memcpy(formatted_name + prefix_length, info->m_name, len); > + info->m_key= register_statement_class(formatted_name, full_length, info->m_flags); > + } > + else > + { > + pfs_print_error("register_statement_v1: name too long <%s>\n", > + info->m_name); > + info->m_key= 0; > + } > } > } > return; > > Regards, > Monty [View Less]

1 1

[Maria-developers] MDEV-4691 MariaDB Kerberos/GSSAPI
by Sergei Golubchik 12 Jan '16

12 Jan '16

Hi, Wlad! Here's a review, below. Just a few comments or questions. Great to have it done finally! :) > diff --git a/plugin/auth_gssapi/CMakeLists.txt b/plugin/auth_gssapi/CMakeLists.txt > new file mode 100644 > index 0000000..e20be70 > --- /dev/null > +++ b/plugin/auth_gssapi/CMakeLists.txt > @@ -0,0 +1,33 @@ > +IF (WIN32) > + SET(USE_SSPI 1) > +ENDIF() > + > +IF(USE_SSPI) > + SET(GSSAPI_LIBRARIES secur32) > + ADD_DEFINITIONS(-DPLUGIN_SSPI) > + … [View More]SET(GSSAPI_CLIENT sspi_client.cc) > + SET(GSSAPI_SERVER sspi_server.cc) > + SET(GSSAPI_ERRMSG sspi_errmsg.cc) > +ELSE() > + SET(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_CURRENT_SOURCE_DIR}/cmake) > + FIND_PACKAGE(GSSAPI) > + IF(GSSAPI_FOUND) > + INCLUDE_DIRECTORIES(${GSSAPI_INCS}) > + ADD_DEFINITIONS(-DPLUGIN_GSSAPI) > + SET(GSSAPI_CLIENT gssapi_client.cc) > + SET(GSSAPI_SERVER gssapi_server.cc) > + SET(GSSAPI_ERRMSG gssapi_errmsg.cc) > + ELSE() > + # Can't build plugin > + RETURN() > + ENDIF() > +ENDIF () > + > + > +MYSQL_ADD_PLUGIN(auth_gssapi server_plugin.cc ${GSSAPI_SERVER} ${GSSAPI_ERRMSG} > + LINK_LIBRARIES ${GSSAPI_LIBS} > + MODULE_ONLY) please, specify a component here and below. Like "COMPONENT Kerberos" (or gssapi, whatever). It would mean that this plugin will be in a separate rpm package - it must be, because it brings additional dependencies that the server itself does not need. > + > +MYSQL_ADD_PLUGIN(auth_gssapi_client client_plugin.cc ${GSSAPI_CLIENT} ${GSSAPI_ERRMSG} > + LINK_LIBRARIES ${GSSAPI_LIBS} > + MODULE_ONLY) same here. I'm not sure whether it should be the same component as above, or a different one. also, I'm not sure whether plugin.cmake will do a correct client plugin rpm, if you'll have a different component here. > diff --git a/plugin/auth_gssapi/README.md b/plugin/auth_gssapi/README.md > new file mode 100644 > index 0000000..f20128f > --- /dev/null > +++ b/plugin/auth_gssapi/README.md > @@ -0,0 +1,129 @@ > +# GSSAPI/SSPI authentication for MariaDB > + > +This article gives instructions on configuring GSSAPI authentication plugin > +for MariaDB for passwordless login. > + > +On Unix systems, GSSAPI is usually synonymous with Kerberos authentication. > +Windows has slightly different but very similar API called SSPI, that along with Kerberos, also supports NTLM authentication. > + > +This plugin includes support for Kerberos on Unix, but also can be used as for Windows authentication with or without domain > +environment. > + > +## Server-side preparations on Unix > +To use the plugin, some preparation need to be done on the server side on Unixes. > +MariaDB server will read need access to the Kerberos keytab file, that contains service principal name for the MariaDB server. s/read need/need read/ :) > + > + > +If you are using **Unix Kerberos KDC (MIT,Heimdal)** > + > +- Create service principal using kadmin tool > + > +``` > +kadmin -q "addprinc -randkey mariadb/host.domain.com" > +``` > + > +(replace host.domain.com with fully qualified DNS name for the server host) > + > +- Export the newly created user to the keytab file > + > +``` > +kadmin -q "ktadd -k /path/to/mariadb.keytab mariadb/host.domain.com" > +``` > + > +More details can be found [here](http://www.microhowto.info/howto/create_a_service_principal_using_mit_kerberos.html) > +and [here](http://www.microhowto.info/howto/add_a_host_or_service_principal_to_a_keytab_using_mit_kerberos.html) > + > +If you are using **Windows Active Directory KDC** > +you can need to create keytab using ktpass.exe tool on Windows, map principal user to an existing domain user like this > + > +``` > +ktpass.exe /princ mariadb/host.domain.com(a)DOMAIN.COM /mapuser someuser /pass MyPas$w0rd /out mariadb.keytab /crypto all /ptype KRB5_NT_PRINCIPAL /mapop set > +``` > + > +and then transfer the keytab file to the Unix server. See [Microsoft documentation](https://technet.microsoft.com/en-us/library/cc753771.aspx) for details. > + > + > +## Server side preparations on Windows. > +Usually nothing need to be done. MariaDB server should to run on a domain joined machine, either as NetworkService account > +(which is default if it runs as service) or run under any other domain account credentials. > +Creating service principal is not required here (but you can still do it using [_setspn_](https://technet.microsoft.com/en-us/library/cc731241.aspx) tool) > + > + > +# Installing plugin > +- Start the server > + > +- On Unix, edit my the my.cnf/my.ini configuration file, set the parameter gssapi-keytab-path to point to previously > +created keytab path. > + > +``` > + gssapi-keytab-path=/path/to/mariadb.keytab > +``` > + > +- Optionally on Unix, in case the service principal name differs from default mariadb/host.domain.com@REALM, > +configure alternative principal name with > + > +``` > + gssapi-principal-name=alternative/principalname@REALM > +``` > + > +- In mysql command line client, execute > + > +``` > + INSTALL SONAME 'auth_gssapi' > +``` > + > +#Creating users > + > +Now, you can create a user for GSSAPI/SSPI authentication. CREATE USER command, for Kerberos user > +would be like this (*long* form, see below for short one) > + > +``` > +CREATE USER usr1 IDENTIFIED WITH gssapi AS 'usr1(a)EXAMPLE.COM'; > +``` > + > +(replace with real username and realm) > + > +The part after AS is mechanism specific, and needs to be ``machine\\usr1`` for Windows users identified with NTLM. > + > +You may also use alternative *short* form of CREATE USER > + > +``` > +CREATE USER usr1 IDENTIFIED WITH gssapi; > +``` > + > +If this syntax is used, realm part is used for comparison realm part is NOT used for comparison > +thus 'usr1(a)EXAMPLE.COM', 'usr1(a)EXAMPLE.CO.UK' and 'mymachine\usr1' will all identify as 'usr1'. > + > +#Login as GSSAPI user with command line clients > + > +Using command line client, do > + > +``` > +mysql --plugin-dir=/path/to/plugin-dir -u usr1 > +``` > + > +#Plugin variables > +- **gssapi-keytab-path** (Unix only) - Path to the server keytab file > +- **gssapi-principal-name** - name of the service principal. > +- **gssapi-mech-name** (Windows only) - Name of the SSPI package used by server. Can be either 'Kerberos' or 'Negotiate'. > + Defaults to 'Negotiate' (both Kerberos and NTLM users can connect) > + Set it to 'Kerberos', to prevent less secure NTLM in domain environments, but leave it as default(Negotiate) > + to allow non-domain environment (e.g if server does not run in domain enviroment). > + > + > +#Implementation > + > +Overview of the protocol between client and server > + > +1. Server : Construct gssapi-principal-name if not set in my.cnf. On Unixes defaults to hostbased name for service "mariadb". On Windows to user's or machine's domain names. > +Acquire credentials for gssapi-principal-name with ```gss_acquire_cred() / AcquireSecurityCredentials()```. > +Send packet with principal name and mech ```"gssapi-principal-name\0gssapi-mech-name\0"``` to client ( on Unix, empty string used for gssapi-mech) > + > +2. Client: execute ```gss_init_sec_context() / InitializeSecurityContext()``` passing gssapi-principal-name / gssapi-mech-name parameters. > +Send resulting GSSAPI blob to server. > + > +3. Server : receive blob from client, execute ```gss_accept_sec_context()/ AcceptSecurityContext()```, send resulting blob back to client > + > +4. Perform 2. and 3. can until both client and server decide that authentication is done, or until some error occured. If authentication was successful, GSSAPI context (an opaque structure) is generated on both client and server sides. > + > +5. Server : Client name is extracted from the context, and compared to the name provided by client(with or without realm). If name matches, plugin returns success. This is a very good documentation. But it also has to be in the KB (perhaps minus with "Implementation" part). You can put it there yourself or ask Ian to do it. There's a feature in KB that allows to "link" a KB page to a "Github flavored markup" document (meaning the markup document is the original and KB is a copy of it). Perhaps it can be useful here. > diff --git a/plugin/auth_gssapi/cmake/FindGSSAPI.cmake b/plugin/auth_gssapi/cmake/FindGSSAPI.cmake > new file mode 100644 > index 0000000..faee428 > --- /dev/null > +++ b/plugin/auth_gssapi/cmake/FindGSSAPI.cmake > @@ -0,0 +1,78 @@ > +# - Try to detect the GSSAPI support > +# Once done this will define > +# > +# GSSAPI_FOUND - system supports GSSAPI > +# GSSAPI_INCS - the GSSAPI include directory > +# GSSAPI_LIBS - the libraries needed to use GSSAPI > +# GSSAPI_FLAVOR - the type of API - MIT or HEIMDAL > + > +# Copyright (c) 2006, Pino Toscano, <toscano.pino(a)tiscali.it> > +# > +# Redistribution and use is allowed according to the terms of the BSD license. > +# For details see the accompanying COPYING-CMAKE-SCRIPTS file. There's no accompanying COPYING-CMAKE-SCRIPTS file, as far as I can see. You either need to add the license in this file or take this COPYING-CMAKE-SCRIPTS too. I'd copied the complete license into this file, I think. > + > + > +if(GSSAPI_LIBS AND GSSAPI_FLAVOR) > + > + # in cache already > + set(GSSAPI_FOUND TRUE) > + > +else(GSSAPI_LIBS AND GSSAPI_FLAVOR) > + > + find_program(KRB5_CONFIG NAMES krb5-config PATHS > + /opt/local/bin > + ONLY_CMAKE_FIND_ROOT_PATH # this is required when cross compiling with cmake 2.6 and ignored with cmake 2.4, Alex > + ) > + mark_as_advanced(KRB5_CONFIG) > + > + #reset vars > + set(GSSAPI_INCS) > + set(GSSAPI_LIBS) > + set(GSSAPI_FLAVOR) > + > + if(KRB5_CONFIG) > + > + set(HAVE_KRB5_GSSAPI TRUE) > + exec_program(${KRB5_CONFIG} ARGS --libs gssapi RETURN_VALUE _return_VALUE OUTPUT_VARIABLE GSSAPI_LIBS) > + if(_return_VALUE) > + message(STATUS "GSSAPI configure check failed.") > + set(HAVE_KRB5_GSSAPI FALSE) > + endif(_return_VALUE) > + > + exec_program(${KRB5_CONFIG} ARGS --cflags gssapi RETURN_VALUE _return_VALUE OUTPUT_VARIABLE GSSAPI_INCS) > + string(REGEX REPLACE "(\r?\n)+$" "" GSSAPI_INCS "${GSSAPI_INCS}") > + string(REGEX REPLACE " *-I" ";" GSSAPI_INCS "${GSSAPI_INCS}") > + > + exec_program(${KRB5_CONFIG} ARGS --vendor RETURN_VALUE _return_VALUE OUTPUT_VARIABLE gssapi_flavor_tmp) > + set(GSSAPI_FLAVOR_MIT) > + if(gssapi_flavor_tmp MATCHES ".*Massachusetts.*") > + set(GSSAPI_FLAVOR "MIT") > + else(gssapi_flavor_tmp MATCHES ".*Massachusetts.*") > + set(GSSAPI_FLAVOR "HEIMDAL") > + endif(gssapi_flavor_tmp MATCHES ".*Massachusetts.*") > + > + if(NOT HAVE_KRB5_GSSAPI) > + if (gssapi_flavor_tmp MATCHES "Sun Microsystems.*") > + message(STATUS "Solaris Kerberos does not have GSSAPI; this is normal.") > + set(GSSAPI_LIBS) > + set(GSSAPI_INCS) > + else(gssapi_flavor_tmp MATCHES "Sun Microsystems.*") > + message(WARNING "${KRB5_CONFIG} failed unexpectedly.") > + endif(gssapi_flavor_tmp MATCHES "Sun Microsystems.*") > + endif(NOT HAVE_KRB5_GSSAPI) > + > + if(GSSAPI_LIBS) # GSSAPI_INCS can be also empty, so don't rely on that > + set(GSSAPI_FOUND TRUE CACHE STRING "") > + message(STATUS "Found GSSAPI: ${GSSAPI_LIBS}") > + > + set(GSSAPI_INCS ${GSSAPI_INCS} CACHE STRING "") > + set(GSSAPI_LIBS ${GSSAPI_LIBS} CACHE STRING "") > + set(GSSAPI_FLAVOR ${GSSAPI_FLAVOR} CACHE STRING "") > + > + mark_as_advanced(GSSAPI_INCS GSSAPI_LIBS GSSAPI_FLAVOR) > + > + endif(GSSAPI_LIBS) > + > + endif(KRB5_CONFIG) > + > +endif(GSSAPI_LIBS AND GSSAPI_FLAVOR) > diff --git a/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.pm b/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.pm > new file mode 100644 > index 0000000..3ffc6f1 > --- /dev/null > +++ b/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.pm > @@ -0,0 +1,47 @@ > +package My::Suite::AuthGSSAPI; > + > +@ISA = qw(My::Suite); > + > +return "No AUTH_GSSAPI plugin" unless $ENV{AUTH_GSSAPI_SO}; > + > +return "Not run for embedded server" if $::opt_embedded_server; > + > +# Following environment variables may need to be set > +if ($^O eq "MSWin32") > +{ > + chomp(my $whoami =`whoami /UPN 2>NUL` || `whoami`); don't you need to check whether the command worked and whether it returned something valid? > + my $fullname = $whoami; > + $fullname =~ s/\\/\\\\/; # SQL escaping for backslash > + $ENV{'GSSAPI_FULLNAME'} = $fullname; > + $ENV{'GSSAPI_SHORTNAME'} = $ENV{'USERNAME'}; > +} > +else > +{ > + if (!$ENV{'GSSAPI_FULLNAME'}) > + { > + my $s = `klist |grep 'Default principal: '`; > + if ($s) > + { > + chomp($s); > + my $fullname = substr($s,19); > + $ENV{'GSSAPI_FULLNAME'} = $fullname; > + } > + } > + $ENV{'GSSAPI_SHORTNAME'} = (split /@/, $ENV{'GSSAPI_FULLNAME'}) [0]; > +} > + > + > +if (!$ENV{'GSSAPI_FULLNAME'} || !$ENV{'GSSAPI_SHORTNAME'}) > +{ > + return "Environment variable GSSAPI_SHORTNAME and GSSAPI_FULLNAME need to be set" > +} > + > +foreach $var ('GSSAPI_SHORTNAME','GSSAPI_FULLNAME','GSSAPI_KEYTAB_PATH','GSSAPI_PRINCIPAL_NAME') > +{ > + print "$var=$ENV{$var}\n"; Really? It would make mtr runs a bit too verbose. at least put it inside if ($::opt_verbose) { ... } > +} > + > +sub is_default { 1 } > + > +bless { }; > + > diff --git a/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.opt b/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.opt > new file mode 100644 > index 0000000..3077d70 > --- /dev/null > +++ b/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.opt > @@ -0,0 +1 @@ > +--loose-gssapi-keytab-path=$GSSAPI_KEYTAB_PATH --loose-gssapi-principal-name=$GSSAPI_PRINCIPAL_NAME 1. who sets GSSAPI_KEYTAB_PATH and GSSAPI_PRINCIPAL_NAME? 2. if the user, do you need to check in suite.pm whether they're set? > diff --git a/plugin/auth_gssapi/mysql-test/auth_gssapi/basic.result b/plugin/auth_gssapi/mysql-test/auth_gssapi/basic.result > new file mode 100644 > index 0000000..a859ce5 > --- /dev/null > +++ b/plugin/auth_gssapi/mysql-test/auth_gssapi/basic.result > @@ -0,0 +1,18 @@ > +INSTALL SONAME 'auth_gssapi'; > +CREATE USER GSSAPI_SHORTNAME IDENTIFIED WITH gssapi; > +SELECT USER(),CURRENT_USER(); > +USER() CURRENT_USER() > +GSSAPI_SHORTNAME@localhost GSSAPI_SHORTNAME@% > +DROP USER GSSAPI_SHORTNAME; > +CREATE USER nosuchuser IDENTIFIED WITH gssapi; > +ERROR 28000: GSSAPI name mismatch, requested 'nosuchuser', actual name 'GSSAPI_SHORTNAME' What does the real unmasked error message look like? Is it something that client can know without connecting to the server? If not - this user-friendly verbose error is an information disclosure security vulnerability > +DROP USER nosuchuser; > +CREATE USER usr1 IDENTIFIED WITH gssapi as 'GSSAPI_FULLNAME'; > +SELECT USER(),CURRENT_USER(); > +USER() CURRENT_USER() > +usr1@localhost usr1@% > +DROP USER usr1; > +CREATE USER nosuchuser IDENTIFIED WITH gssapi AS 'nosuchuser(a)EXAMPLE.COM'; > +ERROR 28000: GSSAPI name mismatch, requested 'nosuchuser(a)EXAMPLE.COM', actual name 'GSSAPI_FULLNAME' > +DROP USER nosuchuser; > +UNINSTALL SONAME 'auth_gssapi'; > diff --git a/plugin/auth_gssapi/server_plugin.cc b/plugin/auth_gssapi/server_plugin.cc > new file mode 100644 > index 0000000..64f52a3 > --- /dev/null > +++ b/plugin/auth_gssapi/server_plugin.cc > @@ -0,0 +1,175 @@ > +/* Copyright (c) 2015, Shuang Qiu, Robbie Hardwood, > + Vladislav Vaintroub & MariaDB Corporation > + > + All rights reserved. > + > + Redistribution and use in source and binary forms, with or without > + modification, are permitted provided that the following conditions are met: > + > + 1. Redistributions of source code must retain the above copyright notice, > + this list of conditions and the following disclaimer. > + > + 2. Redistributions in binary form must reproduce the above copyright notice, > + this list of conditions and the following disclaimer in the documentation > + and/or other materials provided with the distribution. > + > + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" > + AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE > + ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE > + LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > + CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > + SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS > + INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > + ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE > + POSSIBILITY OF SUCH DAMAGE. 2-clause BSD. Was it Robbie who asked for it? > +*/ > + > +/** > + @file > + > + GSSAPI authentication plugin, server side > +*/ > +#include <my_sys.h> > +#include <mysqld_error.h> > +#include <mysql/plugin_auth.h> > +#include "server_plugin.h" > +#include "common.h" > + > +/* First packet sent from server to client, contains srv_principal_name\0mech\0 */ > +static char first_packet[PRINCIPAL_NAME_MAX + MECH_NAME_MAX +2]; > +static int first_packet_len; > + > +/* > + Target name in GSSAPI/SSPI , for Kerberos it is service principal name > + (often user principal name of the server user will work) > +*/ > +char *srv_principal_name; > +char *srv_keytab_path; > +char *srv_mech_name=(char *)""; > +unsigned long srv_mech; > + > +/** > + The main server function of the GSSAPI plugin. > + */ > +static int gssapi_auth(MYSQL_PLUGIN_VIO *vio, MYSQL_SERVER_AUTH_INFO *auth_info) > +{ > + int use_full_name; > + const char *user; > + int user_len; > + > + /* Send first packet with target name and mech name */ > + if (vio->write_packet(vio, (unsigned char *)first_packet, first_packet_len)) > + { > + return CR_ERROR; > + } > + > + /* Figure out whether to use full name (as given in IDENTIFIED AS clause) > + * or just short username auth_string > + */ > + if (auth_info->auth_string_length > 0) Hmm, I wonder how that can possibly work. auth_info->auth_string can only be set when the user is known. And the user is known only after the first read... ... ok ... got it. the first packet always uses the default, mysql_native_password, plugin so when the server switches to gssapi plugin, the user name is already known. Still, in case we'll have a --default-auth-plugin server option, I'd suggest to check auth_info->auth_string_length only after the first vio->read_packet() > + { > + use_full_name= 1; > + user= auth_info->auth_string; > + user_len= auth_info->auth_string_length; > + } > + else > + { > + use_full_name= 0; > + user= auth_info->user_name; > + user_len= auth_info->user_name_length; > + } > + > + return auth_server(vio, user, user_len, use_full_name); > +} > + > +static int initialize_plugin(void *unused) > +{ > + int rc; > + rc = plugin_init(); > + if (rc) > + return rc; > + > + strcpy(first_packet, srv_principal_name); > + strcpy(first_packet + strlen(srv_principal_name) + 1,srv_mech_name); > + first_packet_len = strlen(srv_principal_name) + strlen(srv_mech_name) + 2; in the server code I'd use strmov here, like first_packet_len = strmov(strmov(first_packet, srv_principal_name) + 1, srv_mech_name) - first_packet + 1; (may be with intermediate variables, for readability). I understand that a plugin might not want to do it, to avoid depending on server code outside of the plugin API. But if that's the case, why do you include my_sys.h? And mysqld_error.h? > + > + return 0; > +} > + > +static int deinitialize_plugin(void *unused) > +{ > + return plugin_deinit(); > +} > + > +/* system variable */ > +static MYSQL_SYSVAR_STR(keytab_path, srv_keytab_path, > + PLUGIN_VAR_RQCMDARG|PLUGIN_VAR_READONLY, > + "Keytab file path (Kerberos)", > + NULL, > + NULL, > + ""); > +static MYSQL_SYSVAR_STR(principal_name, srv_principal_name, > + PLUGIN_VAR_RQCMDARG|PLUGIN_VAR_READONLY, > + "GSSAPI target name - service principal name for Kerberos authentication.", A bit inconsistent. I'd suggest either to change the first to Keytab file path for Kerberos authentication or the second to GSSAPI target name - service principal name (Kerberos) > + NULL, > + NULL, > + ""); > +#ifdef PLUGIN_SSPI > +static const char* mech_names[] = { > + "Kerberos", > + "Negotiate", > + "", > + NULL > +}; > +static TYPELIB mech_name_typelib = { > + array_elements(mech_names) - 1, > + "mech_name_typelib", > + mech_names, > + NULL > +}; > +static MYSQL_SYSVAR_ENUM(mech_name, srv_mech, > + PLUGIN_VAR_RQCMDARG|PLUGIN_VAR_READONLY, > + "GSSAPI mechanism : either Kerberos or Negotiate", You don't need "either Kerberos or Negotiate" here, simply "GSSAPI mechanism" is enough. On --help, my_getopt automatically will add ", one of: Kerberos, Negotiate". Which is consistent with other ENUM options. It only makes sense to list values explicitly, when the list contains additional information, like "GSSAPI mechanism : either Kerberos or Negotiate (automatically choose between Kerberos and NTLM)." > + NULL, > + NULL, > + 2,&mech_name_typelib); > +#endif > + > +static struct st_mysql_sys_var *system_variables[]= { > + MYSQL_SYSVAR(principal_name), > +#ifdef PLUGIN_SSPI > + MYSQL_SYSVAR(mech_name), > +#endif > +#ifdef PLUGIN_GSSAPI > + MYSQL_SYSVAR(keytab_path), > +#endif > + NULL > +}; > + > +/* Register authentication plugin */ > +static struct st_mysql_auth server_handler= { > + MYSQL_AUTHENTICATION_INTERFACE_VERSION, > + "auth_gssapi_client", > + gssapi_auth > +}; > + > +maria_declare_plugin(gssapi_server) > +{ > + MYSQL_AUTHENTICATION_PLUGIN, > + &server_handler, > + "gssapi", > + "Shuang Qiu, Robbie Harwood, Vladislav Vaintroub", > + "Plugin for GSSAPI/SSPI based authentication.", > + PLUGIN_LICENSE_BSD, > + initialize_plugin, > + deinitialize_plugin, /* destructor */ > + 0x0100, /* version */ > + NULL, /* status variables */ > + system_variables, /* system variables */ > + "1.0", > + MariaDB_PLUGIN_MATURITY_EXPERIMENTAL I'd say BETA > +} > +maria_declare_plugin_end; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org -- Vote for my Percona Live 2016 talks: https://www.percona.com/live/data-performance-conference-2016/sessions/mari… https://www.percona.com/live/data-performance-conference-2016/sessions/mari… [View Less]

2 2

[Maria-developers] Please review MDEV-8092 Change Create_field::field from "const char *" to LEX_CSTRING
by Alexander Barkov 11 Jan '16

11 Jan '16

Hi Serg, Please review a patch for MDEV-8092. Thanks. Perhaps something interesting, about #3 in the task description. I added a debug output and collected some statistics for "mtr --suite=main", to check how these changes can potentially affect another task: MDEV-7954 my_strcasecmp_utf8() takes 0.19% in OLTP RO Here are the results: my_strcasecmp_utf8() is now called 18,709,905 times total during "mtr --suite=main" 9,200,498 of those are called to compare Field/Create_field names in … [View More]

3 10