Re: [Maria-developers] MDEV-9659 Encryption plugin that uses AWS Key management service
Hi, Wlad! On Mar 03, wlad@mariadb.com wrote:
revision-id: c1ed3c0267da2ece7664e9f876314979114b2186 (mariadb-10.1.11-26-gc1ed3c0) parent(s): a62db8186e40a62366bc86f49a0195e9b687d86d author: Vladislav Vaintroub committer: Vladislav Vaintroub timestamp: 2016-03-03 21:29:29 +0100 message:
MDEV-9659 Encryption plugin that uses AWS Key management service
Looks pretty good, thanks! See comment below...
diff --git a/cmake/plugin.cmake b/cmake/plugin.cmake index c24239c..361cb59 100644 --- a/cmake/plugin.cmake +++ b/cmake/plugin.cmake @@ -74,7 +74,9 @@ MACRO(MYSQL_ADD_PLUGIN) SET(compat "with${compat}") ENDIF()
- IF (compat STREQUAL ".") + IF (ARG_DISABLED) + SET(howtobuild NO) + ELSEIF (compat STREQUAL ".") SET(howtobuild DYNAMIC) ELSEIF (compat STREQUAL "with.") IF (NOT ARG_MODULE_ONLY) @@ -122,7 +124,7 @@ MACRO(MYSQL_ADD_PLUGIN)
# Build either static library or module IF (PLUGIN_${plugin} MATCHES "(STATIC|AUTO|YES)" AND NOT ARG_MODULE_ONLY - AND NOT ARG_DISABLED AND NOT ARG_CLIENT) + AND NOT ARG_CLIENT)
IF(CMAKE_GENERATOR MATCHES "Makefiles|Ninja") # If there is a shared library from previous shared build, @@ -178,8 +180,7 @@ MACRO(MYSQL_ADD_PLUGIN) SET (mysql_optional_plugins ${mysql_optional_plugins} PARENT_SCOPE) ENDIF() ELSEIF(PLUGIN_${plugin} MATCHES "(DYNAMIC|AUTO|YES)" - AND NOT ARG_STATIC_ONLY AND NOT WITHOUT_DYNAMIC_PLUGINS - AND NOT ARG_DISABLED) + AND NOT ARG_STATIC_ONLY AND NOT WITHOUT_DYNAMIC_PLUGINS)
ADD_VERSION_INFO(${target} MODULE SOURCES) ADD_LIBRARY(${target} MODULE ${SOURCES})
a bit more of the explanation would've been nice. in the changeset comment or - even better - pushing this change separately. you can select what files to include in the commit, you know...
diff --git a/plugin/aws_key_management/CMakeLists.txt b/plugin/aws_key_management/CMakeLists.txt new file mode 100644 index 0000000..ec9ee46 --- /dev/null +++ b/plugin/aws_key_management/CMakeLists.txt @@ -0,0 +1,144 @@ +# We build parts of AWS C++ SDK as CMake external project +# The restrictions of the SDK (https://github.com/awslabs/aws-sdk-cpp/blob/master/README.md) +# are + +# - OS : Windows,Linux or OSX +# - C++11 compiler : VS2013+, gcc 4.7+, clang 3.3+ +# - libcurl development package needs to be present on Unixes +# +# Since we're using ExternalProject that reads from git repository +# we also require GIT to be present on the build machine + + +# Give message why the building this plugin is skipped (only if -DVERBOSE is defined) +# and and bail out. +MACRO(SKIP_AWS_PLUGIN msg) + IF(DEFINED VERBOSE)
VERBOSE - is it used somewhere else? Or you've invented it on the spot for this plugin? I understand that it can be used in other cmake files, but is it?
+ MESSAGE(STATUS "Skip aws_key_management - ${msg}") + RETURN() + ENDIF() +ENDMACRO()
Ok, as discussed on irc, I'd suggest to add here a check whether AWS SDK is already installed. Like, CheckLibraryExists or CheckSymbolExists. And if not - then try to download and compile it (which would require new cmake, git, curl, etc).
+IF(CMAKE_VERSION VERSION_LESS "2.8.8") + SKIP_AWS_PLUGIN("CMake is too old") +ENDIF() + +FIND_PACKAGE(Git) +IF(NOT GIT_FOUND) + SKIP_AWS_PLUGIN("no GIT") +ENDIF() + +INCLUDE(ExternalProject) +IF (NOT(WIN32 OR APPLE OR (CMAKE_SYSTEM_NAME MATCHES "Linux"))) + SKIP_AWS_PLUGIN("OS unsupported by AWS SDK") +ENDIF() + +IF(UNIX) + FIND_PACKAGE(CURL) + IF(NOT CURL_FOUND) + SKIP_AWS_PLUGIN("AWS C++ SDK requires libcurl development package") + ENDIF() + SET(PIC_FLAG -fPIC) +ENDIF() + +SET(CXX11_FLAGS) +SET(OLD_COMPILER_MSG "AWS SDK requires c++11 -capable compiler (minimal supported versions are g++ 4.7, clang 3.3, VS2103)") +IF(CMAKE_CXX_COMPILER_ID MATCHES "GNU") + EXECUTE_PROCESS(COMMAND ${CMAKE_CXX_COMPILER} -dumpversion OUTPUT_VARIABLE GCC_VERSION)
eh? I thought cmake knows compiler versions... let me see This one: CMAKE_<LANG>_COMPILER_VERSION
+ IF (GCC_VERSION VERSION_LESS 4.7) + SKIP_AWS_PLUGIN("${OLD_COMPILER_MSG}") + ENDIF() + SET(CXX11_FLAGS "-std=c++11")
Instead of checking the compiler version, I would rather suggest to check for a feature that you need to use. Like MY_CHECK_CXX_COMPILER_FLAG(-std=c++11) IF (NOT HAVE_CXX__STD_C__11) SKIP_AWS_PLUGIN("${OLD_COMPILER_MSG}") ENDIF() Something like that. Not sure about HAVE_CXX__STD_C__11 name, though.
+ELSEIF (CMAKE_CXX_COMPILER_ID MATCHES "Clang") + IF (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 3.3) + SKIP_AWS_PLUGIN("${OLD_COMPILER_MSG}") + ENDIF() + SET(CXX11_FLAGS "-stdlib=libc++")
Same.
+ELSEIF(MSVC) + IF (MSVC_VERSION LESS 1800) + SKIP_AWS_PLUGIN("${OLD_COMPILER_MSG}") + ENDIF() +ELSE() + SKIP_AWS_PLUGIN("Compiler not supported by AWS C++ SDK") +ENDIF() + +IF(WIN32) + SET(EXTRA_SDK_CMAKE_FLAGS -DCMAKE_CXX_FLAGS_DEBUGOPT="" -DCMAKE_EXE_LINKER_FLAGS_DEBUGOPT="")
For consistency you can put this inside ELSEIF(MSVC), that's where you set options for gcc and clang.
+ENDIF() + +FIND_PROGRAM(PERL_EXECUTABLE perl) + +IF(PERL_EXECUTABLE) + # CMake minimimal version for building AWS SDK can be less than 3.1.2 + # Downgrade this requirement. + SET(AWS_SDK_PATCH_COMMAND + PATCH_COMMAND ${PERL_EXECUTABLE} -i.bak -pe s/3.1.2/2.8/g ${CMAKE_BINARY_DIR}/aws-sdk-cpp/CMakeLists.txt)
1. Without perl it'll require 3.1.2, right? 2. Couldn't you do that with cmake alone? 3. Is that safe at all - you pull the latest version from git, they can start using cmake-3.1.2 features any time.
+ENDIF() + +ExternalProject_Add( + aws_sdk_cpp + GIT_REPOSITORY "https://github.com/awslabs/aws-sdk-cpp.git" + UPDATE_COMMAND "" + ${AWS_SDK_PATCH_COMMAND} + SOURCE_DIR "${CMAKE_BINARY_DIR}/aws-sdk-cpp" + CMAKE_ARGS + -DBUILD_ONLY=aws-cpp-sdk-kms -DSTATIC_LINKING=1 + "-DCMAKE_CXX_FLAGS_DEBUG=${CMAKE_CXX_FLAGS_DEBUG} ${PIC_FLAG}" + "-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=${CMAKE_CXX_FLAGS_RELWITHDEBINFO} ${PIC_FLAG}" + "-DCMAKE_CXX_FLAGS_RELEASE=${CMAKE_CXX_FLAGS_RELEASE} ${PIC_FLAG}" + "-DCMAKE_CXX_FLAGS_MINSIZEREL=${CMAKE_CXX_FLAGS_MINSIZEREL} ${PIC_FLAG}" + ${EXTRA_SDK_CMAKE_FLAGS} + -DCMAKE_INSTALL_PREFIX=${CMAKE_BINARY_DIR}/aws_sdk_cpp + TEST_COMMAND "" +) + +# Figure out where AWS installs SDK libraries +# The below is defined in AWS SDK's CMakeLists.txt +# (and their handling is weird, every OS has special install directory) +if(WIN32) + SET(SDK_INSTALL_BINARY_PREFIX "windows") +ELSEIF(APPLE) + SET(SDK_INSTALL_BINARY_PREFIX "mac") +ELSEIF(UNIX) + SET(SDK_INSTALL_BINARY_PREFIX "linux") +ENDIF() +IF(NOT APPLE) + IF(CMAKE_SIZEOF_VOID_P EQUAL 8) + SET(SDK_INSTALL_BINARY_PREFIX "${SDK_INSTALL_BINARY_PREFIX}/intel64") + ELSE() + SET(SDK_INSTALL_BINARY_PREFIX "${SDK_INSTALL_BINARY_PREFIX}/ia32") + ENDIF() +ENDIF() +IF(CMAKE_CONFIGURATION_TYPES) + SET(SDK_INSTALL_BINARY_PREFIX "${SDK_INSTALL_BINARY_PREFIX}/${CMAKE_CFG_INTDIR}") +ENDIF() + +# We do not need to build the whole SDK , just 2 of its libs +set(AWS_SDK_LIBS aws-cpp-sdk-core aws-cpp-sdk-kms) + +FOREACH(lib ${AWS_SDK_LIBS}) + ADD_LIBRARY(${lib} STATIC IMPORTED) + ADD_DEPENDENCIES(${lib} aws_sdk_cpp) + SET(loc "${CMAKE_BINARY_DIR}/aws_sdk_cpp/lib/${SDK_INSTALL_BINARY_PREFIX}/${CMAKE_STATIC_LIBRARY_PREFIX}${lib}${CMAKE_STATIC_LIBRARY_SUFFIX}") + SET_TARGET_PROPERTIES(${lib} PROPERTIES IMPORTED_LOCATION ${loc}) + IF(WIN32) + SET_TARGET_PROPERTIES(${lib} PROPERTIES IMPORTED_LINK_INTERFACE_LIBRARIES "bcrypt;winhttp;wininet;userenv") + ELSE() + SET_TARGET_PROPERTIES(${lib} PROPERTIES IMPORTED_LINK_INTERFACE_LIBRARIES "${SSL_LIBRARIES};${CURL_LIBRARIES}") + ENDIF() +ENDFOREACH() + +IF(CMAKE_SYSTEM_NAME MATCHES "Linux") + # Need whole-archive , otherwise static libraries are not linked + SET(AWS_SDK_LIBS -Wl,--whole-archive ${AWS_SDK_LIBS} -Wl,--no-whole-archive) +ENDIF() + +SET_TARGET_PROPERTIES(aws_sdk_cpp PROPERTIES EXCLUDE_FROM_ALL TRUE) +INCLUDE_DIRECTORIES(${CMAKE_BINARY_DIR}/aws_sdk_cpp/include) +SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX11_FLAGS}") +MYSQL_ADD_PLUGIN(aws_key_management aws_key_management_plugin.cc + MODULE_ONLY COMPONENT aws-key-management + LINK_LIBRARIES ${AWS_SDK_LIBS} DISABLED) diff --git a/plugin/aws_key_management/aws_key_management_plugin.cc b/plugin/aws_key_management/aws_key_management_plugin.cc new file mode 100644 index 0000000..fa7d042 --- /dev/null +++ b/plugin/aws_key_management/aws_key_management_plugin.cc @@ -0,0 +1,611 @@ +/* + Copyright (c) 2016 MariaDB Corporation + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; version 2 of the License. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ + + +#include <my_global.h> +#include <my_pthread.h> +#include <my_sys.h> +#include <mysql/plugin_encryption.h> +#include <my_crypt.h> +#include <string.h> +#include <stdio.h> +#include <stdlib.h> +#include <mysqld_error.h> +#include <base64.h> +#include <map> +#include <algorithm> +#include <string> +#include <vector> +#include <iterator> +#include <sstream> +#include <fstream> +#ifndef _WIN32 +#include <sys/types.h> +#include <dirent.h> +#endif
Use cmake checks when there are some, HAVE_DIRENT_H, HAVE_SYS_TYPES_H, HAVE_STRING_H, HAVE_STDLIB_H. Hmm, HAVE_STDLIB_H doesn't make sense to me :) could you remove it from configure.cmake, please? In a separate changeset. May be HAVE_SYS_TYPES_H too, not sure. But do use HAVE_DIRENT_H and HAVE_STDLIB_H above instead of _WIN32.
+#include <aws/core/client/AWSError.h> +#include <aws/core/utils/logging/AWSLogging.h> +#include <aws/core/utils/logging/ConsoleLogSystem.h> +#include <aws/kms/KMSClient.h> +#include <aws/kms/model/DecryptRequest.h> +#include <aws/kms/model/DecryptResult.h> +#include <aws/kms/model/GenerateDataKeyWithoutPlaintextRequest.h> +#include <aws/kms/model/GenerateDataKeyWithoutPlaintextResult.h> +#include <aws/core/utils/Outcome.h> +using namespace std; +using namespace Aws::KMS; +using namespace Aws::KMS::Model; +using namespace Aws::Utils::Logging; +extern void sql_print_error(const char *format, ...); +extern void sql_print_warning(const char *format, ...); +extern void sql_print_information(const char *format, ...); + + +/* Plaintext key info struct */ +struct KEY_INFO +{ + unsigned int key_id; + unsigned int key_version; + unsigned int length; + unsigned char data[MY_AES_MAX_KEY_LENGTH]; + bool load_failed; /* if true, do not attempt to reload?*/ +public: + KEY_INFO() : key_id(0), key_version(0), length(0), load_failed(false){}; +}; +#define KEY_ID_AND_VERSION(key_id,version) ((longlong)key_id << 32 | version) + +/* Cache for the latest version, per key id */ +static std::map<uint, uint> latest_version_cache; + +/* Cache for plaintext keys */ +static std::map<ulonglong, KEY_INFO> key_info_cache; + +static const char *key_spec_names[]={ "AES_128", "AES_256", 0 }; + +/* Plugin variables */ +static char* master_key_id; +static unsigned long key_spec; +static unsigned long log_level; +static int rotate_key; + +/* AWS functionality*/ +static int aws_decrypt_key(const char *path, KEY_INFO *info); +static int aws_generate_datakey(uint key_id, uint version); + +static int extract_id_and_version(const char *name, uint *id, uint *ver); +static unsigned int get_latest_key_version(unsigned int key_id); +static unsigned int get_latest_key_version_nolock(unsigned int key_id); +static int load_key(KEY_INFO *info); + +/* Mutex to serialize access to caches */ +static pthread_mutex_t mtx;
Use PSI, please. That's mysql_mutex_t, etc.
+ +static Aws::KMS::KMSClient *client; + +/* Redirect AWS trace to error log */ +class MySQLLogSystem : public Aws::Utils::Logging::FormattedLogSystem +{ +public: + + using Base = FormattedLogSystem; + MySQLLogSystem(LogLevel logLevel) : + Base(logLevel) + { + } + virtual LogLevel GetLogLevel(void) const override + { + return (LogLevel)log_level; + } + virtual ~MySQLLogSystem() + { + } + +protected: + virtual void ProcessFormattedStatement(Aws::String&& statement) override + { +#ifdef _WIN32 + /* + On Windows, we can't use C runtime functions to write to stdout, + because we compile with static C runtime, so plugin has a stdout + different from server. Thus we're using WriteFile(). + */ + DWORD nSize= (DWORD)statement.size(); + DWORD nWritten; + const char *s= statement.c_str(); + HANDLE h= GetStdHandle(STD_OUTPUT_HANDLE); + + WriteFile(h, s, nSize, &nWritten, NULL); +#else + printf("%s", statement.c_str());
printf? You have declared sql_print_information above, why wouldn't you use it here?
+#endif + } +}; + +/* + Read filenames from the current directory. + This is used in plugin initilization. We check for + plugin specific names aws-kms-key.<id>.<version> + to find out what keys/key versions are already there. +*/ +static vector<string> read_dir() +{ + vector<string> v; + DBUG_ENTER("read_dir"); +#ifdef _WIN32 + WIN32_FIND_DATA ffd; + HANDLE h= FindFirstFile("*", &ffd); + if (h == INVALID_HANDLE_VALUE) + { + DBUG_RETURN(v); + } + do + { + /* handle file possibly updates caches*/ + v.push_back(ffd.cFileName); + } while (FindNextFile(h, &ffd)); + FindClose(h); +#else + DIR *dir= opendir("."); + if (!dir) + { + DBUG_RETURN(v); + } + struct dirent *dp; + while ((dp= readdir(dir)) != NULL) + { + v.push_back(dp->d_name); + } + closedir(dir); +#endif + DBUG_RETURN(v);
ok. although you could've used my_dir and save yourself some troubles :)
+} + + +/* + Plugin initialization. + + Create KMS client and scan datadir to find out which keys and versions + are present. +*/ +static int plugin_init(void *p) +{ + DBUG_ENTER("plugin_init"); + client = new KMSClient(); + if (!client) + { + sql_print_error("Can not initialize KMS client"); + DBUG_RETURN(-1); + } + InitializeAWSLogging(Aws::MakeShared<MySQLLogSystem>("aws_key_management_plugin", (Aws::Utils::Logging::LogLevel) log_level)); + pthread_mutex_init(&mtx, NULL); + vector<string> filenames= read_dir(); + for (size_t i= 0; i < filenames.size(); i++) + { + KEY_INFO info; + if (extract_id_and_version(filenames[i].c_str(), &info.key_id, &info.key_version) == 0) + { + key_info_cache[KEY_ID_AND_VERSION(info.key_id, info.key_version)]= info; + latest_version_cache[info.key_id]= max(info.key_version, latest_version_cache[info.key_id]); + } + } + DBUG_RETURN(0); +} + + + +static int plugin_deinit(void *p) +{ + DBUG_ENTER("plugin_deinit"); + latest_version_cache.clear(); + key_info_cache.clear(); + pthread_mutex_destroy(&mtx); + delete client; + ShutdownAWSLogging(); + DBUG_RETURN(0); +} + +/* Generate filename to store the ciphered key */ +static void format_keyfile_name(char *buf, size_t size, uint key_id, uint version) +{ + snprintf(buf, size, "aws-kms-key.%u.%u", key_id, version); +} + +/* Extract key id and version from file name */ +static int extract_id_and_version(const char *name, uint *id, uint *ver) +{ + int len; + int n= sscanf(name, "aws-kms-key.%u.%u%n", id, ver, &len); + if (n == 2 && *id > 0 && *ver > 0 && len == (int)strlen(name)) + return 0; + return 1; +} + +/* + Decrypt key stored in aws-kms-key.<id>.<version> + Cache the decrypted key. +*/ +static int load_key(KEY_INFO *info) +{ + int ret; + char path[256]; + DBUG_ENTER("load_key"); + DBUG_PRINT("enter", ("id=%u,ver=%u", info->key_id, info->key_version)); + format_keyfile_name(path, sizeof(path), info->key_id, info->key_version); + ret= aws_decrypt_key(path, info); + if (ret) + info->load_failed= true; + + latest_version_cache[info->key_id]= max(latest_version_cache[info->key_id], info->key_version); + key_info_cache[KEY_ID_AND_VERSION(info->key_id, info->key_version)]= *info; + + if (!ret) + { + sql_print_information("AWS KMS plugin: loaded key %u, version %u, key length %u bit", + info->key_id, info->key_version,(uint)info->length*8); + } + else + { + sql_print_warning("AWS KMS plugin: key %u, version %u could not be decrypted", + info->key_id, info->key_version); + } + DBUG_RETURN(ret); +} + + +/* + Get latest version for the key. + + If key is not decrypted yet, this function also decrypt the key + and error will be returned if decryption fails. + + The reason for that is that InnoDB crashes + in case errors are returned by get_key(),
Shall we rather fix that in InnoDB? Do you remember where it crashes?
+ + A new key will be created if it does not exist, provided there is + valid master_key_id. +*/ +static unsigned int get_latest_key_version(unsigned int key_id) +{ + unsigned int ret; + DBUG_ENTER("get_latest_key_version"); + + pthread_mutex_lock(&mtx); + ret= get_latest_key_version_nolock(key_id);
why do you need get_latest_key_version_nolock if you never use it directly?
+ pthread_mutex_unlock(&mtx); + + DBUG_PRINT("info", ("key=%u,ret=%u", key_id, ret)); + DBUG_RETURN(ret); +} + +static unsigned int get_latest_key_version_nolock(unsigned int key_id) +{ + KEY_INFO info; + uint ver; + DBUG_ENTER("get_latest_key_version_nolock"); + ver= latest_version_cache[key_id]; + if (ver > 0) + { + info= key_info_cache[KEY_ID_AND_VERSION(key_id, ver)]; + } + if (info.load_failed) + { + /* Decryption failed previously, don't retry */ + DBUG_RETURN(ENCRYPTION_KEY_VERSION_INVALID); + } + else if (ver > 0) + { + /* Key encrypted already, return it*/
may be s/encrypted already/exists/ ?
+ if (info.length > 0) + DBUG_RETURN(ver); + } + else // (ver == 0) + { + /* Generate a new key, version 1 */ + if (!master_key_id[0]) + { + my_printf_error(ER_UNKNOWN_ERROR, + "Can't generate key %u, because aws_key_management_master_key_id parameter is not set", + MYF(ME_JUST_WARNING), key_id); + DBUG_RETURN(ENCRYPTION_KEY_VERSION_INVALID); + } + if (aws_generate_datakey(key_id, 1) != 0) + DBUG_RETURN(ENCRYPTION_KEY_VERSION_INVALID); + info.key_id= key_id; + info.key_version= 1; + info.length= 0; + } + + if (load_key(&info)) + DBUG_RETURN(ENCRYPTION_KEY_VERSION_INVALID); + DBUG_RETURN(info.key_version); +} + + +/* + Decrypt a file with KMS +*/ +static int aws_decrypt_key(const char *path, KEY_INFO *info) +{ + DBUG_ENTER("aws_decrypt_key"); + + /* Read file content into memory */ + ifstream ifs(path, ios::binary | ios::ate); + if (!ifs.good()) + { + sql_print_error("can't open file %s", path); + } + size_t pos = (size_t)ifs.tellg(); + std::vector<char> contents(pos); + ifs.seekg(0, ios::beg); + ifs.read(&contents[0], pos); + + /* Decrypt data the with AWS */ + DecryptRequest request; + Aws::Utils::ByteBuffer byteBuffer((unsigned char *)contents.data(), pos); + request.SetCiphertextBlob(byteBuffer); + DecryptOutcome outcome = client->Decrypt(request); + if (!outcome.IsSuccess()) + { + sql_print_error("AWS KMS plugin: Decrypt failed for %s : %s", path, + outcome.GetError().GetMessage().c_str()); + DBUG_RETURN(-1); + } + Aws::Utils::ByteBuffer plaintext = outcome.GetResult().GetPlaintext(); + size_t len = plaintext.GetLength(); + + if (len > (int)sizeof(info->data)) + { + sql_print_error("AWS KMS plugin: encoding key too large for %s", path); + DBUG_RETURN(ENCRYPTION_KEY_BUFFER_TOO_SMALL); + } + memcpy(info->data, plaintext.GetUnderlyingData(), len); + info->length= len; + DBUG_RETURN(0); +} + + +/* Generate a new datakey and store it a file */ +static int aws_generate_datakey(uint keyid, uint version) +{ + + DBUG_ENTER("aws_generate_datakey"); + GenerateDataKeyWithoutPlaintextRequest request; + request.SetKeyId(master_key_id); + request.SetKeySpec(DataKeySpecMapper::GetDataKeySpecForName(key_spec_names[key_spec])); + + GenerateDataKeyWithoutPlaintextOutcome outcome; + outcome= client->GenerateDataKeyWithoutPlaintext(request); + if (!outcome.IsSuccess()) + { + sql_print_error("AWS KMS plugin : GenerateDataKeyWithoutPlaintext failed : %s - %s", + outcome.GetError().GetExceptionName().c_str(), + outcome.GetError().GetMessage().c_str()); + DBUG_RETURN(-1); + } + + string out; + char filename[20]; + Aws::Utils::ByteBuffer byteBuffer = outcome.GetResult().GetCiphertextBlob(); + + format_keyfile_name(filename, sizeof(filename), keyid, version); + int fd= my_open(filename, O_RDWR | O_CREAT, 0); + if (fd < 0) + { + sql_print_error("AWS KMS plugin: Can't create file %s", filename); + DBUG_RETURN(-1); + } + size_t len= byteBuffer.GetLength(); + if (my_write(fd, byteBuffer.GetUnderlyingData(), len, 0) != len) + { + sql_print_error("AWS KMS plugin: can't write to %s", filename); + my_close(fd, 0); + my_delete(filename, 0); + DBUG_RETURN(-1); + } + my_close(fd, 0); + sql_print_information("AWS KMS plugin: generated encrypted datakey for key id=%u, version=%u", + keyid, version); + DBUG_RETURN(0); +} + +/* Key rotation for a single key */ +static int rotate_single_key(uint key_id) +{ + uint ver; + ver= latest_version_cache[key_id]; + + if (!ver) + { + my_printf_error(ER_UNKNOWN_ERROR, "key %u does not exist", MYF(ME_JUST_WARNING), key_id); + return -1; + } + else if (aws_generate_datakey(key_id, ver + 1)) + { + my_printf_error(ER_UNKNOWN_ERROR, "Could not generate datakey for key id= %u, ver= %u", + MYF(ME_JUST_WARNING), key_id, ver); + return -1; + } + else + { + KEY_INFO info; + info.key_id= key_id; + info.key_version = ver + 1; + if (load_key(&info)) + { + my_printf_error(ER_UNKNOWN_ERROR, "Could not load datakey for key id= %u, ver= %u", + MYF(ME_JUST_WARNING), key_id, ver); + return -1; + } + } + return 0; +} + +/* Key rotation for all key ids */ +static int rotate_all_keys() +{ + int ret= 0; + for (map<uint, uint>::iterator it= latest_version_cache.begin(); it != latest_version_cache.end(); it++) + { + ret= rotate_single_key(it->first); + if (ret) + break; + } + return ret; +} + +static void update_rotate(MYSQL_THD, struct st_mysql_sys_var *, void *, const void *val) +{ + if (!master_key_id[0]) + { + my_printf_error(ER_UNKNOWN_ERROR, + "aws_key_management_master_key_id must be set to generate new data keys", MYF(ME_JUST_WARNING)); + return; + } + pthread_mutex_lock(&mtx); + rotate_key= *(int *)val; + switch (rotate_key) + { + case 0: + break; + case -1: + rotate_all_keys(); + break; + default: + rotate_single_key(rotate_key); + break; + } + rotate_key= 0; + pthread_mutex_unlock(&mtx); +} + +static unsigned int get_key( + unsigned int key_id, + unsigned int version, + unsigned char* dstbuf, + unsigned int* buflen) +{ + KEY_INFO info; + + DBUG_ENTER("get_key"); + pthread_mutex_lock(&mtx); + info= key_info_cache[KEY_ID_AND_VERSION(key_id, version)]; + if (info.length == 0 && !info.load_failed) + { + info.key_id= key_id; + info.key_version= version; + load_key(&info); + } + pthread_mutex_unlock(&mtx); + + if (info.load_failed) + DBUG_RETURN(ENCRYPTION_KEY_VERSION_INVALID); + if (*buflen < info.length) + { + *buflen= info.length; + DBUG_RETURN(ENCRYPTION_KEY_BUFFER_TOO_SMALL); + } + *buflen= info.length; + memcpy(dstbuf, info.data, info.length); + DBUG_RETURN(0); +} + + +/* Plugin defs */ +struct st_mariadb_encryption aws_key_management_plugin= { + MariaDB_ENCRYPTION_INTERFACE_VERSION, + get_latest_key_version, + get_key, + // use default encrypt/decrypt functions + 0, 0, 0, 0, 0 +}; + + +static TYPELIB key_spec_typelib = +{ + array_elements(key_spec_names) - 1, "", + key_spec_names, NULL +}; + +const char *log_level_names[] = +{ + "Off", + "Fatal", + "Error", + "Warn", + "Info", + "Debug", + "Trace", + 0 +}; + +static TYPELIB log_level_typelib = +{ + array_elements(log_level_names) - 1, "", + log_level_names, NULL +}; + +static MYSQL_SYSVAR_STR(master_key_id, master_key_id, + PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_MEMALLOC, + "Key id for master encryption key. Used to create new datakeys. If not set, no new keys will be created", + NULL, NULL, ""); + +static MYSQL_SYSVAR_ENUM(key_spec, key_spec, + PLUGIN_VAR_RQCMDARG, + "Encryption algorithm used to create new keys.", + NULL, NULL, 0, &key_spec_typelib); + + +static MYSQL_SYSVAR_ENUM(log_level, log_level, + PLUGIN_VAR_RQCMDARG, + "Logging for AWS API", + NULL, NULL, 0, &log_level_typelib); + + +static MYSQL_SYSVAR_INT(rotate_key, rotate_key, + PLUGIN_VAR_RQCMDARG, + "Set this variable to key id to perform rotation of the key. Specify -1 to rotate all keys", + NULL, update_rotate, 0, -1, INT_MAX, 1); + +static struct st_mysql_sys_var* settings[]= { + MYSQL_SYSVAR(master_key_id), + MYSQL_SYSVAR(key_spec), + MYSQL_SYSVAR(rotate_key), + MYSQL_SYSVAR(log_level), + NULL +}; + +/* + Plugin library descriptor +*/ +maria_declare_plugin(aws_key_management) +{ + MariaDB_ENCRYPTION_PLUGIN, + &aws_key_management_plugin, + "aws_key_management", + "MariaDB Corporation", + "AWS key management plugin", + PLUGIN_LICENSE_GPL, + plugin_init, + plugin_deinit, + 0x0100, + NULL, + settings, + "1.0", + MariaDB_PLUGIN_MATURITY_EXPERIMENTAL +} +maria_declare_plugin_end;
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi Serg,
thanks a lot ! I recommited , addressing most of the comments .
On 07.03.2016 19:20, Sergei Golubchik wrote:
> Hi, Wlad!
>
> On Mar 03,wlad@mariadb.com wrote:
>> revision-id: c1ed3c0267da2ece7664e9f876314979114b2186 (mariadb-10.1.11-26-gc1ed3c0)
>> parent(s): a62db8186e40a62366bc86f49a0195e9b687d86d
>> author: Vladislav Vaintroub
>> committer: Vladislav Vaintroub
>> timestamp: 2016-03-03 21:29:29 +0100
>> message:
>>
>> MDEV-9659 Encryption plugin that uses AWS Key management service
> Looks pretty good, thanks!
> See comment below...
>
>> diff --git a/cmake/plugin.cmake b/cmake/plugin.cmake
>> index c24239c..361cb59 100644
>> --- a/cmake/plugin.cmake
>> +++ b/cmake/plugin.cmake
>> @@ -74,7 +74,9 @@ MACRO(MYSQL_ADD_PLUGIN)
>> SET(compat "with${compat}")
>> ENDIF()
>>
>> - IF (compat STREQUAL ".")
>> + IF (ARG_DISABLED)
>> + SET(howtobuild NO)
>> + ELSEIF (compat STREQUAL ".")
>> SET(howtobuild DYNAMIC)
>> ELSEIF (compat STREQUAL "with.")
>> IF (NOT ARG_MODULE_ONLY)
>> @@ -122,7 +124,7 @@ MACRO(MYSQL_ADD_PLUGIN)
>>
>> # Build either static library or module
>> IF (PLUGIN_${plugin} MATCHES "(STATIC|AUTO|YES)" AND NOT ARG_MODULE_ONLY
>> - AND NOT ARG_DISABLED AND NOT ARG_CLIENT)
>> + AND NOT ARG_CLIENT)
>>
>> IF(CMAKE_GENERATOR MATCHES "Makefiles|Ninja")
>> # If there is a shared library from previous shared build,
>> @@ -178,8 +180,7 @@ MACRO(MYSQL_ADD_PLUGIN)
>> SET (mysql_optional_plugins ${mysql_optional_plugins} PARENT_SCOPE)
>> ENDIF()
>> ELSEIF(PLUGIN_${plugin} MATCHES "(DYNAMIC|AUTO|YES)"
>> - AND NOT ARG_STATIC_ONLY AND NOT WITHOUT_DYNAMIC_PLUGINS
>> - AND NOT ARG_DISABLED)
>> + AND NOT ARG_STATIC_ONLY AND NOT WITHOUT_DYNAMIC_PLUGINS)
>>
>> ADD_VERSION_INFO(${target} MODULE SOURCES)
>> ADD_LIBRARY(${target} MODULE ${SOURCES})
> a bit more of the explanation would've been nice. in the changeset
> comment or - even better - pushing this change separately. you can
> select what files to include in the commit, you know...
Ok. Made a separate commit for this
>> diff --git a/plugin/aws_key_management/CMakeLists.txt b/plugin/aws_key_management/CMakeLists.txt
>> new file mode 100644
>> index 0000000..ec9ee46
>> --- /dev/null
>> +++ b/plugin/aws_key_management/CMakeLists.txt
>> @@ -0,0 +1,144 @@
>> +# We build parts of AWS C++ SDK as CMake external project
>> +# The restrictions of the SDK (https://github.com/awslabs/aws-sdk-cpp/blob/master/README.md)
>> +# are
>> +
>> +# - OS : Windows,Linux or OSX
>> +# - C++11 compiler : VS2013+, gcc 4.7+, clang 3.3+
>> +# - libcurl development package needs to be present on Unixes
>> +#
>> +# Since we're using ExternalProject that reads from git repository
>> +# we also require GIT to be present on the build machine
>> +
>> +
>> +# Give message why the building this plugin is skipped (only if -DVERBOSE is defined)
>> +# and and bail out.
>> +MACRO(SKIP_AWS_PLUGIN msg)
>> + IF(DEFINED VERBOSE)
> VERBOSE - is it used somewhere else? Or you've invented it on the spot
> for this plugin?
> I understand that it can be used in other cmake files, but is it?
It is not used elsewhere. invented on the spot. It can be used
elsewhere, if we decide to make it a common convention.
It was basically just a tool debug the configuration errors, less
chatty than some other CMakeLists.txt are using :)
I suppose I could use message_once in 10.2 instead.
>> + MESSAGE(STATUS "Skip aws_key_management - ${msg}")
>> + RETURN()
>> + ENDIF()
>> +ENDMACRO()
> Ok, as discussed on irc, I'd suggest to add here a check whether AWS SDK
> is already installed. Like, CheckLibraryExists or CheckSymbolExists.
> And if not - then try to download and compile it (which would require
> new cmake, git, curl, etc).
Ok, I did that . It still requires curl, but no git.
>
>> +IF(CMAKE_VERSION VERSION_LESS "2.8.8")
>> + SKIP_AWS_PLUGIN("CMake is too old")
>> +ENDIF()
>> +
>> +FIND_PACKAGE(Git)
>> +IF(NOT GIT_FOUND)
>> + SKIP_AWS_PLUGIN("no GIT")
>> +ENDIF()
>> +
>> +INCLUDE(ExternalProject)
>> +IF (NOT(WIN32 OR APPLE OR (CMAKE_SYSTEM_NAME MATCHES "Linux")))
>> + SKIP_AWS_PLUGIN("OS unsupported by AWS SDK")
>> +ENDIF()
>> +
>> +IF(UNIX)
>> + FIND_PACKAGE(CURL)
>> + IF(NOT CURL_FOUND)
>> + SKIP_AWS_PLUGIN("AWS C++ SDK requires libcurl development package")
>> + ENDIF()
>> + SET(PIC_FLAG -fPIC)
>> +ENDIF()
>> +
>> +SET(CXX11_FLAGS)
>> +SET(OLD_COMPILER_MSG "AWS SDK requires c++11 -capable compiler (minimal supported versions are g++ 4.7, clang 3.3, VS2103)")
>> +IF(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
>> + EXECUTE_PROCESS(COMMAND ${CMAKE_CXX_COMPILER} -dumpversion OUTPUT_VARIABLE GCC_VERSION)
> eh? I thought cmake knows compiler versions... let me see
> This one: CMAKE_<LANG>_COMPILER_VERSION
I did not change this code, I gave other options a fair try, but no avail.
1. CMAKE_<LANG>_COMPILER_VERSION one exists since CMake 2.8.10, and we
use older versions on many machines. Can't really use that, if we stick
to native cmakes that come with the distros.
2. gcc4.7 does not really work ((I on some reason thought it would ,
but rechecked), even if --std=c++11 . Level of C++11 standard in this
compiler is still insufficient (missing
std::is_trivially_destructible() at least), and also Amazon is using
compile options not available with 4.7 (-Wno-<something>)
So I'm back at checking compiler version with conservative means.
>> +
>> +FIND_PROGRAM(PERL_EXECUTABLE perl)
>> +
>> +IF(PERL_EXECUTABLE)
>> + # CMake minimimal version for building AWS SDK can be less than 3.1.2
>> + # Downgrade this requirement.
>> + SET(AWS_SDK_PATCH_COMMAND
>> + PATCH_COMMAND ${PERL_EXECUTABLE} -i.bak -pe s/3.1.2/2.8/g ${CMAKE_BINARY_DIR}/aws-sdk-cpp/CMakeLists.txt)
> 1. Without perl it'll require 3.1.2, right?
If we do not patch, it'll require 3.1.2
> 2. Couldn't you do that with cmake alone?
Possible, but would require a cmake scriptfile. I'm using our own
"replace" utility - found use for that, for the first time :)
> 3. Is that safe at all - you pull the latest version from git, they can
> start using cmake-3.1.2 features any time.
Ok, I'm no longer use the latest version - I use the latest and the only
one marked with a tag 0.9.6 . Using latest is not a good idea anyway,
if we want a reproducible build.
>
>> +
>> +static Aws::KMS::KMSClient *client;
>> +
>> +/* Redirect AWS trace to error log */
>> +class MySQLLogSystem : public Aws::Utils::Logging::FormattedLogSystem
>> +{
>> +public:
>> +
>> + using Base = FormattedLogSystem;
>> + MySQLLogSystem(LogLevel logLevel) :
>> + Base(logLevel)
>> + {
>> + }
>> + virtual LogLevel GetLogLevel(void) const override
>> + {
>> + return (LogLevel)log_level;
>> + }
>> + virtual ~MySQLLogSystem()
>> + {
>> + }
>> +
>> +protected:
>> + virtual void ProcessFormattedStatement(Aws::String&& statement) override
>> + {
>> +#ifdef _WIN32
>> + /*
>> + On Windows, we can't use C runtime functions to write to stdout,
>> + because we compile with static C runtime, so plugin has a stdout
>> + different from server. Thus we're using WriteFile().
>> + */
>> + DWORD nSize= (DWORD)statement.size();
>> + DWORD nWritten;
>> + const char *s= statement.c_str();
>> + HANDLE h= GetStdHandle(STD_OUTPUT_HANDLE);
>> +
>> + WriteFile(h, s, nSize, &nWritten, NULL);
>> +#else
>> + printf("%s", statement.c_str());
> printf? You have declared sql_print_information above, why wouldn't you
> use it here?
The statement that is being printed is already formatted, it contains a
longish prefix containing tag (like [INFO], [DEBUG], [WARNING]) a
timestamp, a subcomponent name . sql_print_information would add
duplicate timestamp, another INFO tag, and not really added value, but
it will make it look weirder-
>> +#endif
>> + }
>> +};
>> +
>> +/*
>> + Read filenames from the current directory.
>> + This is used in plugin initilization. We check for
>> + plugin specific names aws-kms-key.<id>.<version>
>> + to find out what keys/key versions are already there.
>> +*/
>> +static vector<string> read_dir()
>> +{
>> + vector<string> v;
>> + DBUG_ENTER("read_dir");
>> +#ifdef _WIN32
>> + WIN32_FIND_DATA ffd;
>> + HANDLE h= FindFirstFile("*", &ffd);
>> + if (h == INVALID_HANDLE_VALUE)
>> + {
>> + DBUG_RETURN(v);
>> + }
>> + do
>> + {
>> + /* handle file possibly updates caches*/
>> + v.push_back(ffd.cFileName);
>> + } while (FindNextFile(h, &ffd));
>> + FindClose(h);
>> +#else
>> + DIR *dir= opendir(".");
>> + if (!dir)
>> + {
>> + DBUG_RETURN(v);
>> + }
>> + struct dirent *dp;
>> + while ((dp= readdir(dir)) != NULL)
>> + {
>> + v.push_back(dp->d_name);
>> + }
>> + closedir(dir);
>> +#endif
>> + DBUG_RETURN(v);
> ok. although you could've used my_dir and save yourself some troubles :)
Indeed, thanks for reminding!
>> +}
>> +
>> +
>> +/*
>> + Plugin initialization.
>> +
>> + Create KMS client and scan datadir to find out which keys and versions
>> + are present.
>> +*/
>> +static int plugin_init(void *p)
>> +{
>> + DBUG_ENTER("plugin_init");
>> + client = new KMSClient();
>> + if (!client)
>> + {
>> + sql_print_error("Can not initialize KMS client");
>> + DBUG_RETURN(-1);
>> + }
>> + InitializeAWSLogging(Aws::MakeShared<MySQLLogSystem>("aws_key_management_plugin", (Aws::Utils::Logging::LogLevel) log_level));
>> + pthread_mutex_init(&mtx, NULL);
>> + vector<string> filenames= read_dir();
>> + for (size_t i= 0; i < filenames.size(); i++)
>> + {
>> + KEY_INFO info;
>> + if (extract_id_and_version(filenames[i].c_str(), &info.key_id, &info.key_version) == 0)
>> + {
>> + key_info_cache[KEY_ID_AND_VERSION(info.key_id, info.key_version)]= info;
>> + latest_version_cache[info.key_id]= max(info.key_version, latest_version_cache[info.key_id]);
>> + }
>> + }
>> + DBUG_RETURN(0);
>> +}
>> +
>> +
>> +
>> +static int plugin_deinit(void *p)
>> +{
>> + DBUG_ENTER("plugin_deinit");
>> + latest_version_cache.clear();
>> + key_info_cache.clear();
>> + pthread_mutex_destroy(&mtx);
>> + delete client;
>> + ShutdownAWSLogging();
>> + DBUG_RETURN(0);
>> +}
>> +
>> +/* Generate filename to store the ciphered key */
>> +static void format_keyfile_name(char *buf, size_t size, uint key_id, uint version)
>> +{
>> + snprintf(buf, size, "aws-kms-key.%u.%u", key_id, version);
>> +}
>> +
>> +/* Extract key id and version from file name */
>> +static int extract_id_and_version(const char *name, uint *id, uint *ver)
>> +{
>> + int len;
>> + int n= sscanf(name, "aws-kms-key.%u.%u%n", id, ver, &len);
>> + if (n == 2 && *id > 0 && *ver > 0 && len == (int)strlen(name))
>> + return 0;
>> + return 1;
>> +}
>> +
>> +/*
>> + Decrypt key stored in aws-kms-key.<id>.<version>
>> + Cache the decrypted key.
>> +*/
>> +static int load_key(KEY_INFO *info)
>> +{
>> + int ret;
>> + char path[256];
>> + DBUG_ENTER("load_key");
>> + DBUG_PRINT("enter", ("id=%u,ver=%u", info->key_id, info->key_version));
>> + format_keyfile_name(path, sizeof(path), info->key_id, info->key_version);
>> + ret= aws_decrypt_key(path, info);
>> + if (ret)
>> + info->load_failed= true;
>> +
>> + latest_version_cache[info->key_id]= max(latest_version_cache[info->key_id], info->key_version);
>> + key_info_cache[KEY_ID_AND_VERSION(info->key_id, info->key_version)]= *info;
>> +
>> + if (!ret)
>> + {
>> + sql_print_information("AWS KMS plugin: loaded key %u, version %u, key length %u bit",
>> + info->key_id, info->key_version,(uint)info->length*8);
>> + }
>> + else
>> + {
>> + sql_print_warning("AWS KMS plugin: key %u, version %u could not be decrypted",
>> + info->key_id, info->key_version);
>> + }
>> + DBUG_RETURN(ret);
>> +}
>> +
>> +
>> +/*
>> + Get latest version for the key.
>> +
>> + If key is not decrypted yet, this function also decrypt the key
>> + and error will be returned if decryption fails.
>> +
>> + The reason for that is that InnoDB crashes
>> + in case errors are returned by get_key(),
> Shall we rather fix that in InnoDB?
> Do you remember where it crashes?
When I found it first,Jan was in process of removing assertions, so I
never filed a bug. I rechecked now, hoping everything is fixed, but it
was not the case, I still run into an assertion.
I do "create table .. encrypted=YES". With invalid aws-kms-key.1.1 in
datadir, purposefully invalid, to simulate AWS error
This gives:
a) get_latest_version for key 1 returns 1,
b)attempt to retrieve version 1 for key 1 fails,
c) after b) then fil_crypt_buf() crashes here
https://github.com/MariaDB/server/blob/10.1/storage/xtradb/fil/fil0crypt.cc#L596
if (! ((rc == MY_AES_OK) && ((ulint) dstlen == srclen))) {
...
ut_error;
>> +
>> + A new key will be created if it does not exist, provided there is
>> + valid master_key_id.
>> +*/
>> +static unsigned int get_latest_key_version(unsigned int key_id)
>> +{
>> + unsigned int ret;
>> + DBUG_ENTER("get_latest_key_version");
>> +
>> + pthread_mutex_lock(&mtx);
>> + ret= get_latest_key_version_nolock(key_id);
> why do you need get_latest_key_version_nolock if you never use it
> directly?
No strong reason rreally. I do a lot of early returns from the nolock
function. I could squeeze everything into one function, and do goto
instead of returns, but I felt like using the helper function was easier.
>> + pthread_mutex_unlock(&mtx);
>> +
>> + DBUG_PRINT("info", ("key=%u,ret=%u", key_id, ret));
>> + DBUG_RETURN(ret);
>> +}
>> +
>> +static unsigned int get_latest_key_version_nolock(unsigned int key_id)
>> +{
>> + KEY_INFO info;
>> + uint ver;
>> + DBUG_ENTER("get_latest_key_version_nolock");
>> + ver= latest_version_cache[key_id];
>> + if (ver > 0)
>> + {
>> + info= key_info_cache[KEY_ID_AND_VERSION(key_id, ver)];
>> + }
>> + if (info.load_failed)
>> + {
>> + /* Decryption failed previously, don't retry */
>> + DBUG_RETURN(ENCRYPTION_KEY_VERSION_INVALID);
>> + }
>> + else if (ver > 0)
>> + {
>> + /* Key encrypted already, return it*/
> may be s/encrypted already/exists/ ?
Yes, this is the intention.
Thanks again,
Wlad
Hi, Vladislav!
On Mar 12, Vladislav Vaintroub wrote:
> >> diff --git a/plugin/aws_key_management/CMakeLists.txt b/plugin/aws_key_management/CMakeLists.txt
> >> new file mode 100644
> >> index 0000000..ec9ee46
> >> --- /dev/null
> >> +++ b/plugin/aws_key_management/CMakeLists.txt
> >> @@ -0,0 +1,144 @@
> >> +
> >> +# Give message why the building this plugin is skipped (only if -DVERBOSE is defined)
> >> +# and and bail out.
> >> +MACRO(SKIP_AWS_PLUGIN msg)
> >> + IF(DEFINED VERBOSE)
> > VERBOSE - is it used somewhere else? Or you've invented it on the spot
> > for this plugin?
> > I understand that it can be used in other cmake files, but is it?
> It is not used elsewhere. invented on the spot. It can be used
> elsewhere, if we decide to make it a common convention.
That was the point, exactly. Let's make it a common convention and use
in the future as necessary.
> >> +SET(CXX11_FLAGS)
> >> +SET(OLD_COMPILER_MSG "AWS SDK requires c++11 -capable compiler (minimal supported versions are g++ 4.7, clang 3.3, VS2103)")
> >> +IF(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
> >> + EXECUTE_PROCESS(COMMAND ${CMAKE_CXX_COMPILER} -dumpversion OUTPUT_VARIABLE GCC_VERSION)
> > eh? I thought cmake knows compiler versions... let me see
> > This one: CMAKE_<LANG>_COMPILER_VERSION
> I did not change this code, I gave other options a fair try, but no avail.
> 1. CMAKE_<LANG>_COMPILER_VERSION one exists since CMake 2.8.10, and we
> use older versions on many machines. Can't really use that, if we stick
> to native cmakes that come with the distros.
Hmm. We use CMAKE_C_COMPILER_VERSION in quite a few cmake files.
I wonder how does it work now...
May be in conditions like
IF(CMAKE_C_COMPILER_VERSION VERSION_LESS "4.6")
it doesn't matter whether CMAKE_C_COMPILER_VERSION is defined? If it's
undefined the condition is false, and it's good enough for us?
> 2. gcc4.7 does not really work ((I on some reason thought it would ,
> but rechecked), even if --std=c++11 . Level of C++11 standard in this
> compiler is still insufficient (missing
> std::is_trivially_destructible() at least), and also Amazon is using
> compile options not available with 4.7 (-Wno-<something>)
yes, that was clear. it was not surprising that AWS SDK needs a
reasonably new gcc (as they also require cmake 3.1.2).
> > 3. Is that safe at all - you pull the latest version from git, they can
> > start using cmake-3.1.2 features any time.
> Ok, I'm no longer use the latest version - I use the latest and the only
> one marked with a tag 0.9.6 . Using latest is not a good idea anyway,
> if we want a reproducible build.
Indeed. Good idea!
> >> + printf("%s", statement.c_str());
> > printf? You have declared sql_print_information above, why wouldn't you
> > use it here?
> The statement that is being printed is already formatted, it contains a
> longish prefix containing tag (like [INFO], [DEBUG], [WARNING]) a
> timestamp, a subcomponent name . sql_print_information would add
> duplicate timestamp, another INFO tag, and not really added value, but
> it will make it look weirder-
I see. On the other hand, it's also weird when some log lines don't
match the standard log line pattern. But ok, let's keep it your way and
see how the log will look like...
Regards,
Sergei
participants (2)
-
Sergei Golubchik -
Vladislav Vaintroub