cleanup: ssl handling in the internal rpl client

* type of mi->ssl_verify_server_cert must be my_bool, because it's passed by address to mysql_options(), and the latter expects my_bool * explicitly disable ssl in MYSQL if mi->ssl is 0 * remove dead code (`#ifdef NOT_USED`) * remove useless casts and checks replacing empty strings with NULL (new_VioSSLFd() does that internally)
MariaDB · Feb 4, 2024 · f4e174e · f4e174e
1 parent e951edd
commit f4e174e
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 124 deletions.
diff --git a/mysql-test/suite/rpl/r/rpl_ssl1.result b/mysql-test/suite/rpl/r/rpl_ssl1.result
@@ -16,7 +16,11 @@ select * from t1;
 t
 include/wait_for_slave_io_error.inc [errno=1045]
 include/stop_slave_sql.inc
-change master to master_ssl=1 , master_ssl_ca ='MYSQL_TEST_DIR/std_data/cacert.pem', master_ssl_cert='MYSQL_TEST_DIR/std_data/client-cert.pem', master_ssl_key='MYSQL_TEST_DIR/std_data/client-key.pem';
+change master to
+master_ssl=1,
+master_ssl_ca ='MYSQL_TEST_DIR/std_data/cacert.pem',
+master_ssl_cert='MYSQL_TEST_DIR/std_data/client-cert.pem',
+master_ssl_key='MYSQL_TEST_DIR/std_data/client-key.pem';
 start slave;
 include/wait_for_slave_to_start.inc
 connection master;
@@ -61,7 +65,6 @@ connection master;
 create table t1 (t int);
 insert into t1 values (1);
 connection slave;
-on slave
 select * from t1;
 t
 1

diff --git a/mysql-test/suite/rpl/t/rpl_ssl1.test b/mysql-test/suite/rpl/t/rpl_ssl1.test
@@ -34,7 +34,11 @@ select * from t1;
 --source include/stop_slave_sql.inc
 
 --replace_result $MYSQL_TEST_DIR MYSQL_TEST_DIR
-eval change master to master_ssl=1 , master_ssl_ca ='$MYSQL_TEST_DIR/std_data/cacert.pem', master_ssl_cert='$MYSQL_TEST_DIR/std_data/client-cert.pem', master_ssl_key='$MYSQL_TEST_DIR/std_data/client-key.pem';
+eval change master to
+  master_ssl=1,
+  master_ssl_ca ='$MYSQL_TEST_DIR/std_data/cacert.pem',
+  master_ssl_cert='$MYSQL_TEST_DIR/std_data/client-cert.pem',
+  master_ssl_key='$MYSQL_TEST_DIR/std_data/client-key.pem';
 start slave;
 --source include/wait_for_slave_to_start.inc
 
@@ -85,9 +89,6 @@ create table t1 (t int);
 insert into t1 values (1);
 
 sync_slave_with_master;
-
-echo on slave;
-#checking that replication is ok
 select * from t1;
 
 #checking show slave status

