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.

Reworked 'components/keyrings/common/data/pfs_string.h' header file so that
it depends on memory functions form
'mysql/components/library_mysys/component_malloc_allocator.h' (available
in 'library_mysys' library ) instead of those from
'mysql/service_mysql_alloc.h' (available in 'mysys' library).

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/data/keyring_alloc.h

@@ -2,12 +2,17 @@
 #ifndef PFS_STRING_INCLUDED
 #define PFS_STRING_INCLUDED
 
+#include <cassert>
 #include <limits>
+#include <new>
 #include <optional>
 #include <sstream>
-#include "my_sys.h"
-#include "mysql/service_mysql_alloc.h"
-#include "sql/psi_memory_key.h"
+#include <string>
+#include <utility>
+
+#include <mysql/components/library_mysys/component_malloc_allocator.h>
+
+#include <my_sys.h>
 
 extern PSI_memory_key KEY_mem_keyring;
 

components/keyrings/common/data/pfs_string.h

@@ -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/keyring_file/CMakeLists.txt

@@ -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_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,12 @@ bool Keyring_kmip_backend::load_cache(
         return true;
       }
     }
-
+  } catch (const std::exception &e) {
+    LogComponentErr(ERROR_LEVEL, ER_STD_UNKNOWN_EXCEPTION, e.what(), __func__);
+    return true;
   } catch (...) {
-    mysql_components_handle_std_exception(__func__);
+    LogComponentErr(ERROR_LEVEL, ER_UNKNOWN_ERROR, MYF(0));
+    return true;
   }
 
   return false;
@@ -137,13 +137,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 +182,11 @@ bool Keyring_kmip_backend::store(const Metadata &metadata,
       return true;
     }
     data.set_extension({id});
+  } catch (const std::exception &e) {
+    LogComponentErr(ERROR_LEVEL, ER_STD_UNKNOWN_EXCEPTION, e.what(), __func__);
+    return true;
   } catch (...) {
-    mysql_components_handle_std_exception(__func__);
+    LogComponentErr(ERROR_LEVEL, ER_UNKNOWN_ERROR, MYF(0));
     return true;
   }
   return false;
@@ -204,15 +205,17 @@ 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) {
+    LogComponentErr(ERROR_LEVEL, ER_STD_UNKNOWN_EXCEPTION, e.what(), __func__);
+    return 0;
   } catch (...) {
-    mysql_components_handle_std_exception(__func__);
+    LogComponentErr(ERROR_LEVEL, ER_UNKNOWN_ERROR, MYF(0));
     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 +241,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

@@ -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_file "${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

@@ -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(

components/keyrings/keyring_vault/CMakeLists.txt

@@ -83,11 +83,13 @@ bool Keyring_vault_backend::init() {
     m_valid = true;
 
     return false;
+  } catch (const std::exception &e) {
+    LogComponentErr(ERROR_LEVEL, ER_STD_UNKNOWN_EXCEPTION, e.what(), __func__);
   } catch (...) {
-    mysql_components_handle_std_exception(__func__);
-    curl_global_cleanup();
-    return true;
+    LogComponentErr(ERROR_LEVEL, ER_UNKNOWN_ERROR, MYF(0));
   }
+  curl_global_cleanup();
+  return true;
 }
 
 bool Keyring_vault_backend::load_cache(

components/keyrings/keyring_vault/backend/backend.cc

@@ -20,7 +20,6 @@ along with this program; if not, write to the Free Software
 #define KEYRING_I_VAULT_CURL_INCLUDED
 
 #include <components/keyrings/common/data/data.h>
-#include <components/keyrings/common/data/keyring_alloc.h>
 #include <components/keyrings/common/data/meta.h>
 #include <components/keyrings/common/data/pfs_string.h>
 #include <components/keyrings/keyring_vault/config/config.h>
@@ -29,13 +28,11 @@ along with this program; if not, write to the Free Software
 
 namespace keyring_vault::backend {
 
-using keyring_common::data::Comp_keyring_alloc;
 using keyring_common::data::Data;
 using keyring_common::meta::Metadata;
 using keyring_vault::config::Vault_version_type;
 
-class IKeyring_vault_curl : public Comp_keyring_alloc,
-                            private boost::noncopyable {
+class IKeyring_vault_curl : private boost::noncopyable {
  public:
   virtual bool init() = 0;
   virtual bool list_keys(pfs_string *response) = 0;

components/keyrings/keyring_vault/backend/i_vault_curl.h

@@ -24,6 +24,8 @@
 #include <algorithm>
 #include <memory>
 
+#include <mysql/components/library_mysys/my_memory.h>
+
 namespace keyring_vault::backend {
 
 bool Vault_base64::encode(const void *src, size_t src_len, pfs_string *encoded,
@@ -35,7 +37,7 @@ bool Vault_base64::encode(const void *src, size_t src_len, pfs_string *encoded,
   // provide access to underlying  data when they are empty. Calling reserve on
   // those containers does not help.
   if (::base64_encode(src, src_len, base64_encoded_text.get()) != 0) {
-    memset_s(base64_encoded_text.get(), memory_needed, 0, memory_needed);
+    my_memset_s(base64_encoded_text.get(), memory_needed, 0, memory_needed);
     return true;
   }
 
@@ -49,7 +51,7 @@ bool Vault_base64::encode(const void *src, size_t src_len, pfs_string *encoded,
   // base64 encode below returns data with NULL terminating string - which we do
   // not care about
   encoded->assign(base64_encoded_text.get(), memory_needed - 1);
-  memset_s(base64_encoded_text.get(), memory_needed, 0, memory_needed);
+  my_memset_s(base64_encoded_text.get(), memory_needed, 0, memory_needed);
 
   return false;
 }
@@ -59,7 +61,7 @@ bool Vault_base64::decode(const pfs_string &src, pfs_string *dst) {
   uint64 data_length = 0;
   if (decode(src, data, &data_length)) return true;
   dst->assign(data.get(), data_length);
-  memset_s(data.get(), data_length, 0, data_length);
+  my_memset_s(data.get(), data_length, 0, data_length);
   return false;
 }
 
@@ -74,8 +76,8 @@ bool Vault_base64::decode(const pfs_string &src, std::unique_ptr<char[]> &dst,
       ::base64_decode(src.c_str(), src.length(), data.get(), nullptr, 0);
 
   if (decoded_length <= 0) {
-    memset_s(data.get(), base64_length_of_memory_needed_for_decode, 0,
-             base64_length_of_memory_needed_for_decode);
+    my_memset_s(data.get(), base64_length_of_memory_needed_for_decode, 0,
+                base64_length_of_memory_needed_for_decode);
     return true;
   }