Fix for CONC-102:

Since we use one SSL context per library instance (which might be shared by several threads) we need to protect certification loading by a mutex.
2025-08-08 14:02:17 +03:00 · 2014-10-12 05:35:43 +02:00
parent a292115104
commit d12429bf99
3 changed files with 153 additions and 43 deletions
--- a/libmariadb/ma_secure.c
+++ b/libmariadb/ma_secure.c
@@ -32,9 +32,9 @@ static SSL_CTX *SSL_context= NULL;
 #define MAX_SSL_ERR_LEN 100

 extern pthread_mutex_t LOCK_ssl_config;
-static pthread_mutex_t *LOCK_crypto;
+static pthread_mutex_t *LOCK_crypto= NULL;

-/* 
+/*
 SSL error handling
 */
 static void my_SSL_error(MYSQL *mysql)
@@ -70,21 +70,41 @@ static void my_SSL_error(MYSQL *mysql)
   Crypto call back functions will be
   set during ssl_initialization
 */
-static unsigned long my_cb_threadid(void)
+static void my_cb_threadid(CRYPTO_THREADID *id)
 {
-  /* cast pthread_t to unsigned long */
-	return (unsigned long) pthread_self();
+  CRYPTO_THREADID_set_numeric(id, (unsigned long)pthread_self());
 }

-static void
-my_cb_locking(int mode, int n, const char *file, int line)
+static void my_cb_locking(int mode, int n, const char *file, int line)
 {
-	if (mode & CRYPTO_LOCK)
-		pthread_mutex_lock(&LOCK_crypto[n]);
-	else
-		pthread_mutex_unlock(&LOCK_crypto[n]);
+  if (mode & CRYPTO_LOCK)
+    pthread_mutex_lock(&LOCK_crypto[n]);
+  else
+    pthread_mutex_unlock(&LOCK_crypto[n]);
 }

+
+static int ssl_thread_init()
+{
+  int i, max= CRYPTO_num_locks();
+
+  if (LOCK_crypto == NULL)
+  {
+    if (!(LOCK_crypto= 
+          (pthread_mutex_t *)my_malloc(sizeof(pthread_mutex_t) * max, MYF(0))))
+      return 1;
+
+    for (i=0; i < max; i++)
+      pthread_mutex_init(&LOCK_crypto[i], NULL);
+  }
+
+  CRYPTO_THREADID_set_callback(my_cb_threadid);
+  CRYPTO_set_locking_callback(my_cb_locking);
+
+  return 0;
+}
+
+
 /*
  Initializes SSL and allocate global
  context SSL_context
@@ -103,30 +123,15 @@ int my_ssl_start(MYSQL *mysql)
  DBUG_ENTER("my_ssl_start");
  /* lock mutex to prevent multiple initialization */
  pthread_mutex_lock(&LOCK_ssl_config);
