maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #12003
Re: 387839e44cc: Plugin for Hashicorp Vault Key Management System
Hi, Julius!
Looks good! A couple of minor comments, see below
On Dec 02, Julius Goryavsky wrote:
> revision-id: 387839e44cc (mariadb-10.4.10-179-g387839e44cc)
> parent(s): 8115a02283d
> author: Julius Goryavsky <julius.goryavsky@xxxxxxxxxxx>
> committer: Julius Goryavsky <julius.goryavsky@xxxxxxxxxxx>
> timestamp: 2019-11-15 13:37:35 +0100
> message:
>
> Plugin for Hashicorp Vault Key Management System
>
> diff --git a/plugin/hashicorp_key_management/CMakeLists.txt b/plugin/hashicorp_key_management/CMakeLists.txt
> new file mode 100644
> index 00000000000..4b5f79afa66
> --- /dev/null
> +++ b/plugin/hashicorp_key_management/CMakeLists.txt
> @@ -0,0 +1,11 @@
> +INCLUDE(FindCURL)
> +IF(NOT CURL_FOUND)
> + # Can't build plugin
> + RETURN()
> +ENDIF()
> +
> +INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/sql ${CURL_INCLUDE_DIR})
> +
> +MYSQL_ADD_PLUGIN(HASHICORP_KEY_MANAGEMENT
> + hashicorp_key_management_plugin.cc
> + LINK_LIBRARIES ${CURL_LIBRARIES})
> diff --git a/plugin/hashicorp_key_management/hashicorp_key_management_plugin.cc b/plugin/hashicorp_key_management/hashicorp_key_management_plugin.cc
> new file mode 100644
> index 00000000000..26ee7b7cd3d
> --- /dev/null
> +++ b/plugin/hashicorp_key_management/hashicorp_key_management_plugin.cc
> @@ -0,0 +1,325 @@
> +/* Copyright (C) 2019 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-1335 USA */
> +
> +#include <mysql/plugin_encryption.h>
> +#include <mysqld_error.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include <string>
> +#include <sstream>
> +#include <curl/curl.h>
> +
> +static char* vault_url;
> +static char* token;
> +static char* vault_ca;
> +
> +static MYSQL_SYSVAR_STR(vault_ca, vault_ca,
> + PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY,
> + "Path to Certificate Authority (CA) bundle (is a file "
> + "that contains root and intermediate certificates)",
> + NULL, NULL, "");
> +
> +static MYSQL_SYSVAR_STR(vault_url, vault_url,
> + PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY,
> + "HTTP[s] URL that is used to connect to the Hashicorp Vault server",
> + NULL, NULL, "https://127.0.0.1:8200/v1/secret";);
> +
> +static MYSQL_SYSVAR_STR(token, token,
> + PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY | PLUGIN_VAR_NOSYSVAR,
> + "Authentication token that passed to the Hashicorp Vault "
> + "in the request header",
> + NULL, NULL, "");
> +
> +static struct st_mysql_sys_var* settings[] = {
> + MYSQL_SYSVAR(vault_url),
> + MYSQL_SYSVAR(token),
> + MYSQL_SYSVAR(vault_ca),
> + NULL
> +};
> +
> +static std::string get_error_from_curl (CURLcode curl_code, char *curl_errbuf)
> +{
> + size_t len = strlen(curl_errbuf);
> + std::ostringstream stream;
> + if (curl_code != CURLE_OK)
> + {
> + stream << "CURL returned this error code: " << curl_code;
> + stream << " with error message : ";
> + if (len)
> + stream << curl_errbuf;
> + else
> + stream << curl_easy_strerror(curl_code);
> + }
> + return stream.str();
> +}
> +
> +#define max_response_size 65536
> +
> +static size_t write_response_memory (void *contents, size_t size, size_t nmemb,
> + void *userp)
> +{
> + size_t realsize = size * nmemb;
> + if (size != 0 && realsize / size != nmemb)
> + return 0; // overflow detected
> + std::ostringstream *read_data = static_cast<std::ostringstream*>(userp);
> + size_t current_length = read_data->tellp();
> + if (current_length + realsize > max_response_size)
> + return 0; // response size limit exceeded
> + read_data->write(static_cast<char*>(contents), realsize);
> + if (!read_data->good())
> + return 0;
> + return realsize;
> +}
> +
> +#define timeout 300
> +
> +static bool curl_run (char *url, std::string *response)
> +{
> + char curl_errbuf [CURL_ERROR_SIZE];
> + struct curl_slist *list = NULL;
> + std::ostringstream read_data_stream;
> + CURLcode curl_res = CURLE_OK;
> + long http_code = 0;
> + CURL *curl = curl_easy_init();
> + if (curl == NULL)
> + {
> + my_printf_error(ER_UNKNOWN_ERROR,
> + "Cannot initialize curl session",
> + ME_ERROR_LOG_ONLY);
> + return true;
> + }
> + std::string token_header = std::string("X-Vault-Token:") +
> + std::string(token);
> + curl_errbuf[0] = '\0';
> + if ((list= curl_slist_append(list, token_header.c_str())) == NULL ||
> + (curl_res= curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, curl_errbuf)) !=
> + CURLE_OK ||
> + (curl_res= curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION,
> + write_response_memory)) != CURLE_OK ||
> + (curl_res= curl_easy_setopt(curl, CURLOPT_WRITEDATA,
> + &read_data_stream)) !=
> + CURLE_OK ||
> + (curl_res= curl_easy_setopt(curl, CURLOPT_HTTPHEADER, list)) !=
> + CURLE_OK ||
> + /*
> + The options CURLOPT_SSL_VERIFYPEER and CURLOPT_SSL_VERIFYHOST are
> + set explicitly to withstand possible future changes in curl defaults:
> + */
> + (curl_res= curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1)) !=
> + CURLE_OK ||
> + (curl_res= curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L)) !=
> + CURLE_OK ||
> + (vault_ca != NULL && strlen(vault_ca) != 0 &&
vault_ca still cannot be NULL :)
> + (curl_res= curl_easy_setopt(curl, CURLOPT_CAINFO, vault_ca)) !=
> + CURLE_OK) ||
> + (curl_res= curl_easy_setopt(curl, CURLOPT_USE_SSL, CURLUSESSL_ALL)) !=
> + CURLE_OK ||
> + (curl_res= curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L)) !=
> + CURLE_OK ||
> + (curl_res= curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, timeout)) !=
> + CURLE_OK ||
> + (curl_res= curl_easy_setopt(curl, CURLOPT_TIMEOUT, timeout)) !=
> + CURLE_OK)
> + {
if you get an error here you won't free list, so it'll be a memory leak.
In fact, I don't understand why you need two if() here, just combine
them in one, the error message is the same anyway, and the second if()
already has curl_slist_free_all().
> + my_printf_error(ER_UNKNOWN_ERROR,
> + get_error_from_curl(curl_res, curl_errbuf).c_str(),
> + ME_ERROR_LOG_ONLY);
> + return true;
> + }
> + if ((curl_res = curl_easy_setopt(curl, CURLOPT_URL, url)) != CURLE_OK ||
> + (curl_res = curl_easy_perform(curl)) != CURLE_OK ||
> + (curl_res = curl_easy_getinfo (curl, CURLINFO_RESPONSE_CODE,
> + &http_code)) != CURLE_OK)
> + {
> + curl_slist_free_all(list);
> + my_printf_error(ER_UNKNOWN_ERROR,
> + get_error_from_curl(curl_res, curl_errbuf).c_str(),
> + ME_ERROR_LOG_ONLY);
> + return true;
> + }
> + curl_slist_free_all(list);
> + if (http_code == 404)
> + {
> + *response = std::string("");
> + return false;
> + }
> + *response = read_data_stream.str();
> + return http_code < 200 || http_code >= 300;
> +}
> +
> +static unsigned int get_latest_version (uint key_id)
> +{
> + char url[2048];
> + std::string response_str;
> + const char *response;
> + const char *js, *ver;
> + int js_len, ver_len;
> + size_t response_len;
> + snprintf(url, sizeof(url), "%s/%u", vault_url, key_id);
> + if (curl_run(url, &response_str)) {
> + my_printf_error(ER_UNKNOWN_ERROR,
> + "Unable to get key data",
> + 0);
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + response = response_str.c_str();
> + response_len = response_str.size();
> + if (json_get_object_key(response, response + response_len,
> + "metadata", &js, &js_len) != JSV_OBJECT)
> + {
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + if (json_get_object_key(js, js+js_len,"version",
> + &ver, &ver_len) != JSV_NUMBER)
> + {
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + /*
> + Here we may use atoi, but it has undefined overflow behavior
> + and does not work with unsigned integers, so use strtoul:
> + */
> + unsigned long version = strtoul(ver, NULL, 10);
> + /*
> + If the result of the conversion does not fit into an
> + unsigned integer or if an error (for example, overflow)
> + occurred during the conversion, then we need to return
> + an error:
> + */
> + if (version > UINT_MAX || errno)
this might be an error with gcc, "condition is always true"
double check this please, no need to fix anything if there's no error
> + {
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + return version;
this might be an error with VS, because of the implicit ulong->uint cast.
double check this please, no need to fix anything if there's no error
> +}
> +
> +static unsigned int get_key_from_vault (unsigned int key_id,
> + unsigned int key_version,
> + unsigned char *dstbuf,
> + unsigned int *buflen)
> +{
> + char url[2048];
> + std::string response_str;
> + const char *response;
> + const char *js, *ver, *key;
> + int js_len, key_len, ver_len;
> + size_t response_len;
> + if (key_version != ENCRYPTION_KEY_VERSION_INVALID)
> + snprintf(url, sizeof(url), "%s/%u?%u", vault_url, key_id, key_version);
> + else
> + snprintf(url, sizeof(url), "%s/%u", vault_url, key_id);
> + if (curl_run(url, &response_str))
> + {
> + my_printf_error(ER_UNKNOWN_ERROR,
> + "Unable to get key data",
> + 0);
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + response = response_str.c_str();
> + response_len = response_str.size();
> +#ifndef NDEBUG
> + /*
> + An internal check that is needed only for debugging the plugin
> + operation - in order to ensure that we get from the Hashicorp Vault
> + server exactly the version of the key that is needed:
> + */
> + if (key_version != ENCRYPTION_KEY_VERSION_INVALID)
> + {
> + if (json_get_object_key(response, response + response_len,
> + "metadata", &js, &js_len) != JSV_OBJECT)
> + {
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + if (json_get_object_key(js, js+js_len,"version",
> + &ver, &ver_len) != JSV_NUMBER)
> + {
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + /*
> + Here we may use atoi, but it has undefined overflow behavior
> + and does not work with unsigned integers, so use strtoul:
> + */
> + unsigned long version = strtoul(ver, NULL, 10);
> + /*
> + If the result of the conversion does not fit into an
> + unsigned integer or if an error (for example, overflow)
> + occurred during the conversion, then we need to return
> + an error:
> + */
> + if (errno || key_version != version)
> + {
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + }
> +#endif
> + if (json_get_object_key(response, response + response_len,
> + "data", &js, &js_len) != JSV_OBJECT)
> + {
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + if (json_get_object_key(js, js + js_len, "data",
> + &key, &key_len) != JSV_STRING)
> + {
> + return ENCRYPTION_KEY_VERSION_INVALID;
> + }
> + if (*buflen < (unsigned) key_len) {
> + *buflen= key_len;
> + return ENCRYPTION_KEY_BUFFER_TOO_SMALL;
> + }
> + *buflen= key_len;
> + memcpy(dstbuf, key, key_len);
> + return 0;
> +}
> +
> +struct st_mariadb_encryption hashicorp_key_management_plugin= {
> + MariaDB_ENCRYPTION_INTERFACE_VERSION,
> + get_latest_version,
> + get_key_from_vault,
> + 0, 0, 0, 0, 0
> +};
> +
> +static int hashicorp_key_management_plugin_init(void *p)
> +{
> + curl_global_init(CURL_GLOBAL_ALL);
> + return 0;
> +}
> +
> +static int hashicorp_key_management_plugin_deinit(void *p)
> +{
> + curl_global_cleanup();
> + return 0;
> +}
> +
> +/*
> + Plugin library descriptor
> +*/
> +maria_declare_plugin(hashicorp_key_management)
> +{
> + MariaDB_ENCRYPTION_PLUGIN,
> + &hashicorp_key_management_plugin,
> + "hashicorp_key_management",
> + "MariaDB Corporation",
> + "HashiCorp Vault key management plugin",
> + PLUGIN_LICENSE_GPL,
> + hashicorp_key_management_plugin_init,
> + hashicorp_key_management_plugin_deinit,
> + 0x0100 /* 1.0 */,
> + NULL, /* status variables */
> + settings,
> + "1.0",
> + MariaDB_PLUGIN_MATURITY_ALPHA
> +}
> +maria_declare_plugin_end;
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx
