Hi, Julius! Just a couple of comments, see below: On Dec 04, Julius Goryavsky wrote:

revision-id: 387839e44cc (mariadb-10.4.10-179-g387839e44cc) parent(s): 8115a02283d author: Julius Goryavsky <julius.goryavsky@mariadb.com> committer: Julius Goryavsky <julius.goryavsky@mariadb.com> timestamp: 2019-11-15 13:37:35 +0100 message:

Plugin for Hashicorp Vault Key Management System

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..32c0e6417a3 --- /dev/null +++ b/plugin/hashicorp_key_management/hashicorp_key_management_plugin.cc ... + if (tolen_len > max_token_size) + { + my_printf_error(ER_UNKNOWN_ERROR, + "Maximum allowed token length exceeded", + 0); + return true; + } + size_t buf_len = x_vault_token_len + tolen_len + 1;

typo: "token_len" I think

+#ifdef _WIN32 + char *token_header = (char *) _alloca(sizeof(char) * buf_len); +#else + char *token_header = (char *) alloca(sizeof(char) * buf_len); +#endif

Please, remove all these ifdefs and add one at the very top of the file: #ifdef _WIN32 #define alloca _alloca #endif Here I presume that you've tested it in buildbot and alloca is indeed present on all builders and we don't need a fallback here. Also remove sizeof(char) everywhere, it's always 1. C99 standard says 6.5.3.4 The sizeof operator ... When applied to an operand that has type char, unsigned char, or signed char, (or a qualified version thereof) the result is 1.

+ snprintf(token_header, buf_len, "%s%s", x_vault_token, token); + curl_errbuf[0] = '\0'; + if ((list= curl_slist_append(list, token_header)) == 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 || ... + /* + 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 ULONG_MAX > UINT_MAX + if (version > UINT_MAX || errno) +#else + if (errno) +#endif

did you verify that otherwise it makes compilation to fail? better to keep the number of ifdefs in the code to a minimum, so don't add this one unless absolutely necessary

+ { + return ENCRYPTION_KEY_VERSION_INVALID; + } + return (unsigned int) version; +}

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org