Skip to content

Commit d680e0c

Browse files
rschu1zerschu1ze
authored
Merge pull request ClickHouse#78727 from rschu1ze/openssl-consistent-error-handling
More consistent error handling of OpenSSL calls
2 parents 9df9133 + 3834937 commit d680e0c

File tree

8 files changed

+130
-138
lines changed

8 files changed

+130
-138
lines changed

‎src/Access/AuthenticationData.cpp‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -549,12 +549,8 @@ AuthenticationData AuthenticationData::fromAST(const ASTAuthenticationData & que
549549
#if USE_SSL
550550
///random generator FIPS complaint
551551
uint8_t key[32];
552-
if (RAND_bytes(key, sizeof(key)) != 1)
553-
{
554-
char buf[512] = {0};
555-
ERR_error_string_n(ERR_get_error(), buf, sizeof(buf));
556-
throw Exception(ErrorCodes::OPENSSL_ERROR, "Cannot generate salt for password. OpenSSL {}", buf);
557-
}
552+
if (!RAND_bytes(key, sizeof(key)))
553+
throw Exception(ErrorCodes::OPENSSL_ERROR, "RAND_bytes failed: {}", getOpenSSLErrors());
558554

559555
String salt;
560556
salt.resize(sizeof(key) * 2);
@@ -577,12 +573,8 @@ AuthenticationData AuthenticationData::fromAST(const ASTAuthenticationData & que
577573
#if USE_SSL
578574
///random generator FIPS complaint
579575
uint8_t key[32];
580-
if (RAND_bytes(key, sizeof(key)) != 1)
581-
{
582-
char buf[512] = {0};
583-
ERR_error_string_n(ERR_get_error(), buf, sizeof(buf));
584-
throw Exception(ErrorCodes::OPENSSL_ERROR, "Cannot generate salt for password. OpenSSL {}", buf);
585-
}
576+
if (!RAND_bytes(key, sizeof(key)))
577+
throw Exception(ErrorCodes::OPENSSL_ERROR, "RAND_bytes failed: {}", getOpenSSLErrors());
586578

587579
String salt;
588580
salt.resize(sizeof(key) * 2);

‎src/Compression/CompressionCodecEncrypted.cpp‎

Lines changed: 52 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <Parsers/IAST.h>
77
#include <base/types.h>
88
#include <Common/Exception.h>
9+
#include <Common/OpenSSLHelpers.h>
910
#include <Common/logger_useful.h>
1011
#include <Common/safe_cast.h>
1112
#include "config.h"
@@ -92,14 +93,6 @@ UInt64 methodKeySize(EncryptionMethod Method)
9293
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown encryption method. Got {}", getMethodName(Method));
9394
}
9495

95-
/// Get human-readable string representation of last error
96-
std::string lastErrorString()
97-
{
98-
std::array<char, 1024> buffer = {};
99-
ERR_error_string_n(ERR_get_error(), buffer.data(), buffer.size());
100-
return std::string(buffer.data());
101-
}
102-
10396
/// Get encryption/decryption algorithms.
10497
const char * getMethod(EncryptionMethod Method)
10598
{
@@ -136,49 +129,49 @@ size_t encrypt(std::string_view plaintext, char * ciphertext_and_tag, Encryption
136129
EVP_CIPHER * cipher;
137130

138131
ctx = EVP_CIPHER_CTX_new();
139-
if (ctx == nullptr)
140-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
132+
if (!ctx)
133+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_CIPHER_CTX_new failed: {}", getOpenSSLErrors());
141134

142135
try
143136
{
144137
cipher = EVP_CIPHER_fetch(nullptr, getMethod(method), nullptr);
145-
if (cipher == nullptr)
146-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
138+
if (!cipher)
139+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_CIPHER_fetch failed: {}", getOpenSSLErrors());
147140

148-
if (int ok = EVP_EncryptInit_ex(ctx, cipher, nullptr, nullptr, nullptr); ok == 0)
149-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
141+
if (!EVP_EncryptInit_ex(ctx, cipher, nullptr, nullptr, nullptr))
142+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_EncryptInit_ex failed: {}", getOpenSSLErrors());
150143

151-
if (int ok = EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, static_cast<int32_t>(nonce.size()), nullptr); ok == 0)
152-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
144+
if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, static_cast<int32_t>(nonce.size()), nullptr))
145+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_CIPHER_CTX_ctrl failed: {}", getOpenSSLErrors());
153146

154-
if (int ok = EVP_EncryptInit_ex(ctx, nullptr, nullptr,
155-
reinterpret_cast<const uint8_t*>(key.data()),
156-
reinterpret_cast<const uint8_t *>(nonce.data())); ok == 0)
157-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
147+
if (!EVP_EncryptInit_ex(ctx, nullptr, nullptr,
148+
reinterpret_cast<const uint8_t*>(key.data()),
149+
reinterpret_cast<const uint8_t *>(nonce.data())))
150+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_EncryptInit_ex failed: {}", getOpenSSLErrors());
158151

159-
if (int ok = EVP_EncryptUpdate(ctx,
160-
reinterpret_cast<uint8_t *>(ciphertext_and_tag),
161-
&out_len,
162-
reinterpret_cast<const uint8_t *>(plaintext.data()),
163-
static_cast<int32_t>(plaintext.size())); ok == 0)
164-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
152+
if (!EVP_EncryptUpdate(ctx,
153+
reinterpret_cast<uint8_t *>(ciphertext_and_tag),
154+
&out_len,
155+
reinterpret_cast<const uint8_t *>(plaintext.data()),
156+
static_cast<int32_t>(plaintext.size())))
157+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_EncryptUpdate failed: {}", getOpenSSLErrors());
165158

166159
__msan_unpoison(ciphertext_and_tag, out_len); /// OpenSSL uses assembly which evades msan's analysis
167160

168161
ciphertext_len = out_len;
169162

170-
if (int ok = EVP_EncryptFinal_ex(ctx,
171-
reinterpret_cast<uint8_t *>(ciphertext_and_tag) + out_len,
172-
reinterpret_cast<int32_t *>(&out_len)); ok == 0)
173-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
163+
if (!EVP_EncryptFinal_ex(ctx,
164+
reinterpret_cast<uint8_t *>(ciphertext_and_tag) + out_len,
165+
reinterpret_cast<int32_t *>(&out_len)))
166+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_EncryptFinal_ex failed: {}", getOpenSSLErrors());
174167

175168
__msan_unpoison(ciphertext_and_tag, out_len); /// OpenSSL uses assembly which evades msan's analysis
176169

177170
ciphertext_len += out_len;
178171

179172
/// Get the tag
180-
if (int ok = EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, tag_size, reinterpret_cast<uint8_t *>(ciphertext_and_tag) + plaintext.size()); ok == 0)
181-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
173+
if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, tag_size, reinterpret_cast<uint8_t *>(ciphertext_and_tag) + plaintext.size()))
174+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_CIPHER_CTX_ctrl failed: {}", getOpenSSLErrors());
182175
}
183176
catch (...)
184177
{
@@ -203,47 +196,47 @@ size_t decrypt(std::string_view ciphertext, char * plaintext, EncryptionMethod m
203196
EVP_CIPHER * cipher;
204197

205198
ctx = EVP_CIPHER_CTX_new();
206-
if (ctx == nullptr)
207-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
199+
if (!ctx)
200+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_CIPHER_CTX_new failed: {}", getOpenSSLErrors());
208201

209202
try
210203
{
211204
cipher = EVP_CIPHER_fetch(nullptr, getMethod(method), nullptr);
212-
if (cipher == nullptr)
213-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
205+
if (!cipher)
206+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_CIPHER_fetch failed: {}", getOpenSSLErrors());
214207

215-
if (int ok = EVP_DecryptInit_ex(ctx, cipher, nullptr, nullptr, nullptr); ok == 0)
216-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
208+
if (!EVP_DecryptInit_ex(ctx, cipher, nullptr, nullptr, nullptr))
209+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_DecryptInit_ex failed: {}", getOpenSSLErrors());
217210

218-
if (int ok = EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, static_cast<int32_t>(nonce.size()), nullptr); ok == 0)
219-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
211+
if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, static_cast<int32_t>(nonce.size()), nullptr))
212+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_CIPHER_CTX_ctrl failed: {}", getOpenSSLErrors());
220213

