Skip to content

Commit

Permalink
MDEV-30389 Ensure correct dlen during encryption
Browse files Browse the repository at this point in the history
This patch ensures that all direct and indirect calls to
encryption_crypt provide a `dlen` value correctly initialized to the
destination buffer length, allowing encryption plugins to verify
available space. It also adds assertions to verify related invariants.

Signed-off-by: Trevor Gross <tmgross@umich.edu>
  • Loading branch information
tgross35 authored and vuvova committed Jun 26, 2023
1 parent 01e9e39 commit b91d5bc
Show file tree
Hide file tree
Showing 17 changed files with 84 additions and 30 deletions.
5 changes: 4 additions & 1 deletion include/mysql/plugin_audit.h.pp
Expand Up @@ -57,10 +57,13 @@
{
void *ctx= alloca(encryption_handler.encryption_ctx_size_func((key_id),(key_version)));
int res1, res2;
unsigned int d1, d2;
unsigned int d1, d2= *dlen;
assert(*dlen >= slen);
assert((dst[*dlen - 1]= 1));
if ((res1= encryption_handler.encryption_ctx_init_func((ctx),(key),(klen),(iv),(ivlen),(flags),(key_id),(key_version))))
return res1;
res1= encryption_handler.encryption_ctx_update_func((ctx),(src),(slen),(dst),(&d1));
d2-= d1;
res2= encryption_handler.encryption_ctx_finish_func((ctx),(dst + d1),(&d2));
*dlen= d1 + d2;
return res1 ? res1 : res2;
Expand Down
5 changes: 4 additions & 1 deletion include/mysql/plugin_auth.h.pp
Expand Up @@ -57,10 +57,13 @@
{
void *ctx= alloca(encryption_handler.encryption_ctx_size_func((key_id),(key_version)));
int res1, res2;
unsigned int d1, d2;
unsigned int d1, d2= *dlen;
assert(*dlen >= slen);
assert((dst[*dlen - 1]= 1));
if ((res1= encryption_handler.encryption_ctx_init_func((ctx),(key),(klen),(iv),(ivlen),(flags),(key_id),(key_version))))
return res1;
res1= encryption_handler.encryption_ctx_update_func((ctx),(src),(slen),(dst),(&d1));
d2-= d1;
res2= encryption_handler.encryption_ctx_finish_func((ctx),(dst + d1),(&d2));
*dlen= d1 + d2;
return res1 ? res1 : res2;
Expand Down
5 changes: 4 additions & 1 deletion include/mysql/plugin_data_type.h.pp
Expand Up @@ -57,10 +57,13 @@
{
void *ctx= alloca(encryption_handler.encryption_ctx_size_func((key_id),(key_version)));
int res1, res2;
unsigned int d1, d2;
unsigned int d1, d2= *dlen;
assert(*dlen >= slen);
assert((dst[*dlen - 1]= 1));
if ((res1= encryption_handler.encryption_ctx_init_func((ctx),(key),(klen),(iv),(ivlen),(flags),(key_id),(key_version))))
return res1;
res1= encryption_handler.encryption_ctx_update_func((ctx),(src),(slen),(dst),(&d1));
d2-= d1;
res2= encryption_handler.encryption_ctx_finish_func((ctx),(dst + d1),(&d2));
*dlen= d1 + d2;
return res1 ? res1 : res2;
Expand Down
6 changes: 4 additions & 2 deletions include/mysql/plugin_encryption.h
Expand Up @@ -96,8 +96,11 @@ struct st_mariadb_encryption
/**
processes (encrypts or decrypts) a chunk of data
writes the output to th dst buffer. note that it might write
writes the output to the dst buffer. note that it might write
more bytes that were in the input. or less. or none at all.
dlen points to the starting lenght of the output buffer. Upon return, it
should be set to the number of bytes written.
*/
int (*crypt_ctx_update)(void *ctx, const unsigned char* src, unsigned int slen,
unsigned char* dst, unsigned int* dlen);
Expand All @@ -123,4 +126,3 @@ struct st_mariadb_encryption
}
#endif
#endif

5 changes: 4 additions & 1 deletion include/mysql/plugin_encryption.h.pp
Expand Up @@ -57,10 +57,13 @@
{
void *ctx= alloca(encryption_handler.encryption_ctx_size_func((key_id),(key_version)));
int res1, res2;
unsigned int d1, d2;
unsigned int d1, d2= *dlen;
assert(*dlen >= slen);
assert((dst[*dlen - 1]= 1));
if ((res1= encryption_handler.encryption_ctx_init_func((ctx),(key),(klen),(iv),(ivlen),(flags),(key_id),(key_version))))
return res1;
res1= encryption_handler.encryption_ctx_update_func((ctx),(src),(slen),(dst),(&d1));
d2-= d1;
res2= encryption_handler.encryption_ctx_finish_func((ctx),(dst + d1),(&d2));
*dlen= d1 + d2;
return res1 ? res1 : res2;
Expand Down
5 changes: 4 additions & 1 deletion include/mysql/plugin_ftparser.h.pp
Expand Up @@ -57,10 +57,13 @@
{
void *ctx= alloca(encryption_handler.encryption_ctx_size_func((key_id),(key_version)));
int res1, res2;
unsigned int d1, d2;
unsigned int d1, d2= *dlen;
assert(*dlen >= slen);
assert((dst[*dlen - 1]= 1));
if ((res1= encryption_handler.encryption_ctx_init_func((ctx),(key),(klen),(iv),(ivlen),(flags),(key_id),(key_version))))
return res1;
res1= encryption_handler.encryption_ctx_update_func((ctx),(src),(slen),(dst),(&d1));
d2-= d1;
res2= encryption_handler.encryption_ctx_finish_func((ctx),(dst + d1),(&d2));
*dlen= d1 + d2;
return res1 ? res1 : res2;
Expand Down
5 changes: 4 additions & 1 deletion include/mysql/plugin_function.h.pp
Expand Up @@ -57,10 +57,13 @@
{
void *ctx= alloca(encryption_handler.encryption_ctx_size_func((key_id),(key_version)));
int res1, res2;
unsigned int d1, d2;
unsigned int d1, d2= *dlen;
assert(*dlen >= slen);
assert((dst[*dlen - 1]= 1));
if ((res1= encryption_handler.encryption_ctx_init_func((ctx),(key),(klen),(iv),(ivlen),(flags),(key_id),(key_version))))
return res1;
res1= encryption_handler.encryption_ctx_update_func((ctx),(src),(slen),(dst),(&d1));
d2-= d1;
res2= encryption_handler.encryption_ctx_finish_func((ctx),(dst + d1),(&d2));
*dlen= d1 + d2;
return res1 ? res1 : res2;
Expand Down
5 changes: 4 additions & 1 deletion include/mysql/plugin_password_validation.h.pp
Expand Up @@ -57,10 +57,13 @@
{
void *ctx= alloca(encryption_handler.encryption_ctx_size_func((key_id),(key_version)));
int res1, res2;
unsigned int d1, d2;
unsigned int d1, d2= *dlen;
assert(*dlen >= slen);
assert((dst[*dlen - 1]= 1));
if ((res1= encryption_handler.encryption_ctx_init_func((ctx),(key),(klen),(iv),(ivlen),(flags),(key_id),(key_version))))
return res1;
res1= encryption_handler.encryption_ctx_update_func((ctx),(src),(slen),(dst),(&d1));
d2-= d1;
res2= encryption_handler.encryption_ctx_finish_func((ctx),(dst + d1),(&d2));
*dlen= d1 + d2;
return res1 ? res1 : res2;
Expand Down
14 changes: 12 additions & 2 deletions include/mysql/service_encryption.h
Expand Up @@ -36,6 +36,9 @@
#ifdef __cplusplus
extern "C" {
#endif
#ifndef MYSQL_ABI_CHECK
#include <assert.h>
#endif

/* returned from encryption_key_get_latest_version() */
#define ENCRYPTION_KEY_VERSION_INVALID (~(unsigned int)0)
Expand Down Expand Up @@ -101,6 +104,7 @@ static inline unsigned int encryption_key_version_exists(unsigned int id, unsign
return encryption_key_get(id, version, NULL, &unused) != ENCRYPTION_KEY_VERSION_INVALID;
}

/* main entrypoint to perform encryption or decryption */
static inline int encryption_crypt(const unsigned char* src, unsigned int slen,
unsigned char* dst, unsigned int* dlen,
const unsigned char* key, unsigned int klen,
Expand All @@ -109,11 +113,18 @@ static inline int encryption_crypt(const unsigned char* src, unsigned int slen,
{
void *ctx= alloca(encryption_ctx_size(key_id, key_version));
int res1, res2;
unsigned int d1, d2;
unsigned int d1, d2= *dlen;

// Verify dlen is initialized properly. See MDEV-30389
assert(*dlen >= slen);
assert((dst[*dlen - 1]= 1));

if ((res1= encryption_ctx_init(ctx, key, klen, iv, ivlen, flags, key_id, key_version)))
return res1;
res1= encryption_ctx_update(ctx, src, slen, dst, &d1);
d2-= d1;
res2= encryption_ctx_finish(ctx, dst + d1, &d2);

*dlen= d1 + d2;
return res1 ? res1 : res2;
}
Expand All @@ -124,4 +135,3 @@ static inline int encryption_crypt(const unsigned char* src, unsigned int slen,

#define MYSQL_SERVICE_ENCRYPTION_INCLUDED
#endif

16 changes: 15 additions & 1 deletion sql/encryption.cc
Expand Up @@ -187,6 +187,11 @@ static uint scheme_get_key(st_encryption_scheme *scheme,
return rc;
}

/** Run encryption or decryption on a block.
* `i32_1`, `i32_2`, and `i64` are used to create the initialization vector
* @invariant `src` must be valid for `slen`
* @invariant `dst` is valid for `*dlen`, `*dlen` is initialized
*/
int do_crypt(const unsigned char* src, unsigned int slen,
unsigned char* dst, unsigned int* dlen,
struct st_encryption_scheme *scheme,
Expand Down Expand Up @@ -224,6 +229,11 @@ int do_crypt(const unsigned char* src, unsigned int slen,
iv, sizeof(iv), flag, scheme->key_id, key_version);
}

/** Encrypt a block.
* `i32_1`, `i32_2`, and `i64` are used to create the initialization vector
* @invariant `src` is valid for `slen`
* @invariant `dst` is valid for `*dlen`, `*dlen` is initialized
*/
int encryption_scheme_encrypt(const unsigned char* src, unsigned int slen,
unsigned char* dst, unsigned int* dlen,
struct st_encryption_scheme *scheme,
Expand All @@ -234,7 +244,11 @@ int encryption_scheme_encrypt(const unsigned char* src, unsigned int slen,
i32_2, i64, ENCRYPTION_FLAG_NOPAD | ENCRYPTION_FLAG_ENCRYPT);
}


/** Decrypt a block.
* `i32_1`, `i32_2`, and `i64` are used to create the initialization vector
* @invariant `src` is valid for `slen`
* @invariant `dst` is valid for `*dlen`, `*dlen` is initialized
*/
int encryption_scheme_decrypt(const unsigned char* src, unsigned int slen,
unsigned char* dst, unsigned int* dlen,
struct st_encryption_scheme *scheme,
Expand Down
2 changes: 1 addition & 1 deletion sql/log.cc
Expand Up @@ -5662,7 +5662,7 @@ bool MYSQL_BIN_LOG::write_event_buffer(uchar* buf, uint len)
{
DBUG_ASSERT(crypto.scheme == 1);

uint elen;
uint elen= len - 4;
uchar iv[BINLOG_IV_LENGTH];

ebuf= (uchar*)my_safe_alloca(len);
Expand Down
2 changes: 1 addition & 1 deletion sql/log_event.cc
Expand Up @@ -838,7 +838,7 @@ int Log_event::read_log_event(IO_CACHE* file, String* packet,
DBUG_RETURN(LOG_READ_MEM);
memcpy(newpkt, packet->ptr(), ev_offset);

uint dstlen;
uint dstlen= (uint) sz - ev_offset - 4;
uchar *src= (uchar*)packet->ptr() + ev_offset;
uchar *dst= (uchar*)newpkt + ev_offset;
memcpy(src + EVENT_LEN_OFFSET, src, 4);
Expand Down
8 changes: 5 additions & 3 deletions sql/mf_iocache_encr.cc
Expand Up @@ -85,7 +85,8 @@ static int my_b_encr_read(IO_CACHE *info, uchar *Buffer, size_t Count)

do
{
uint elength, wlength, length;
uint elength, wlength;
uint length= static_cast<uint>(info->buffer_length);
uchar iv[MY_AES_BLOCK_SIZE]= {0};

DBUG_ASSERT(pos_in_file % info->buffer_length == 0);
Expand All @@ -102,6 +103,7 @@ static int my_b_encr_read(IO_CACHE *info, uchar *Buffer, size_t Count)
}

elength= wlength - (uint)(ebuffer - wbuffer);
length= elength;
set_iv(iv, pos_in_file, crypt_data->inbuf_counter);

if (encryption_crypt(ebuffer, elength, info->buffer, &length,
Expand Down Expand Up @@ -181,8 +183,9 @@ static int my_b_encr_write(IO_CACHE *info, const uchar *Buffer, size_t Count)

do
{
uint wlength;
size_t length= MY_MIN(info->buffer_length, Count);
uint elength, wlength;
uint elength= static_cast<uint>(length);
uchar iv[MY_AES_BLOCK_SIZE]= {0};

crypt_data->inbuf_counter= crypt_data->counter;
Expand Down Expand Up @@ -272,4 +275,3 @@ int init_io_cache_encryption()
_my_b_encr_write= 0;
return 0;
}

8 changes: 4 additions & 4 deletions storage/innobase/fil/fil0crypt.cc
Expand Up @@ -445,11 +445,11 @@ static byte* fil_encrypt_buf_for_non_full_checksum(
uint srclen = size - unencrypted_bytes;
const byte* src = src_frame + header_len;
byte* dst = dst_frame + header_len;
uint32 dstlen = 0;

if (page_compressed) {
srclen = mach_read_from_2(src_frame + FIL_PAGE_DATA);
}
uint dstlen = srclen;

int rc = encryption_scheme_encrypt(src, srclen, dst, &dstlen,
crypt_data, key_version,
Expand Down Expand Up @@ -516,7 +516,7 @@ static byte* fil_encrypt_buf_for_full_crc32(
+ FIL_PAGE_FCRC32_CHECKSUM);
const byte* src = src_frame + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION;
byte* dst = dst_frame + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION;
uint dstlen = 0;
uint dstlen = srclen;

ut_a(key_version != ENCRYPTION_KEY_VERSION_INVALID);

Expand Down Expand Up @@ -647,7 +647,6 @@ static dberr_t fil_space_decrypt_full_crc32(
/* Calculate the offset where decryption starts */
const byte* src = src_frame + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION;
byte* dst = tmp_frame + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION;
uint dstlen = 0;
bool corrupted = false;
uint size = buf_page_full_crc32_size(src_frame, NULL, &corrupted);
if (UNIV_UNLIKELY(corrupted)) {
Expand All @@ -656,6 +655,7 @@ static dberr_t fil_space_decrypt_full_crc32(

uint srclen = size - (FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION
+ FIL_PAGE_FCRC32_CHECKSUM);
uint dstlen = srclen;

int rc = encryption_scheme_decrypt(src, srclen, dst, &dstlen,
crypt_data, key_version,
Expand Down Expand Up @@ -711,8 +711,8 @@ static dberr_t fil_space_decrypt_for_non_full_checksum(
/* Calculate the offset where decryption starts */
const byte* src = src_frame + header_len;
byte* dst = tmp_frame + header_len;
uint32 dstlen = 0;
uint srclen = uint(physical_size) - header_len - FIL_PAGE_DATA_END;
uint dstlen = srclen;

if (page_compressed) {
srclen = mach_read_from_2(src_frame + FIL_PAGE_DATA);
Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/include/log0crypt.h
Expand Up @@ -85,7 +85,7 @@ void log_decrypt_buf(const byte *iv, byte *buf, const byte *const end);
/** Encrypt or decrypt a temporary file block.
@param[in] src block to encrypt or decrypt
@param[in] size size of the block
@param[out] dst destination block
@param[out] dst destination block, also of length `size`
@param[in] offs offset to block
@param[in] encrypt true=encrypt; false=decrypt
@return whether the operation succeeded */
Expand Down
12 changes: 6 additions & 6 deletions storage/innobase/log/log0crypt.cc
Expand Up @@ -221,9 +221,9 @@ ATTRIBUTE_COLD bool log_decrypt(byte* buf, lsn_t lsn, ulint size)
ut_ad(LOG_CRYPT_HDR_SIZE + dst_size
== 512 - LOG_BLOCK_CHECKSUM - LOG_BLOCK_KEY);

uint dst_len;
uint dst_len = static_cast<uint>(dst_size);
int rc = encryption_crypt(
buf + LOG_CRYPT_HDR_SIZE, static_cast<uint>(dst_size),
buf + LOG_CRYPT_HDR_SIZE, dst_len,
reinterpret_cast<byte*>(dst), &dst_len,
const_cast<byte*>(info.crypt_key),
MY_AES_BLOCK_SIZE,
Expand Down Expand Up @@ -332,10 +332,10 @@ ATTRIBUTE_COLD bool log_crypt_101_read_block(byte* buf, lsn_t start_lsn)
}
found:
byte dst[512];
uint dst_len;
byte aes_ctr_iv[MY_AES_BLOCK_SIZE];

const uint src_len = 512 - LOG_BLOCK_HDR_SIZE;
uint dst_len = src_len;

ulint log_block_no = log_block_get_hdr_no(buf);

Expand Down Expand Up @@ -429,8 +429,8 @@ ATTRIBUTE_COLD bool log_crypt_read_checkpoint_buf(const byte* buf)

/** Encrypt or decrypt a temporary file block.
@param[in] src block to encrypt or decrypt
@param[in] size size of the block
@param[out] dst destination block
@param[in] size size of 'src' block
@param[out] dst destination block, also of length 'size'
@param[in] offs offset to block
@param[in] encrypt true=encrypt; false=decrypt
@return whether the operation succeeded */
Expand All @@ -441,7 +441,7 @@ bool log_tmp_block_encrypt(
uint64_t offs,
bool encrypt)
{
uint dst_len;
uint dst_len = (uint) size;
uint64_t iv[MY_AES_BLOCK_SIZE / sizeof(uint64_t)];
iv[0] = offs;
memcpy(iv + 1, tmp_iv, sizeof iv - sizeof *iv);
Expand Down
9 changes: 7 additions & 2 deletions storage/maria/ma_crypt.c
Expand Up @@ -459,7 +459,7 @@ static int ma_encrypt(MARIA_SHARE *share, MARIA_CRYPT_DATA *crypt_data,
uint *key_version)
{
int rc;
uint32 dstlen= 0; /* Must be set because of error message */
uint32 dstlen= size;

*key_version = encryption_key_get_latest_version(crypt_data->scheme.key_id);
if (*key_version == ENCRYPTION_KEY_VERSION_INVALID)
Expand All @@ -486,6 +486,9 @@ static int ma_encrypt(MARIA_SHARE *share, MARIA_CRYPT_DATA *crypt_data,
DBUG_ASSERT(!my_assert_on_error || dstlen == size);
if (! (rc == MY_AES_OK && dstlen == size))
{
if (rc != MY_AES_OK)
dstlen= 0; /* reset dstlen if failed, to match expected message */

my_errno= HA_ERR_DECRYPTION_FAILED;
my_printf_error(HA_ERR_DECRYPTION_FAILED,
"failed to encrypt '%s' rc: %d dstlen: %u size: %u\n",
Expand All @@ -503,7 +506,7 @@ static int ma_decrypt(MARIA_SHARE *share, MARIA_CRYPT_DATA *crypt_data,
uint key_version)
{
int rc;
uint32 dstlen= 0; /* Must be set because of error message */
uint32 dstlen= size;

rc= encryption_scheme_decrypt(src, size, dst, &dstlen,
&crypt_data->scheme, key_version,
Expand All @@ -513,6 +516,8 @@ static int ma_decrypt(MARIA_SHARE *share, MARIA_CRYPT_DATA *crypt_data,
DBUG_ASSERT(!my_assert_on_error || dstlen == size);
if (! (rc == MY_AES_OK && dstlen == size))
{
if (rc != MY_AES_OK)
dstlen= 0; /* reset dstlen if failed, to match expected message */
my_errno= HA_ERR_DECRYPTION_FAILED;
if (!share->silence_encryption_errors)
my_printf_error(HA_ERR_DECRYPTION_FAILED,
Expand Down

0 comments on commit b91d5bc

Please sign in to comment.