Skip to content

Commit

Permalink
Update EVP cipher APIs to gracefully handle null EVP_CIPHER_CTX. Add …
Browse files Browse the repository at this point in the history 
…new safety macro GUARD_PTR
  • Loading branch information
@andrewhop
andrewhop committed Mar 18, 2024
1 parent 32143aa commit 00c7e05
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 5 deletions.
26 changes: 23 additions & 3 deletions crypto/cipher_extra/cipher_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1047,8 +1047,8 @@ TEST(CipherTest, GCMIncrementingIV) {
bool enc) {
// Make a reference ciphertext.
bssl::ScopedEVP_CIPHER_CTX ref;
ASSERT_TRUE(EVP_EncryptInit_ex(ref.get(), kCipher, /*impl=*/nullptr,
kKey, /*iv=*/nullptr));
ASSERT_TRUE(EVP_EncryptInit_ex(ref.get(), kCipher, /*impl=*/nullptr, kKey,
/*iv=*/nullptr));
ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ref.get(), EVP_CTRL_AEAD_SET_IVLEN,
static_cast<int>(iv.size()), nullptr));
ASSERT_TRUE(EVP_EncryptInit_ex(ref.get(), /*cipher=*/nullptr,
Expand Down Expand Up @@ -1328,7 +1328,7 @@ TEST(CipherTest, GCMIncrementingIV) {
ASSERT_NO_FATAL_FAILURE(expect_iv(ctx.get(), iv, /*enc=*/true));
}

{
{
// Same as above, but with a larger IV.
const uint8_t kFixedIV[8] = {1, 2, 3, 4, 5, 6, 7, 8};
bssl::ScopedEVP_CIPHER_CTX ctx;
Expand Down Expand Up @@ -1357,3 +1357,23 @@ TEST(CipherTest, GCMIncrementingIV) {
ASSERT_NO_FATAL_FAILURE(expect_iv(ctx.get(), iv, /*enc=*/true));
}
}

#define CHECK_ERROR(function, err) \
ERR_clear_error(); \
EXPECT_FALSE(function); \
EXPECT_EQ(err, ERR_GET_REASON(ERR_peek_last_error()));

TEST(CipherTest, Empty_EVP_CIPHER_CTX_V1187459157) {
int in_len = 10;
std::vector<uint8_t> in_vec(in_len);
int out_len = in_len + 256;
std::vector<uint8_t> out_vec(out_len);

CHECK_ERROR(EVP_EncryptUpdate(nullptr, out_vec.data(), &out_len, in_vec.data(), in_len), ERR_R_PASSED_NULL_PARAMETER);

bssl::UniquePtr<EVP_CIPHER_CTX> ctx(EVP_CIPHER_CTX_new());
CHECK_ERROR(EVP_EncryptUpdate(ctx.get(), out_vec.data(), &out_len, in_vec.data(), in_len), ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
CHECK_ERROR(EVP_EncryptFinal(ctx.get(), out_vec.data(), &out_len), ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
CHECK_ERROR(EVP_DecryptUpdate(ctx.get(), out_vec.data(), &out_len, in_vec.data(), in_len), ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
CHECK_ERROR(EVP_DecryptFinal(ctx.get(), out_vec.data(), &out_len), ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
}
12 changes: 10 additions & 2 deletions crypto/fipsmodule/cipher/cipher.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@

void EVP_CIPHER_CTX_init(EVP_CIPHER_CTX *ctx) {
OPENSSL_memset(ctx, 0, sizeof(EVP_CIPHER_CTX));
ctx->poisoned = 1;
}

EVP_CIPHER_CTX *EVP_CIPHER_CTX_new(void) {
Expand Down Expand Up @@ -255,6 +256,7 @@ static int block_remainder(const EVP_CIPHER_CTX *ctx, int len) {

int EVP_EncryptUpdate(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len,
const uint8_t *in, int in_len) {
GUARD_PTR(ctx);
if (ctx->poisoned) {
OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
return 0;
Expand All @@ -266,6 +268,7 @@ int EVP_EncryptUpdate(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len,

// Ciphers that use blocks may write up to |bl| extra bytes. Ensure the output
// does not overflow |*out_len|.
GUARD_PTR(ctx->cipher);
int bl = ctx->cipher->block_size;
if (bl > 1 && in_len > INT_MAX - bl) {
OPENSSL_PUT_ERROR(CIPHER, ERR_R_OVERFLOW);
Expand Down Expand Up @@ -347,12 +350,13 @@ int EVP_EncryptUpdate(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len,
int EVP_EncryptFinal_ex(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len) {
int n;
unsigned int i, b, bl;
GUARD_PTR(ctx);

if (ctx->poisoned) {
OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
return 0;
}

GUARD_PTR(ctx->cipher);
if (ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_CIPHER) {
// When EVP_CIPH_FLAG_CUSTOM_CIPHER is set, the return value of |cipher| is
// the number of bytes written, or -1 on error. Otherwise the return value
Expand Down Expand Up @@ -398,13 +402,16 @@ int EVP_EncryptFinal_ex(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len) {

int EVP_DecryptUpdate(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len,
const uint8_t *in, int in_len) {
GUARD_PTR(ctx);
if (ctx->poisoned) {
OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
return 0;
}


// Ciphers that use blocks may write up to |bl| extra bytes. Ensure the output
// does not overflow |*out_len|.
GUARD_PTR(ctx->cipher);
unsigned int b = ctx->cipher->block_size;
if (b > 1 && in_len > INT_MAX - (int)b) {
OPENSSL_PUT_ERROR(CIPHER, ERR_R_OVERFLOW);
Expand Down Expand Up @@ -464,6 +471,7 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *out_len) {
int i, n;
unsigned int b;
*out_len = 0;
GUARD_PTR(ctx);

// |ctx->cipher->cipher| calls the static aes encryption function way under
// the hood instead of |EVP_Cipher|, so the service indicator does not need
Expand All @@ -473,7 +481,7 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *out_len) {
OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
return 0;
}

GUARD_PTR(ctx->cipher);
if (ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_CIPHER) {
i = ctx->cipher->cipher(ctx, out, NULL, 0);
if (i < 0) {
Expand Down
22 changes: 22 additions & 0 deletions crypto/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1155,6 +1155,28 @@ OPENSSL_EXPORT int OPENSSL_vasprintf_internal(char **str, const char *format,
va_list args, int system_malloc)
OPENSSL_PRINTF_FORMAT_FUNC(2, 0);


// Safety Macros

// Inspired by s2n-tls

// __AWS_LC_ENSURE checks if |cond| is true nothing happens, else |action| is called
#define __AWS_LC_ENSURE(cond, action) \
do { \
if (!(cond)) { \
action; \
} \
} while (0)

#define AWS_LC_ERROR 0
#define AWS_LC_SUCCESS 1

// RESULT_GUARD_PTR checks if |ptr|, if it is null it adds ERR_R_PASSED_NULL_PARAMETER
// to the error queue and returns 0. NOTE: this macro should only be used with
// functions that return 0 (for error) and 1 (for success).
#define GUARD_PTR(ptr) __AWS_LC_ENSURE((ptr) != NULL, OPENSSL_PUT_ERROR(CRYPTO, ERR_R_PASSED_NULL_PARAMETER); \
return AWS_LC_ERROR)

#if defined(__cplusplus)
} // extern C
#endif
Expand Down

0 comments on commit 00c7e05

Please sign in to comment.