diff --git a/sql/replication.h b/sql/replication.h
@@ -480,27 +480,6 @@ int register_binlog_relay_io_observer(Binlog_relay_IO_observer *observer, void *
 */
 int unregister_binlog_relay_io_observer(Binlog_relay_IO_observer *observer, void *p);
 
-/**
-   Connect to master
-
-   This function can only used in the slave I/O thread context, and
-   will use the same master information to do the connection.
-
-   @code
-   MYSQL *mysql = mysql_init(NULL);
-   if (rpl_connect_master(mysql))
-   {
-     // do stuff with the connection
-   }
-   mysql_close(mysql); // close the connection
-   @endcode
-   
-   @param mysql address of MYSQL structure to use, pass NULL will
-   create a new one
-
-   @return address of MYSQL structure on success, NULL on failure
-*/
-MYSQL *rpl_connect_master(MYSQL *mysql);
 
 /**
    Get the value of user variable as an integer.
@@ -535,8 +514,7 @@ int get_user_var_int(const char *name,
    @retval 0 Success
    @retval 1 Variable not found
 */
-int get_user_var_real(const char *name,
-                      double *value, int *null_value);
+int get_user_var_real(const char *name, double *value, int *null_value);
 
 /**
    Get the value of user variable as a string.

diff --git a/sql/rpl_mi.cc b/sql/rpl_mi.cc
@@ -31,7 +31,7 @@ static void init_master_log_pos(Master_info* mi);
 Master_info::Master_info(LEX_CSTRING *connection_name_arg,
                          bool is_slave_recovery)
   :Slave_reporting_capability("I/O"),
-   ssl(0), ssl_verify_server_cert(1), fd(-1), io_thd(0), 
+   ssl(0), ssl_verify_server_cert(0), fd(-1), io_thd(0),
    rli(is_slave_recovery), port(MYSQL_PORT),
    checksum_alg_before_fd(BINLOG_CHECKSUM_ALG_UNDEF),
    connect_retry(DEFAULT_CONNECT_RETRY), inited(0), abort_slave(0),
@@ -219,8 +219,6 @@ void init_master_log_pos(Master_info* mi)
   mi->gtid_reconnect_event_skip_count= 0;
   mi->gtid_event_seen= false;
 
-  /* Intentionally init ssl_verify_server_cert to 0, no option available  */
-  mi->ssl_verify_server_cert= 0;
   /* 
     always request heartbeat unless master_heartbeat_period is set
     explicitly zero.  Here is the default value for heartbeat period

diff --git a/sql/rpl_mi.h b/sql/rpl_mi.h
@@ -231,7 +231,7 @@ class Master_info : public Slave_reporting_capability
   char ssl_ca[FN_REFLEN], ssl_capath[FN_REFLEN], ssl_cert[FN_REFLEN];
   char ssl_cipher[FN_REFLEN], ssl_key[FN_REFLEN];
   char ssl_crl[FN_REFLEN], ssl_crlpath[FN_REFLEN];
-  bool ssl_verify_server_cert;
+  my_bool ssl_verify_server_cert; /* MUST be my_bool, see mysql_option() */
 
   my_off_t master_log_pos;
   File fd; // we keep the file open, so we need to remember the file pointer

diff --git a/sql/slave.cc b/sql/slave.cc
@@ -7173,28 +7173,23 @@ static int connect_to_master(THD* thd, MYSQL* mysql, Master_info* mi,
   if (opt_slave_compressed_protocol)
     client_flag|= CLIENT_COMPRESS;                /* We will use compression */
 
-  mysql_options(mysql, MYSQL_OPT_CONNECT_TIMEOUT, (char *) &slave_net_timeout);
-  mysql_options(mysql, MYSQL_OPT_READ_TIMEOUT, (char *) &slave_net_timeout);
-  mysql_options(mysql, MYSQL_OPT_USE_THREAD_SPECIFIC_MEMORY,
-                (char*) &my_true);
+  mysql_options(mysql, MYSQL_OPT_CONNECT_TIMEOUT, &slave_net_timeout);
+  mysql_options(mysql, MYSQL_OPT_READ_TIMEOUT, &slave_net_timeout);
+  mysql_options(mysql, MYSQL_OPT_USE_THREAD_SPECIFIC_MEMORY, &my_true);
 
 #ifdef HAVE_OPENSSL
   if (mi->ssl)
   {
-    mysql_ssl_set(mysql,
-                  mi->ssl_key[0]?mi->ssl_key:0,
-                  mi->ssl_cert[0]?mi->ssl_cert:0,
-                  mi->ssl_ca[0]?mi->ssl_ca:0,
-                  mi->ssl_capath[0]?mi->ssl_capath:0,
-                  mi->ssl_cipher[0]?mi->ssl_cipher:0);
-    mysql_options(mysql, MYSQL_OPT_SSL_CRL,
-                  mi->ssl_crl[0] ? mi->ssl_crl : 0);
-    mysql_options(mysql, MYSQL_OPT_SSL_CRLPATH,
-                  mi->ssl_crlpath[0] ? mi->ssl_crlpath : 0);
+    mysql_ssl_set(mysql, mi->ssl_key, mi->ssl_cert, mi->ssl_ca, mi->ssl_capath,
+                  mi->ssl_cipher);
+    mysql_options(mysql, MYSQL_OPT_SSL_CRL, mi->ssl_crl);
+    mysql_options(mysql, MYSQL_OPT_SSL_CRLPATH, mi->ssl_crlpath);
     mysql_options(mysql, MYSQL_OPT_SSL_VERIFY_SERVER_CERT,
                   &mi->ssl_verify_server_cert);
   }
+  else
 #endif
+    mysql->options.use_ssl= 0;
 
   /*
     If server's default charset is not supported (like utf16, utf32) as client
@@ -7214,7 +7209,7 @@ static int connect_to_master(THD* thd, MYSQL* mysql, Master_info* mi,
   }
 
   /* This one is not strictly needed but we have it here for completeness */
-  mysql_options(mysql, MYSQL_SET_CHARSET_DIR, (char *) charsets_dir);
+  mysql_options(mysql, MYSQL_SET_CHARSET_DIR, charsets_dir);
 
   /* Set MYSQL_PLUGIN_DIR in case master asks for an external authentication plugin */
   if (opt_plugin_dir_ptr && *opt_plugin_dir_ptr)
@@ -7307,80 +7302,6 @@ static int safe_reconnect(THD* thd, MYSQL* mysql, Master_info* mi,
 }
 
 
-#ifdef NOT_USED
-MYSQL *rpl_connect_master(MYSQL *mysql)
-{
-  Master_info *mi= my_pthread_getspecific_ptr(Master_info*, RPL_MASTER_INFO);
-  bool allocated= false;
-  my_bool my_true= 1;
-  THD *thd;
-
-  if (!mi)
-  {
-    sql_print_error("'rpl_connect_master' must be called in slave I/O thread context.");
-    return NULL;
-  }
-  thd= mi->io_thd;
-  if (!mysql)
-  {
-    if(!(mysql= mysql_init(NULL)))
-    {
-      sql_print_error("rpl_connect_master: failed in mysql_init()");
-      return NULL;
-    }
-    allocated= true;
-  }
-
-  /*
-    XXX: copied from connect_to_master, this function should not
-    change the slave status, so we cannot use connect_to_master
-    directly
-    
-    TODO: make this part a seperate function to eliminate duplication
-  */
-  mysql_options(mysql, MYSQL_OPT_CONNECT_TIMEOUT, (char *) &slave_net_timeout);
-  mysql_options(mysql, MYSQL_OPT_READ_TIMEOUT, (char *) &slave_net_timeout);
-  mysql_options(mysql, MYSQL_OPT_USE_THREAD_SPECIFIC_MEMORY,
-                (char*) &my_true);
-
-#ifdef HAVE_OPENSSL
-  if (mi->ssl)
-  {
-    mysql_ssl_set(mysql,
-                  mi->ssl_key[0]?mi->ssl_key:0,
-                  mi->ssl_cert[0]?mi->ssl_cert:0,
-                  mi->ssl_ca[0]?mi->ssl_ca:0,
-                  mi->ssl_capath[0]?mi->ssl_capath:0,
-                  mi->ssl_cipher[0]?mi->ssl_cipher:0);
-    mysql_options(mysql, MYSQL_OPT_SSL_VERIFY_SERVER_CERT,
-                  &mi->ssl_verify_server_cert);
-  }
-#endif
-
-  mysql_options(mysql, MYSQL_SET_CHARSET_NAME,
-                default_charset_info->cs_name.str);
-  /* This one is not strictly needed but we have it here for completeness */
-  mysql_options(mysql, MYSQL_SET_CHARSET_DIR, (char *) charsets_dir);
-
-  if (mi->user == NULL
-      || mi->user[0] == 0
-      || io_slave_killed( mi)
-      || !mysql_real_connect(mysql, mi->host, mi->user, mi->password, 0,
-                             mi->port, 0, 0))
-  {
-    if (!io_slave_killed( mi))
-      sql_print_error("rpl_connect_master: error connecting to master: %s (server_error: %d)",
-                      mysql_error(mysql), mysql_errno(mysql));
-
-    if (allocated)
-      mysql_close(mysql);                       // this will free the object
-    return NULL;
-  }
-  return mysql;
-}
-#endif
-
-
 /*
   Called when we notice that the current "hot" log got rotated under our feet.
 */
@@ -7392,8 +7313,8 @@ static IO_CACHE *reopen_relay_log(Relay_log_info *rli, const char **errmsg)
   DBUG_ASSERT(rli->cur_log_fd == -1);
 
   IO_CACHE *cur_log = rli->cur_log=&rli->cache_buf;
-  if ((rli->cur_log_fd=open_binlog(cur_log,rli->event_relay_log_name,
-                                   errmsg)) <0)
+  rli->cur_log_fd= open_binlog(cur_log,rli->event_relay_log_name, errmsg);
+  if (rli->cur_log_fd <0)
     DBUG_RETURN(0);
   /*
     We want to start exactly where we was before:

diff --git a/vio/viosslfactories.c b/vio/viosslfactories.c
@@ -323,6 +323,7 @@ new_VioSSLFd(const char *key_file, const char *cert_file, const char *ca_file,
   fix_value(ca_path);
   fix_value(crl_file);
   fix_value(crl_path);
+  fix_value(cipher);
 
   DBUG_PRINT("enter",
              ("key_file: '%s'  cert_file: '%s'  ca_file: '%s'  ca_path: '%s'  "