Merge branch 'dev/PS-9823-8.0-mysql_migrate_keyring_unusable' into dev/PS-9823-8.4-mysql_migrate_keyring_unusable

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 both
'component_keyring_kmip' and 'component_keyring_vault' replaced with
'LogComponentErr()' to avoid dependency on 'minchassis'.

Added explicit dependency on 'OpenSSL::Crypto' for the
component_keyring_vault' (needed for AES functions).

'memset_s()' Percona's extension function moved from 'mysys' to 'library_mysys'
and renamed to 'my_memset_s()'.

Removed unused 'components/keyrings/common/data/keyring_alloc.h'.
Removed unused 'plugin/keyring/common/secure_string.h'.
Removed unused 'Secure_allocator' class template from the
'plugin/keyring/common/keyring_memory.h'.

Added a series of 'component_keyring_xxx.dynamic_loading' MTR test cases (one
for each keyring component: 'file', 'vault', '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 'component_keyring_vault.migrate_keyring' MTR test case that tests for
keyring data migration from 'component_keyring_vault' to
'component_keyring_file' and back.

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

components/keyrings/common/data/keyring_alloc.h

@@ -2,112 +2,14 @@
 #ifndef PFS_STRING_INCLUDED
 #define PFS_STRING_INCLUDED
 
-#include <limits>
 #include <optional>
 #include <sstream>
-#include "my_sys.h"
-#include "mysql/service_mysql_alloc.h"
-#include "sql/psi_memory_key.h"
+#include <string>
 
-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 Comp_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 Comp_malloc_allocator() {}
-
-  template <class U>
-  Comp_malloc_allocator(const Comp_malloc_allocator<U> &other [[maybe_unused]])
-      : m_key(other.psi_key()) {}
-
-  template <class U>
-  Comp_malloc_allocator &operator=(const Comp_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 Comp_malloc_allocator<U> other;
-  };
-
-  PSI_memory_key psi_key() const { return m_key; }
-};
-
-template <class T>
-bool operator==(const Comp_malloc_allocator<T> &a1,
-                const Comp_malloc_allocator<T> &a2) {
-  return a1.psi_key() == a2.psi_key();
-}
-
-template <class T>
-bool operator!=(const Comp_malloc_allocator<T> &a1,
-                const Comp_malloc_allocator<T> &a2) {
-  return a1.psi_key() != a2.psi_key();
-}
-
-using pfs_string = std::basic_string<char, std::char_traits<char>,
-                                     Comp_malloc_allocator<char>>;
+using pfs_string = std::string;
 
 using pfs_optional_string = std::optional<pfs_string>;
 
-using pfs_secure_ostringstream =
-    std::basic_ostringstream<char, std::char_traits<char>,
-                             Comp_malloc_allocator<char>>;
+using pfs_ostringstream = std::ostringstream;
 
 #endif  // PFS_STRING_INCLUDED

components/keyrings/common/data/pfs_string.cpp

@@ -85,6 +85,8 @@ MYSQL_ADD_COMPONENT(keyring_file
   MODULE_ONLY
 )
 
+MY_TARGET_LINK_OPTIONS(component_keyring_file "${LINK_FLAG_NO_UNDEFINED}")
+
 DOWNGRADE_STRINGOP_WARNINGS(component_keyring_file)
 
 IF(APPLE)

components/keyrings/common/data/pfs_string.h

@@ -84,6 +84,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_file/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/CMakeLists.txt

@@ -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/backend/backend.cc

@@ -73,6 +73,8 @@ SET(KEYRING_KMS_LIBRARIES keyring_common ext::curl OpenSSL::SSL OpenSSL::Crypto)
 
 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_kmip/keyring_kmip.cc

@@ -75,14 +75,16 @@ set(KEYRING_VAULT_SOURCE
   component_callbacks.cc
 )
 
-set(KEYRING_VAULT_LIBRARIES keyring_common ext::curl extra::rapidjson)
+set(KEYRING_VAULT_LIBRARIES keyring_common ext::curl extra::rapidjson OpenSSL::Crypto)
 
 MYSQL_ADD_COMPONENT(keyring_vault
   ${KEYRING_VAULT_SOURCE}
   LINK_LIBRARIES ${KEYRING_VAULT_LIBRARIES}
   MODULE_ONLY
 )
 
+MY_TARGET_LINK_OPTIONS(component_keyring_vault "${LINK_FLAG_NO_UNDEFINED}")
+
 target_compile_definitions(component_keyring_vault PRIVATE LOG_COMPONENT_TAG="component_keyring_vault")
 
 target_include_directories(