Merge pull request #31707 from ClickHouse/fix-decrypt-nullable

Fix exception on some of the applications of "decrypt" function on Nullable columns
This commit is contained in:
alexey-milovidov 2021-11-28 16:21:49 +03:00 committed by GitHub
commit d1e1255e38
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 111 additions and 40 deletions

View File

@ -279,37 +279,33 @@ private:
// That may lead later to reading unallocated data from underlying PaddedPODArray
// due to assumption that it is safe to read up to 15 bytes past end.
const auto pad_to_next_block = block_size == 1 ? 0 : 1;
for (size_t r = 0; r < input_rows_count; ++r)
for (size_t row_idx = 0; row_idx < input_rows_count; ++row_idx)
{
resulting_size += (input_column->getDataAt(r).size / block_size + pad_to_next_block) * block_size + 1;
resulting_size += (input_column->getDataAt(row_idx).size / block_size + pad_to_next_block) * block_size + 1;
if constexpr (mode == CipherMode::RFC5116_AEAD_AES_GCM)
resulting_size += tag_size;
}
#if defined(MEMORY_SANITIZER)
encrypted_result_column_data.resize_fill(resulting_size, 0xFF);
#else
encrypted_result_column_data.resize(resulting_size);
#endif
}
auto * encrypted = encrypted_result_column_data.data();
KeyHolder<mode> key_holder;
for (size_t r = 0; r < input_rows_count; ++r)
for (size_t row_idx = 0; row_idx < input_rows_count; ++row_idx)
{
const auto key_value = key_holder.setKey(key_size, key_column->getDataAt(r));
const auto key_value = key_holder.setKey(key_size, key_column->getDataAt(row_idx));
auto iv_value = StringRef{};
if (iv_column)
{
iv_value = iv_column->getDataAt(r);
iv_value = iv_column->getDataAt(row_idx);
/// If the length is zero (empty string is passed) it should be treat as no IV.
if (iv_value.size == 0)
iv_value.data = nullptr;
}
const StringRef input_value = input_column->getDataAt(r);
const StringRef input_value = input_column->getDataAt(row_idx);
if constexpr (mode != CipherMode::MySQLCompatibility)
{
@ -348,7 +344,7 @@ private:
// 1.a.2 Set AAD
if (aad_column)
{
const auto aad_data = aad_column->getDataAt(r);
const auto aad_data = aad_column->getDataAt(row_idx);
int tmp_len = 0;
if (aad_data.size != 0 && EVP_EncryptUpdate(evp_ctx, nullptr, &tmp_len,
reinterpret_cast<const unsigned char *>(aad_data.data), aad_data.size) != 1)
@ -408,7 +404,7 @@ private:
};
/// AES_decrypt(string, key, block_mode[, init_vector])
/// decrypt(string, key, block_mode[, init_vector])
template <typename Impl>
class FunctionDecrypt : public IFunction
{
@ -471,7 +467,9 @@ private:
ColumnPtr result_column;
if (arguments.size() <= 3)
{
result_column = doDecrypt(evp_cipher, input_rows_count, input_column, key_column, nullptr, nullptr);
}
else
{
const auto iv_column = arguments[3].column;
@ -548,59 +546,58 @@ private:
{
size_t resulting_size = 0;
for (size_t r = 0; r < input_rows_count; ++r)
for (size_t row_idx = 0; row_idx < input_rows_count; ++row_idx)
{
size_t string_size = input_column->getDataAt(r).size;
size_t string_size = input_column->getDataAt(row_idx).size;
resulting_size += string_size + 1; /// With terminating zero.
if constexpr (mode == CipherMode::RFC5116_AEAD_AES_GCM)
{
if (string_size < tag_size)
throw Exception("Encrypted data is smaller than the size of additional data for AEAD mode, cannot decrypt.",
ErrorCodes::BAD_ARGUMENTS);
if (string_size > 0)
{
if (string_size < tag_size)
throw Exception("Encrypted data is smaller than the size of additional data for AEAD mode, cannot decrypt.",
ErrorCodes::BAD_ARGUMENTS);
resulting_size -= tag_size;
resulting_size -= tag_size;
}
}
}
#if defined(MEMORY_SANITIZER)
// Pre-fill result column with values to prevent MSAN from dropping dead on
// aes-X-ecb mode with "WARNING: MemorySanitizer: use-of-uninitialized-value".
// This is most likely to be caused by the underlying assembler implementation:
// see crypto/aes/aesni-x86_64.s, function aesni_ecb_encrypt
// which msan seems to fail instrument correctly.
decrypted_result_column_data.resize_fill(resulting_size, 0xFF);
#else
decrypted_result_column_data.resize(resulting_size);
#endif
}
auto * decrypted = decrypted_result_column_data.data();
KeyHolder<mode> key_holder;
for (size_t r = 0; r < input_rows_count; ++r)
for (size_t row_idx = 0; row_idx < input_rows_count; ++row_idx)
{
// 0: prepare key if required
auto key_value = key_holder.setKey(key_size, key_column->getDataAt(r));
auto key_value = key_holder.setKey(key_size, key_column->getDataAt(row_idx));
auto iv_value = StringRef{};
if (iv_column)
{
iv_value = iv_column->getDataAt(r);
iv_value = iv_column->getDataAt(row_idx);
/// If the length is zero (empty string is passed) it should be treat as no IV.
if (iv_value.size == 0)
iv_value.data = nullptr;
}
auto input_value = input_column->getDataAt(r);
auto input_value = input_column->getDataAt(row_idx);
if constexpr (mode == CipherMode::RFC5116_AEAD_AES_GCM)
{
// empty plaintext results in empty ciphertext + tag, means there should be at least tag_size bytes.
if (input_value.size < tag_size)
throw Exception(fmt::format("Encrypted data is too short: only {} bytes, "
"should contain at least {} bytes of a tag.",
input_value.size, block_size, tag_size), ErrorCodes::BAD_ARGUMENTS);
input_value.size -= tag_size;
if (input_value.size > 0)
{
// empty plaintext results in empty ciphertext + tag, means there should be at least tag_size bytes.
if (input_value.size < tag_size)
throw Exception(fmt::format("Encrypted data is too short: only {} bytes, "
"should contain at least {} bytes of a tag.",
input_value.size, block_size, tag_size), ErrorCodes::BAD_ARGUMENTS);
input_value.size -= tag_size;
}
}
if constexpr (mode != CipherMode::MySQLCompatibility)
@ -619,8 +616,9 @@ private:
}
}
// Avoid extra work on empty ciphertext/plaintext for some ciphers
if (!(input_value.size == 0 && block_size == 1 && mode != CipherMode::RFC5116_AEAD_AES_GCM))
/// Avoid extra work on empty ciphertext/plaintext. Always decrypt empty to empty.
/// This makes sense for default implementation for NULLs.
if (input_value.size > 0)
{
// 1: Init CTX
if constexpr (mode == CipherMode::RFC5116_AEAD_AES_GCM)
@ -641,7 +639,7 @@ private:
// 1.a.2: Set AAD if present
if (aad_column)
{
StringRef aad_data = aad_column->getDataAt(r);
StringRef aad_data = aad_column->getDataAt(row_idx);
int tmp_len = 0;
if (aad_data.size != 0 && EVP_DecryptUpdate(evp_ctx, nullptr, &tmp_len,
reinterpret_cast<const unsigned char *>(aad_data.data), aad_data.size) != 1)

View File

@ -0,0 +1,16 @@
aes_encrypt_mysql
\N
D1B43643E1D0E9390E39BA4EAE150851
aes_decrypt_mysql
\N
48656C6C6F20576F726C6421
encrypt
aes-256-ecb \N
aes-256-gcm \N
aes-256-ecb D1B43643E1D0E9390E39BA4EAE150851
aes-256-gcm 219E6478A1A3BB5B686DA4BAD70323F192EFEDCCBBD6F49E78A7E2F6
decrypt
aes-256-ecb \N
aes-256-gcm \N
aes-256-ecb Hello World!
aes-256-gcm Hello World!

View File

@ -0,0 +1,57 @@
-- Tags: no-fasttest
-- Tag no-fasttest: Depends on OpenSSL
-------------------------------------------------------------------------------
-- Validate that encrypt/decrypt (and mysql versions) work against Nullable(String).
-- null gets encrypted/decrypted as null, non-null encrypted/decrypted as usual.
-------------------------------------------------------------------------------
-- using nullIf since that is the easiest way to produce `Nullable(String)` with a `null` value
-----------------------------------------------------------------------------------------------------------------------------------
-- MySQL compatibility
SELECT 'aes_encrypt_mysql';
SELECT aes_encrypt_mysql('aes-256-ecb', CAST(null as Nullable(String)), 'test_key________________________');
WITH 'aes-256-ecb' as mode, 'Hello World!' as plaintext, 'test_key________________________' as key
SELECT hex(aes_encrypt_mysql(mode, toNullable(plaintext), key));
SELECT 'aes_decrypt_mysql';
SELECT aes_decrypt_mysql('aes-256-ecb', CAST(null as Nullable(String)), 'test_key________________________');
WITH 'aes-256-ecb' as mode, unhex('D1B43643E1D0E9390E39BA4EAE150851') as ciphertext, 'test_key________________________' as key
SELECT hex(aes_decrypt_mysql(mode, toNullable(ciphertext), key));
-----------------------------------------------------------------------------------------------------------------------------------
-- encrypt both non-null and null values of Nullable(String)
SELECT 'encrypt';
WITH 'aes-256-ecb' as mode, 'test_key________________________' as key
SELECT mode, encrypt(mode, CAST(null as Nullable(String)), key);
WITH 'aes-256-gcm' as mode, 'test_key________________________' as key, 'test_iv_____' as iv
SELECT mode, encrypt(mode, CAST(null as Nullable(String)), key, iv);
WITH 'aes-256-ecb' as mode, 'test_key________________________' as key
SELECT mode, hex(encrypt(mode, toNullable('Hello World!'), key));
WITH 'aes-256-gcm' as mode, 'test_key________________________' as key, 'test_iv_____' as iv
SELECT mode, hex(encrypt(mode, toNullable('Hello World!'), key, iv));
-----------------------------------------------------------------------------------------------------------------------------------
-- decrypt both non-null and null values of Nullable(String)
SELECT 'decrypt';
WITH 'aes-256-ecb' as mode, 'test_key________________________' as key
SELECT mode, decrypt(mode, CAST(null as Nullable(String)), key);
WITH 'aes-256-gcm' as mode, 'test_key________________________' as key, 'test_iv_____' as iv
SELECT mode, decrypt(mode, CAST(null as Nullable(String)), key, iv);
WITH 'aes-256-ecb' as mode, unhex('D1B43643E1D0E9390E39BA4EAE150851') as ciphertext, 'test_key________________________' as key
SELECT mode, decrypt(mode, toNullable(ciphertext), key);
WITH 'aes-256-gcm' as mode, unhex('219E6478A1A3BB5B686DA4BAD70323F192EFEDCCBBD6F49E78A7E2F6') as ciphertext, 'test_key________________________' as key, 'test_iv_____' as iv
SELECT mode, decrypt(mode, toNullable(ciphertext), key, iv);