From 7b14ba63f22e6c98c1982dae8847035ad1b8e155 Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 28 Jan 2016 14:06:05 +0200 Subject: [PATCH] MDEV-8724 Assertion `rc == 0' failed in ma_decrypt on reading an Aria table Don't assert if decrypt or encrypt fails if my_assert_on_error is not set. Added failed file name if encryption/decryption fails. --- .../suite/maria/encrypt-wrong-key.result | 16 +++++ mysql-test/suite/maria/encrypt-wrong-key.test | 52 +++++++++++++++ storage/maria/ma_crypt.c | 66 +++++++++++-------- 3 files changed, 106 insertions(+), 28 deletions(-) create mode 100644 mysql-test/suite/maria/encrypt-wrong-key.result create mode 100644 mysql-test/suite/maria/encrypt-wrong-key.test diff --git a/mysql-test/suite/maria/encrypt-wrong-key.result b/mysql-test/suite/maria/encrypt-wrong-key.result new file mode 100644 index 00000000000..bc22481296d --- /dev/null +++ b/mysql-test/suite/maria/encrypt-wrong-key.result @@ -0,0 +1,16 @@ +call mtr.add_suppression("file_key_management"); +call mtr.add_suppression("System key id 1 is missing"); +call mtr.add_suppression("Unknown key id 1"); +call mtr.add_suppression("Failed to decrypt"); +CREATE TABLE t1 (i INT, KEY(i)) ENGINE=Aria; +INSERT INTO t1 VALUES (1); +repair table t1; +Table Op Msg_type Msg_text +test.t1 repair info Wrong CRC on datapage at 1 +test.t1 repair warning Number of rows changed from 1 to 0 +test.t1 repair status OK +INSERT INTO t1 VALUES (2); +select * from t1; +ERROR HY000: failed to decrypt './test/t1' rc: -1 dstlen: 0 size: 8172 + +drop table t1; diff --git a/mysql-test/suite/maria/encrypt-wrong-key.test b/mysql-test/suite/maria/encrypt-wrong-key.test new file mode 100644 index 00000000000..b744c5fac17 --- /dev/null +++ b/mysql-test/suite/maria/encrypt-wrong-key.test @@ -0,0 +1,52 @@ +# +# Test what happens if one removes a decryption key for Aria +# + +call mtr.add_suppression("file_key_management"); +call mtr.add_suppression("System key id 1 is missing"); +call mtr.add_suppression("Unknown key id 1"); +call mtr.add_suppression("Failed to decrypt"); + +--exec echo "wait" > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +--shutdown_server +--source include/wait_until_disconnected.inc + +--write_file $MYSQLTEST_VARDIR/keys1.txt +1;770A8A65DA156D24EE2A093277530142 +EOF + +--exec echo "restart:--aria-encrypt-tables=1 --plugin-load-add=file_key_management.so --file-key-management --file-key-management-filename=$MYSQLTEST_VARDIR/keys1.txt" > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +--enable_reconnect +--source include/wait_until_connected_again.inc + +CREATE TABLE t1 (i INT, KEY(i)) ENGINE=Aria; +INSERT INTO t1 VALUES (1); + +--exec echo "wait" > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +--shutdown_server +--source include/wait_until_disconnected.inc + +--write_file $MYSQLTEST_VARDIR/keys2.txt +2;770A8A65DA156D24EE2A093277530143 +EOF + +--exec echo "restart:--aria-encrypt-tables=1 --plugin-load-add=file_key_management.so --file-key-management --file-key-management-filename=$MYSQLTEST_VARDIR/keys2.txt" > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +--enable_reconnect +--source include/wait_until_connected_again.inc + +repair table t1; + +INSERT INTO t1 VALUES (2); + +--exec echo "wait" > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +--shutdown_server +--source include/wait_until_disconnected.inc + +--exec echo "restart:--aria-encrypt-tables=1 --plugin-load-add=file_key_management.so --file-key-management --file-key-management-filename=$MYSQLTEST_VARDIR/keys1.txt" > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +--enable_reconnect +--source include/wait_until_connected_again.inc + +--replace_result \\ / +--error 192 +select * from t1; +drop table t1; diff --git a/storage/maria/ma_crypt.c b/storage/maria/ma_crypt.c index 55fa8430d22..da82a1a37c8 100644 --- a/storage/maria/ma_crypt.c +++ b/storage/maria/ma_crypt.c @@ -182,10 +182,10 @@ ma_crypt_read(MARIA_SHARE* share, uchar *buff) return buff + 2 + iv_length; } -static int ma_encrypt(MARIA_CRYPT_DATA *, const uchar *, uchar *, uint, - uint, LSN, uint *); -static int ma_decrypt(MARIA_CRYPT_DATA *, const uchar *, uchar *, uint, - uint, LSN, uint); +static int ma_encrypt(MARIA_SHARE *, MARIA_CRYPT_DATA *, const uchar *, + uchar *, uint, uint, LSN, uint *); +static int ma_decrypt(MARIA_SHARE *, MARIA_CRYPT_DATA *, const uchar *, + uchar *, uint, uint, LSN, uint); static my_bool ma_crypt_pre_read_hook(PAGECACHE_IO_HOOK_ARGS *args) { @@ -227,7 +227,7 @@ static my_bool ma_crypt_data_post_read_hook(int res, /* 1 - copy head */ memcpy(dst, src, head); /* 2 - decrypt page */ - res= ma_decrypt(share->crypt_data, + res= ma_decrypt(share, share->crypt_data, src + head, dst + head, size - (head + tail), pageno, lsn, key_version); /* 3 - copy tail */ @@ -294,9 +294,9 @@ static my_bool ma_crypt_data_pre_write_hook(PAGECACHE_IO_HOOK_ARGS *args) /* 1 - copy head */ memcpy(dst, src, head); /* 2 - encrypt page */ - if (ma_encrypt(share->crypt_data, + if (ma_encrypt(share, share->crypt_data, src + head, dst + head, size - (head + tail), pageno, lsn, - &key_version)) + &key_version)) return 1; /* 3 - copy tail */ memcpy(dst + size - tail, src + size - tail, tail); @@ -361,7 +361,7 @@ static my_bool ma_crypt_index_post_read_hook(int res, /* 1 - copy head */ memcpy(dst, src, head); /* 2 - decrypt page */ - res= ma_decrypt(share->crypt_data, + res= ma_decrypt(share, share->crypt_data, src + head, dst + head, size, pageno, lsn, key_version); /* 3 - copy tail */ memcpy(dst + block_size - tail, src + block_size - tail, tail); @@ -414,7 +414,7 @@ static my_bool ma_crypt_index_pre_write_hook(PAGECACHE_IO_HOOK_ARGS *args) /* 1 - copy head */ memcpy(dst, src, head); /* 2 - encrypt page */ - if (ma_encrypt(share->crypt_data, + if (ma_encrypt(share, share->crypt_data, src + head, dst + head, size, pageno, lsn, &key_version)) return 1; /* 3 - copy tail */ @@ -444,19 +444,26 @@ void ma_crypt_set_index_pagecache_callbacks(PAGECACHE_FILE *file, file->post_write_hook= ma_crypt_post_write_hook; } -static int ma_encrypt(MARIA_CRYPT_DATA *crypt_data, - const uchar *src, uchar *dst, uint size, - uint pageno, LSN lsn, - uint *key_version) +static int ma_encrypt(MARIA_SHARE *share, MARIA_CRYPT_DATA *crypt_data, + const uchar *src, uchar *dst, uint size, + uint pageno, LSN lsn, + uint *key_version) { int rc; - uint32 dstlen; + uint32 dstlen= 0; /* Must be set because of error message */ *key_version = encryption_key_get_latest_version(crypt_data->scheme.key_id); if (*key_version == ENCRYPTION_KEY_VERSION_INVALID) { - my_printf_error(HA_ERR_GENERIC, "Unknown key id %u. Can't continue!", - MYF(ME_FATALERROR|ME_NOREFRESH), crypt_data->scheme.key_id); + /* + We use this error for both encryption and decryption, as in normal + cases it should be impossible to get an error here. + */ + my_errno= HA_ERR_DECRYPTION_FAILED; + my_printf_error(HA_ERR_DECRYPTION_FAILED, + "Unknown key id %u. Can't continue!", + MYF(ME_FATALERROR|ME_NOREFRESH), + crypt_data->scheme.key_id); return 1; } @@ -464,40 +471,43 @@ static int ma_encrypt(MARIA_CRYPT_DATA *crypt_data, &crypt_data->scheme, *key_version, crypt_data->space, pageno, lsn); - DBUG_ASSERT(rc == MY_AES_OK); - DBUG_ASSERT(dstlen == size); + /* The following can only fail if the encryption key is wrong */ + DBUG_ASSERT(!my_assert_on_error || rc == MY_AES_OK); + DBUG_ASSERT(!my_assert_on_error || dstlen == size); if (! (rc == MY_AES_OK && dstlen == size)) { - my_printf_error(HA_ERR_GENERIC, - "failed to encrypt! rc: %d, dstlen: %u size: %u\n", + my_errno= HA_ERR_DECRYPTION_FAILED; + my_printf_error(HA_ERR_DECRYPTION_FAILED, + "failed to encrypt '%s' rc: %d dstlen: %u size: %u\n", MYF(ME_FATALERROR|ME_NOREFRESH), - rc, dstlen, size); + share->open_file_name.str, rc, dstlen, size); return 1; } return 0; } -static int ma_decrypt(MARIA_CRYPT_DATA *crypt_data, +static int ma_decrypt(MARIA_SHARE *share, MARIA_CRYPT_DATA *crypt_data, const uchar *src, uchar *dst, uint size, uint pageno, LSN lsn, uint key_version) { int rc; - uint32 dstlen; + uint32 dstlen= 0; /* Must be set because of error message */ rc= encryption_scheme_decrypt(src, size, dst, &dstlen, &crypt_data->scheme, key_version, crypt_data->space, pageno, lsn); - DBUG_ASSERT(rc == MY_AES_OK); - DBUG_ASSERT(dstlen == size); + DBUG_ASSERT(!my_assert_on_error || rc == MY_AES_OK); + DBUG_ASSERT(!my_assert_on_error || dstlen == size); if (! (rc == MY_AES_OK && dstlen == size)) { - my_printf_error(HA_ERR_GENERIC, - "failed to encrypt! rc: %d, dstlen: %u size: %u\n", + my_errno= HA_ERR_DECRYPTION_FAILED; + my_printf_error(HA_ERR_DECRYPTION_FAILED, + "failed to decrypt '%s' rc: %d dstlen: %u size: %u\n", MYF(ME_FATALERROR|ME_NOREFRESH), - rc, dstlen, size); + share->open_file_name.str, rc, dstlen, size); return 1; } return 0;