-
  if (!my_ssl_initialized)
  {
-    if (!(LOCK_crypto= 
-          (pthread_mutex_t *)my_malloc(sizeof(pthread_mutex_t) * 
-                                       CRYPTO_num_locks(), MYF(0))))
-    {
-      rc= 1;
+    if (ssl_thread_init())
      goto end;
-    } else
-    {
-      int i;
+    SSL_library_init();

-      for (i=0; i < CRYPTO_num_locks(); i++)
-        pthread_mutex_init(&LOCK_crypto[i], NULL);
-      CRYPTO_set_id_callback(my_cb_threadid);
-			CRYPTO_set_locking_callback(my_cb_locking);
-    }
 #if SSLEAY_VERSION_NUMBER >= 0x00907000L
    OPENSSL_config(NULL);
 #endif
-
-    /* always returns 1, so we can discard return code */
-    SSL_library_init();
    /* load errors */
    SSL_load_error_strings();
    /* digests and ciphers */
@@ -165,12 +170,14 @@ void my_ssl_end()
  {
    int i;
    CRYPTO_set_locking_callback(NULL);
-		CRYPTO_set_id_callback(NULL);
+    CRYPTO_set_id_callback(NULL);

    for (i=0; i < CRYPTO_num_locks(); i++)
      pthread_mutex_destroy(&LOCK_crypto[i]);

    my_free((gptr)LOCK_crypto, MYF(0));
+    LOCK_crypto= NULL;
+
    if (SSL_context)
    {
      SSL_CTX_free(SSL_context);
@@ -196,7 +203,9 @@ void my_ssl_end()
 */
 static int my_ssl_set_certs(MYSQL *mysql)
 {
-  char *key_file= mysql->options.ssl_key ? mysql->options.ssl_key : mysql->options.ssl_cert;
+  char *certfile= mysql->options.ssl_cert,
+       *keyfile= mysql->options.ssl_key;
+  
  DBUG_ENTER("my_ssl_set_certs");

  /* Make sure that ssl was allocated and 
@@ -220,21 +229,26 @@ static int my_ssl_set_certs(MYSQL *mysql)
      goto error;
  }

+  if (keyfile && !certfile)
+    certfile= keyfile;
+  if (certfile && !keyfile)
+    keyfile= certfile;
+
  /* set cert */
-  if (mysql->options.ssl_cert && mysql->options.ssl_cert[0] != 0)  
-    if (SSL_CTX_use_certificate_chain_file(SSL_context, mysql->options.ssl_cert) <= 0)
+  if (certfile  && certfile[0] != 0)  
+    if (SSL_CTX_use_certificate_file(SSL_context, certfile, SSL_FILETYPE_PEM) != 1)
      goto error; 

-    /* set key */
-  if (key_file)
+  /* set key */
+  if (keyfile && keyfile[0])
  {
-    if (SSL_CTX_use_PrivateKey_file(SSL_context, key_file, SSL_FILETYPE_PEM) <= 0)
-      goto error;
-
-    /* verify key */
-    if (!SSL_CTX_check_private_key(SSL_context))
+    if (SSL_CTX_use_PrivateKey_file(SSL_context, keyfile, SSL_FILETYPE_PEM) != 1)
      goto error;
  }
+  /* verify key */
+  if (certfile && !SSL_CTX_check_private_key(SSL_context))
+    goto error;
+  
  if (mysql->options.extension &&
      (mysql->options.extension->ssl_crl || mysql->options.extension->ssl_crlpath))
  {
@@ -318,6 +332,7 @@ SSL *my_ssl_init(MYSQL *mysql)
  if (!my_ssl_initialized)
    my_ssl_start(mysql); 

+  pthread_mutex_lock(&LOCK_ssl_config);
  if (my_ssl_set_certs(mysql))
    goto error;

@@ -333,8 +348,10 @@ SSL *my_ssl_init(MYSQL *mysql)
  SSL_CTX_set_verify(SSL_context, verify, my_verify_callback);
  SSL_CTX_set_verify_depth(SSL_context, 1);

+  pthread_mutex_unlock(&LOCK_ssl_config);
  DBUG_RETURN(ssl);
 error:
+  pthread_mutex_unlock(&LOCK_ssl_config);
  if (ssl)
    SSL_free(ssl);
  DBUG_RETURN(NULL);
@@ -503,7 +520,10 @@ int my_ssl_close(Vio *vio)
  int i, rc;
  DBUG_ENTER("my_ssl_close");

-  
+  if (!vio || !vio->ssl)
+    DBUG_RETURN(1);
+
+  SSL_set_quiet_shutdown(vio->ssl, 1); 
  /* 2 x pending + 2 * data = 4 */ 
  for (i=0; i < 4; i++)
    if ((rc= SSL_shutdown(vio->ssl)))
--- a/unittest/libmariadb/certs/create_certs.sh
+++ b/unittest/libmariadb/certs/create_certs.sh
@@ -13,3 +13,5 @@ openssl rsa -in client-key-enc.pem -out client-key.pem \
 -passin pass:qwerty -passout pass:

 cat server-cert.pem client-cert.pem > ca.pem
+
+cat client-key.pem client-cert.pem ca.pem > combined.pem
--- a/unittest/libmariadb/ssl.c.in
+++ b/unittest/libmariadb/ssl.c.in
@@ -104,7 +104,6 @@ static int test_conc95(MYSQL *my)
    return SKIP;

  rc= mysql_query(my, "DROP USER 'ssluser1'@'localhost'");
-  check_mysql_rc(rc, my);
  rc= mysql_query(my, "GRANT ALL ON test.* TO 'ssluser1'@'localhost' IDENTIFIED BY 'sslpw' REQUIRE X509");
  check_mysql_rc(rc, my);
  rc= mysql_query(my, "FLUSH PRIVILEGES");
@@ -484,6 +483,96 @@ static int test_bug62743(MYSQL *my)
  return OK;
 }

+#ifndef _WIN32
+int thread_conc102(void)
+#else
+DWORD WINAPI thread_conc102(void)
+#endif
+{
+  MYSQL *mysql;
+  int rc;
+  MYSQL_RES *res;
+  mysql_thread_init();
+  mysql= mysql_init(NULL);
+
+  mysql_ssl_set(mysql, "@CMAKE_SOURCE_DIR@/unittest/libmariadb/certs/combined.pem", 
+                       "@CMAKE_SOURCE_DIR@/unittest/libmariadb/certs/combined.pem", 
+                       "@CMAKE_SOURCE_DIR@/unittest/libmariadb/certs/combined.pem", 0, 0);
+
+  if(!mysql_real_connect(mysql, hostname, username, password, schema,
+          port, socketname, 0))
+  {
+    diag(">Error: %s", mysql_error(mysql));
+    goto end;
+  }
+  if (!mysql_get_ssl_cipher(mysql))
+  {
+    diag("Error: No ssl connection");
+    goto end;
+  }
+  pthread_mutex_lock(&LOCK_test);
+  rc= mysql_query(mysql, "UPDATE t_conc102 SET a=a+1");
+  check_mysql_rc(rc, mysql);
+  pthread_mutex_unlock(&LOCK_test);
+  check_mysql_rc(rc, mysql);
+  if (res= mysql_store_result(mysql))
+    mysql_free_result(res);
+end:
+  mysql_close(mysql);
+  mysql_thread_end();
+  return 0;
+}
+
+static int test_conc_102(MYSQL *mysql)
+{
+
+  int rc;
+  int i;
+  MYSQL_ROW row;
+  MYSQL_RES *res;
+#ifndef _WIN32
+  pthread_t threads[50];
+#else
+  HANDLE hthreads[50];
+  DWORD threads[50];
+#endif
+
+  rc= mysql_query(mysql, "DROP TABLE IF EXISTS t_conc102");
+  check_mysql_rc(rc, mysql);
+  rc= mysql_query(mysql, "CREATE TABLE t_conc102 ( a int)");
+  check_mysql_rc(rc, mysql);
+  rc= mysql_query(mysql, "INSERT INTO t_conc102 VALUES (0)");
+  check_mysql_rc(rc, mysql);
+
+  for (i=0; i < 50; i++)
+  {
+#ifndef _WIN32
+    pthread_create(&threads[i], NULL, (void *)thread_conc102, NULL);
+#else
+    hthreads[i]= CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)thread_conc27, NULL, 0, &threads[i]);
+    if (hthreads[i]==NULL)
+      diag("error while starting thread");
+#endif
+  }
+  for (i=0; i < 50; i++)
+  {
+#ifndef _WIN32
+    pthread_join(threads[i], NULL);
+#else
+    WaitForSingleObject(hthreads[i], INFINITE);
+#endif
+  } 
+  rc= mysql_query(mysql, "SELECT a FROM t_conc102");
+  check_mysql_rc(rc, mysql);
+  res= mysql_store_result(mysql);
+  row= mysql_fetch_row(res);
+  diag("Found: %s", row[0]);
+  FAIL_IF(strcmp(row[0], "50") != 0, "Expected 50");
+  mysql_free_result(res);
+  return OK;
+}
+
+
 struct my_tests_st my_tests[] = {
  {"test_ssl", test_ssl, TEST_CONNECTION_NEW, 0,  NULL,  NULL},
  {"test_conc50", test_conc50, TEST_CONNECTION_NEW, 0,  NULL,  NULL},
@@ -497,9 +586,8 @@ struct my_tests_st my_tests[] = {
  {"test_phpbug51647", test_phpbug51647, TEST_CONNECTION_NONE, 0, NULL, NULL},
  {"test_ssl_cipher", test_ssl_cipher, TEST_CONNECTION_NONE, 0,  NULL,  NULL},
  {"test_multi_ssl_connections", test_multi_ssl_connections, TEST_CONNECTION_NONE, 0,  NULL,  NULL},
-#ifndef WIN32
+  {"test_conc_102", test_conc_102, TEST_CONNECTION_NEW, 0, NULL, NULL},
  {"test_ssl_threads", test_ssl_threads, TEST_CONNECTION_NEW, 0,  NULL,  NULL},
-#endif 

  {NULL, NULL, 0, 0, NULL, NULL}
 };