221-
if (int ok = EVP_DecryptInit_ex(ctx, nullptr, nullptr,
222-
reinterpret_cast<const uint8_t*>(key.data()),
223-
reinterpret_cast<const uint8_t *>(nonce.data())); ok == 0)
224-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
214+
if (!EVP_DecryptInit_ex(ctx, nullptr, nullptr,
215+
reinterpret_cast<const uint8_t*>(key.data()),
216+
reinterpret_cast<const uint8_t *>(nonce.data())))
217+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_DecryptInit_ex failed: {}", getOpenSSLErrors());
225218

226-
if (int ok = EVP_CIPHER_CTX_ctrl(ctx,
227-
EVP_CTRL_GCM_SET_TAG,
228-
tag_size,
229-
reinterpret_cast<uint8_t *>(const_cast<char *>(ciphertext.data())) + ciphertext.size() - tag_size); ok == 0)
230-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
219+
if (!EVP_CIPHER_CTX_ctrl(ctx,
220+
EVP_CTRL_GCM_SET_TAG,
221+
tag_size,
222+
reinterpret_cast<uint8_t *>(const_cast<char *>(ciphertext.data())) + ciphertext.size() - tag_size))
223+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_CIPHER_CTX_ctrl failed: {}", getOpenSSLErrors());
231224

