PS-9823 fix: mysql_migrate_keyring won't work with PS's components

https://perconadev.atlassian.net/browse/PS-9823

Reworked keyring components to make sure their corresponding '.so' objects do not have unresolved symbols (from the 'dlopen(..., RTLD_NOW)' point of view). This change is needed to ensure that keyring components can be loaded not only from the 'mysqld' executable but from utilities like 'mysql_migrate_keyring' as well.

Keyring components' 'CMakeLists.txt' files fortified with aditional linking option '${LINK_FLAG_NO_UNDEFINED}' (-Wl,--no-undefined) which prevents building '.so' shared objects with unresolved sumbols.

Removed custom allocator from the 'components/keyrings/common/data/pfs_string.h' header
to eliminate divergence from upstream code. 'pfs_string' kept as an alias to 'std::string' to
minimize Percona code changes.

Removed 'DBUG_TRACE' calls from the 'component_keyring_kmip' code to get rid of 'mysys' library dependency.

Calls to 'mysql_components_handle_std_exception()' inside 'component_keyring_kmip'
replaced with 'LogComponentErr()' to avoid dependency on 'minchassis'.

'memset_s()' Percona's extension function renamed to 'my_memset_s()'.

Added a series of 'component_keyring_xxx.dynamic_loading' MTR test cases (one for each keyring component: 'file', 'kmip', 'kms') that checks if the component's '.so' file does not have unresolved symbols in order to make sure that it can be loaded from auxiliary utilities (like 'mysql_migrate_keyring'). These MTR test cases internally build a helper utility from the '.cpp' file ('mysql-test/std_data/dlopen_checker.cpp') that simply performs an attempt to call 'dlopen(..., RTLD_NOW)' for the provided '.so' object.

Added 'keyring_vault.migrate_keyring' MTR test case that tests for keyring data migration from 'keyring_vault' plugin to 'keyring_file' plugin and back. Internally, it uses 'mysqld' executable
in keyring data migration mode to perform key stansfer from one plugin to another.

Added 'have_keyring_file_plugin.inc' MTR include file that helps to identify if the server was
build with 'keyring_file' plugin.

Removed 'have_keyring_kmip_plugin.inc' MTR include file added previously by mistake.

Author: percona-ysorokin

components/keyrings/common/CMakeLists.txt

