From 8c43f963882a9d5ac4e4289c8dd3dbcaeb40a0ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 17 Dec 2018 19:00:35 +0200 Subject: Follow-up to MDEV-12112: corruption in encrypted table may be overlooked The initial fix only covered a part of Mariabackup. This fix hardens InnoDB and XtraDB in a similar way, in order to reduce the probability of mistaking a corrupted encrypted page for a valid unencrypted one. This is based on work by Thirunarayanan Balathandayuthapani. fil_space_verify_crypt_checksum(): Assert that key_version!=0. Let the callers guarantee that. Now that we have this assertion, we also know that buf_page_is_zeroes() cannot hold. Also, remove all diagnostic output and related parameters, and let the relevant callers emit such messages. Last but not least, validate the post-encryption checksum according to the innodb_checksum_algorithm (only accepting one checksum for the strict variants), and no longer try to validate the page as if it was unencrypted. buf_page_is_zeroes(): Move to the compilation unit of the only callers, and declare static. xb_fil_cur_read(), buf_page_check_corrupt(): Add a condition before calling fil_space_verify_crypt_checksum(). This is a non-functional change. buf_dblwr_process(): Validate the page only as encrypted or unencrypted, but not both. --- extra/innochecksum.cc | 11 ++++++++++- extra/mariabackup/fil_cur.cc | 11 ++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) (limited to 'extra') diff --git a/extra/innochecksum.cc b/extra/innochecksum.cc index 7ad71aa8159..b3fcbbfb942 100644 --- a/extra/innochecksum.cc +++ b/extra/innochecksum.cc @@ -522,7 +522,16 @@ is_page_corrupted( normal method. */ if (is_encrypted && key_version != 0) { is_corrupted = !fil_space_verify_crypt_checksum(buf, - page_size.is_compressed() ? page_size.physical() : 0, NULL, cur_page_num); + page_size.is_compressed() ? page_size.physical() : 0); + if (is_corrupted && log_file) { + fprintf(log_file, + "Page " ULINTPF ":%llu may be corrupted;" + " key_version=" ULINTPF "\n", + space_id, cur_page_num, + mach_read_from_4( + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION + + buf)); + } } else { is_corrupted = true; } diff --git a/extra/mariabackup/fil_cur.cc b/extra/mariabackup/fil_cur.cc index 97e27fd9c79..c2693e6f616 100644 --- a/extra/mariabackup/fil_cur.cc +++ b/extra/mariabackup/fil_cur.cc @@ -351,9 +351,14 @@ read_retry: && page_no >= FSP_EXTENT_SIZE && page_no < FSP_EXTENT_SIZE * 3) { /* We ignore the doublewrite buffer pages */ - } else if (fil_space_verify_crypt_checksum( - page, cursor->zip_size, - space, page_no)) { + } else if (mach_read_from_4( + page + + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION) + && space->crypt_data + && space->crypt_data->type + != CRYPT_SCHEME_UNENCRYPTED + && fil_space_verify_crypt_checksum( + page, cursor->zip_size)) { ut_ad(mach_read_from_4(page + FIL_PAGE_SPACE_ID) == space->id); -- cgit v1.2.1