232-
if (int ok = EVP_DecryptUpdate(ctx,
233-
reinterpret_cast<uint8_t *>(plaintext),
234-
reinterpret_cast<int32_t *>(&out_len),
235-
reinterpret_cast<const uint8_t *>(ciphertext.data()),
236-
static_cast<int32_t>(ciphertext.size()) - tag_size); ok == 0)
237-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
225+
if (!EVP_DecryptUpdate(ctx,
226+
reinterpret_cast<uint8_t *>(plaintext),
227+
reinterpret_cast<int32_t *>(&out_len),
228+
reinterpret_cast<const uint8_t *>(ciphertext.data()),
229+
static_cast<int32_t>(ciphertext.size()) - tag_size))
230+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_DecryptUpdate failed: {}", getOpenSSLErrors());
238231

239232
__msan_unpoison(plaintext, out_len); /// OpenSSL uses assembly which evades msan's analysis
240233

241234
plaintext_len = out_len;
242235

243-
if (int ok = EVP_DecryptFinal_ex(ctx,
244-
reinterpret_cast<uint8_t *>(plaintext) + out_len,
245-
reinterpret_cast<int32_t *>(&out_len)); ok == 0)
246-
throw Exception::createDeprecated(lastErrorString(), ErrorCodes::OPENSSL_ERROR);
236+
if (!EVP_DecryptFinal_ex(ctx,
237+
reinterpret_cast<uint8_t *>(plaintext) + out_len,
238+
reinterpret_cast<int32_t *>(&out_len)))
239+
throw Exception(ErrorCodes::OPENSSL_ERROR, "EVP_DecryptFinal_ex failed: {}", getOpenSSLErrors());
247240

248241
__msan_unpoison(plaintext, out_len); /// OpenSSL uses assembly which evades msan's analysis
249242
}

‎src/Functions/FunctionsAES.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include <Functions/FunctionsAES.h>
22
#include <Interpreters/Context.h>
3+
#include <Common/OpenSSLHelpers.h>
34

45
#if USE_SSL
56

@@ -16,12 +17,11 @@ namespace ErrorCodes
1617
extern const int OPENSSL_ERROR;
1718
}
1819

19-
2020
namespace OpenSSLDetails
2121
{
2222
void onError(std::string error_message)
2323
{
24-
throw Exception(ErrorCodes::OPENSSL_ERROR, "{}. OpenSSL error code: {}", error_message, ERR_get_error());
24+
throw Exception(ErrorCodes::OPENSSL_ERROR, "{} failed: {}", error_message, getOpenSSLErrors());
2525
}
2626

2727
StringRef foldEncryptionKeyInMySQLCompatitableMode(size_t cipher_key_size, StringRef key, std::array<char, EVP_MAX_KEY_LENGTH> & folded_key)

0 commit comments

Comments
 (0)