@@ -30,7 +30,6 @@ SET(KEYRING_COMMON_SOURCES
   # Data representation
   data/data.cc
   data/meta.cc
-  data/pfs_string.cpp
   # File reader/writer
   data_file/reader.cc
   data_file/writer.cc
@@ -51,10 +50,6 @@ IF(COMPONENT_COMPILE_VISIBILITY)
   SET(COMPILE_OPTIONS_ARG COMPILE_OPTIONS "${COMPONENT_COMPILE_VISIBILITY}")
 ENDIF()
 
-INCLUDE_DIRECTORIES(SYSTEM
-        ${BOOST_PATCHES_DIR}
-        ${BOOST_INCLUDE_DIR})
-
 ADD_CONVENIENCE_LIBRARY(
   keyring_common
   ${KEYRING_COMMON_SOURCES}

components/keyrings/common/data/pfs_string.cpp

@@ -1,103 +1,8 @@
-
 #ifndef PFS_STRING_INCLUDED
 #define PFS_STRING_INCLUDED
 
-#include <limits>
-#include "my_sys.h"
-#include "mysql/service_mysql_alloc.h"
-#include "sql/psi_memory_key.h"
-
-extern PSI_memory_key KEY_mem_keyring;
-
-/**
-  Malloc_allocator is based on sql/malloc_allocator.h, but uses a fixed PSI key
-  instead
-*/
-template <class T = void *>
-class Malloc_allocator {
-  // This cannot be const if we want to be able to swap.
-  PSI_memory_key m_key = KEY_mem_keyring;
-
- public:
-  typedef T value_type;
-  typedef size_t size_type;
-  typedef ptrdiff_t difference_type;
-
-  typedef T *pointer;
-  typedef const T *const_pointer;
-
-  typedef T &reference;
-  typedef const T &const_reference;
-
-  pointer address(reference r) const { return &r; }
-  const_pointer address(const_reference r) const { return &r; }
-
-  explicit Malloc_allocator() {}
-
-  template <class U>
-  Malloc_allocator(const Malloc_allocator<U> &other [[maybe_unused]])
-      : m_key(other.psi_key()) {}
-
-  template <class U>
-  Malloc_allocator &operator=(const Malloc_allocator<U> &other
-                              [[maybe_unused]]) {
-    assert(m_key == other.psi_key());  // Don't swap key.
-  }
-
-  pointer allocate(size_type n, const_pointer hint [[maybe_unused]] = nullptr) {
-    if (n == 0) return nullptr;
-    if (n > max_size()) throw std::bad_alloc();
-
-    pointer p = static_cast<pointer>(
-        my_malloc(m_key, n * sizeof(T), MYF(MY_WME | ME_FATALERROR)));
-    if (p == nullptr) throw std::bad_alloc();
-    return p;
-  }
-
-  void deallocate(pointer p, size_type) { my_free(p); }
-
-  template <class U, class... Args>
-  void construct(U *p, Args &&... args) {
-    assert(p != nullptr);
-    try {
-      ::new ((void *)p) U(std::forward<Args>(args)...);
-    } catch (...) {
-      assert(false);  // Constructor should not throw an exception.
-    }
-  }
-
-  void destroy(pointer p) {
-    assert(p != nullptr);
-    try {
-      p->~T();
-    } catch (...) {
-      assert(false);  // Destructor should not throw an exception
-    }
-  }
-
-  size_type max_size() const {
-    return std::numeric_limits<size_t>::max() / sizeof(T);
-  }
-
-  template <class U>
-  struct rebind {
-    typedef Malloc_allocator<U> other;
-  };
-
-  PSI_memory_key psi_key() const { return m_key; }
-};
-
-template <class T>
-bool operator==(const Malloc_allocator<T> &a1, const Malloc_allocator<T> &a2) {
-  return a1.psi_key() == a2.psi_key();
-}
-
-template <class T>
-bool operator!=(const Malloc_allocator<T> &a1, const Malloc_allocator<T> &a2) {
-  return a1.psi_key() != a2.psi_key();
-}
+#include <string>
 
-using pfs_string =
-    std::basic_string<char, std::char_traits<char>, Malloc_allocator<char>>;
+using pfs_string = std::string;
 
 #endif  // PFS_STRING_INCLUDED

components/keyrings/common/data/pfs_string.h

@@ -84,6 +84,9 @@ MYSQL_ADD_COMPONENT(keyring_file
   LINK_LIBRARIES ${KEYRING_FILE_LIBRARIES}
   MODULE_ONLY
 )
+
+MY_TARGET_LINK_OPTIONS(component_keyring_file "${LINK_FLAG_NO_UNDEFINED}")
+
 IF(APPLE)
   SET_TARGET_PROPERTIES(component_keyring_file PROPERTIES
     LINK_FLAGS "-undefined dynamic_lookup")

components/keyrings/keyring_file/CMakeLists.txt

@@ -83,6 +83,9 @@ MYSQL_ADD_COMPONENT(keyring_kmip
   LINK_LIBRARIES ${KEYRING_KMIP_LIBRARIES}
   MODULE_ONLY
 )
+
+MY_TARGET_LINK_OPTIONS(component_keyring_kmip "${LINK_FLAG_NO_UNDEFINED}")
+
 IF(APPLE)
   SET_TARGET_PROPERTIES(component_keyring_kmip PROPERTIES
     LINK_FLAGS "-undefined dynamic_lookup")

components/keyrings/keyring_kmip/CMakeLists.txt

@@ -25,7 +25,6 @@
 #include <memory>
 
 #include "backend.h"
-#include "my_dbug.h"
 
 #include <mysql/components/minimal_chassis.h>
 
@@ -47,15 +46,13 @@ using keyring_common::utils::get_random_data;
 
 Keyring_kmip_backend::Keyring_kmip_backend(config::Config_pod const &config)
     : valid_(false), config_(config) {
-  DBUG_TRACE;
   valid_ = true;
 }
 
 bool Keyring_kmip_backend::load_cache(
     keyring_common::operations::Keyring_operations<
         Keyring_kmip_backend, keyring_common::data::Data_extension<IdExt>>
         &operations) {
-  DBUG_TRACE;
   // We have to load keys and secrets with state==ACTIVE only
   //TODO: implement better logic with the new KMIP library
   try {
@@ -126,9 +123,16 @@ bool Keyring_kmip_backend::load_cache(
         return true;
       }
     }
-
+  } catch (const std::exception &e) {
+    std::string err_msg = std::string("std exception in function '") +
+                          __func__ + "': " + e.what();
+    LogComponentErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG, err_msg.c_str());
+    return true;
   } catch (...) {
-    mysql_components_handle_std_exception(__func__);
+    std::string err_msg =
+        std::string("Unknown exception in function '") + __func__ + '\'';
+    LogComponentErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG, err_msg.c_str());
+    return true;
   }
 
   return false;
