maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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/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@xxxxxxxxxxx
-- 
Vote for my Percona Live 2016 talks:
https://www.percona.com/live/data-performance-conference-2016/sessions/mariadb-connectors-fast-and-smart-new-protocol-optimizations#community-voting
https://www.percona.com/live/data-performance-conference-2016/sessions/mariadb-101-security-validation-authentication-encryption#community-voting