@@ -137,13 +141,11 @@ bool Keyring_kmip_backend::load_cache(
 bool Keyring_kmip_backend::get(const Metadata &, Data &) const {
   /* Shouldn't have reached here if we cache things. */
   assert(0);
-  DBUG_TRACE;
   return false;
 }
 
 bool Keyring_kmip_backend::store(const Metadata &metadata,
                                  Data_extension<IdExt> &data) {
-  DBUG_TRACE;
   if (!metadata.valid() || !data.valid()) return true;
   kmippp::context::id_t id;
   try {
@@ -184,8 +186,15 @@ bool Keyring_kmip_backend::store(const Metadata &metadata,
       return true;
     }
     data.set_extension({id});
+  } catch (const std::exception &e) {
+    std::string err_msg = std::string("std exception in function '") +
+                          __func__ + "': " + e.what();
+    LogComponentErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG, err_msg.c_str());
+    return true;
   } catch (...) {
-    mysql_components_handle_std_exception(__func__);
+    std::string err_msg =
+        std::string("Unknown exception in function '") + __func__ + '\'';
+    LogComponentErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG, err_msg.c_str());
     return true;
   }
   return false;
@@ -204,15 +213,21 @@ size_t Keyring_kmip_backend::size() const {
     return keys.size() + secrets.size();
     //we may have deactivated keys counted, so we need to count active keys only
     //TODO: implement better logic with the new KMIP library
+  } catch (const std::exception &e) {
+    std::string err_msg = std::string("std exception in function '") +
+                          __func__ + "': " + e.what();
+    LogComponentErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG, err_msg.c_str());
+    return 0;
   } catch (...) {
-    mysql_components_handle_std_exception(__func__);
+    std::string err_msg =
+        std::string("Unknown exception in function '") + __func__ + '\'';
+    LogComponentErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG, err_msg.c_str());
     return 0;
   }
 }
 
 bool Keyring_kmip_backend::erase(const Metadata &metadata,
                                  Data_extension<IdExt> &data) {
-  DBUG_TRACE;
   if (!metadata.valid()) return true;
 
   auto ctx = kmip_ctx();
@@ -238,7 +253,6 @@ bool Keyring_kmip_backend::erase(const Metadata &metadata,
 bool Keyring_kmip_backend::generate(const Metadata &metadata,
                                     Data_extension<IdExt> &data,
                                     size_t length) {
-  DBUG_TRACE;
   if (!metadata.valid()) return true;
 
   std::unique_ptr<unsigned char[]> key(new unsigned char[length]);

components/keyrings/keyring_kmip/backend/backend.cc

@@ -218,8 +218,6 @@ PROVIDES_SERVICE(component_keyring_kmip, keyring_aes),
     PROVIDES_SERVICE(component_keyring_kmip, log_builtins_string),
     END_COMPONENT_PROVIDES();
 
-PSI_memory_key KEY_mem_keyring_kmip;
-
 /** List of dependencies */
 BEGIN_COMPONENT_REQUIRES(component_keyring_kmip)
 REQUIRES_SERVICE(registry), REQUIRES_SERVICE(log_builtins),

components/keyrings/keyring_kmip/keyring_kmip.cc

@@ -73,6 +73,8 @@ SET(KEYRING_KMS_LIBRARIES keyring_common ext::curl ${SSL_LIBRARIES})
 
 MYSQL_ADD_COMPONENT(keyring_kms ${KEYRING_KMS_SOURCE} LINK_LIBRARIES ${KEYRING_KMS_LIBRARIES} MODULE_ONLY)
 
+MY_TARGET_LINK_OPTIONS(component_keyring_kms "${LINK_FLAG_NO_UNDEFINED}")
+
 MY_CHECK_CXX_COMPILER_WARNING("-Wno-suggest-override" HAS_FLAG)
 IF(HAS_FLAG)
   TARGET_COMPILE_OPTIONS(component_keyring_kms PUBLIC "-Wno-suggest-override")

components/keyrings/keyring_kms/CMakeLists.txt

@@ -34,7 +34,6 @@
 #include <my_inttypes.h>              /* typedefs */
 #include <my_macros.h>                /* STRINGIFY_ARG */
 #include <mysql.h>                    /* MYSQL */
-#include <mysql/service_mysql_alloc.h>/* my_strdup */
 #include <mysql_com.h>                /* get_tty_password */
 #include <print_version.h>            /* print_version */
 #include <typelib.h>                  /* find_type_or_exit */

components/test/keyring_encryption_test/options.cc

@@ -612,9 +612,7 @@ extern size_t my_fwrite(FILE *stream, const uchar *Buffer, size_t Count,
                         myf MyFlags);
 extern my_off_t my_fseek(FILE *stream, my_off_t pos, int whence);
 extern my_off_t my_ftell(FILE *stream);
-#if !defined(HAVE_MEMSET_S)
-void memset_s(void *dest, size_t dest_max, int c, size_t n);
-#endif
+void my_memset_s(void *dest, size_t dest_max, int c, size_t n);
 
 /* implemented in my_syslog.c */