MDEV-26747 improve corruption check for encrypted tables on ALTER IMPORTbb-10.4-MDEV-26747-corruption-check

fil_space_decrypt_full_crc32()
fil_space_decrypt_for_non_full_checksum()
fil_space_decrypt(): change signature to return status via dberr_t only.
Also replace impossible condition with an assertion and prove it via
test cases.

7 files changed, 111 insertions, 73 deletions

diff --git a/mysql-test/suite/encryption/r/encryption_key_corruption.result b/mysql-test/suite/encryption/r/encryption_key_corruption.result
new file mode 100644
index 00000000000..055abd687b6
--- /dev/null
+++ b/mysql-test/suite/encryption/r/encryption_key_corruption.result
@@ -0,0 +1,13 @@
+call mtr.add_suppression("InnoDB: .*: Page 0 at offset 0 looks corrupted");
+call mtr.add_suppression("Index for table 'dst' is corrupt; try to repair it");
+call mtr.add_suppression("Page for tablespace .* is index page with id .* but that index is not found from configuration file");
+CREATE TABLE src (pk INT PRIMARY KEY, value INT) ENGINE=INNODB;
+INSERT INTO src VALUES (1, 1), (2, 2), (3, 3);
+FLUSH TABLES src FOR EXPORT;
+UNLOCK TABLES;
+DROP TABLE src;
+CREATE TABLE dst (pk INT PRIMARY KEY, value INT) ENGINE=INNODB;
+ALTER TABLE dst DISCARD TABLESPACE;
+ALTER TABLE dst IMPORT TABLESPACE;
+ERROR HY000: Index for table 'dst' is corrupt; try to repair it
+DROP TABLE dst;
diff --git a/mysql-test/suite/encryption/t/encryption_key_corruption.combinations b/mysql-test/suite/encryption/t/encryption_key_corruption.combinations
new file mode 100644
index 00000000000..ec77ac17760
--- /dev/null
+++ b/mysql-test/suite/encryption/t/encryption_key_corruption.combinations
@@ -0,0 +1,6 @@
+[crc32]
+innodb-checksum-algorithm=crc32
+[none]
+innodb-checksum-algorithm=none
+[innodb]
+innodb-checksum-algorithm=innodb
diff --git a/mysql-test/suite/encryption/t/encryption_key_corruption.opt b/mysql-test/suite/encryption/t/encryption_key_corruption.opt
new file mode 100644
index 00000000000..682d06d5732
--- /dev/null
+++ b/mysql-test/suite/encryption/t/encryption_key_corruption.opt
@@ -0,0 +1 @@
+--innodb-encrypt-tables=1
diff --git a/mysql-test/suite/encryption/t/encryption_key_corruption.test b/mysql-test/suite/encryption/t/encryption_key_corruption.test
new file mode 100644
index 00000000000..9b4bd172a3a
--- /dev/null
+++ b/mysql-test/suite/encryption/t/encryption_key_corruption.test
@@ -0,0 +1,44 @@
+--source include/have_innodb.inc
+--source include/have_example_key_management_plugin.inc
+
+call mtr.add_suppression("InnoDB: .*: Page 0 at offset 0 looks corrupted");
+call mtr.add_suppression("Index for table 'dst' is corrupt; try to repair it");
+call mtr.add_suppression("Page for tablespace .* is index page with id .* but that index is not found from configuration file");
+
+let MYSQLD_DATADIR = `SELECT @@datadir`;
+
+
+CREATE TABLE src (pk INT PRIMARY KEY, value INT) ENGINE=INNODB;
+INSERT INTO src VALUES (1, 1), (2, 2), (3, 3);
+
+FLUSH TABLES src FOR EXPORT;
+
+--copy_file $MYSQLD_DATADIR/test/src.ibd $MYSQLD_DATADIR/test/tmp.ibd
+--copy_file $MYSQLD_DATADIR/test/src.cfg $MYSQLD_DATADIR/test/tmp.cfg
+
+perl;
+use strict;
+die unless open(FILE, "+<$ENV{MYSQLD_DATADIR}/test/tmp.ibd");
+binmode FILE;
+die unless seek(FILE, 3 * 16384 + 26, 0);
+print FILE pack("N", 0x00000000);
+close(FILE);
+EOF
+
+UNLOCK TABLES;
+
+DROP TABLE src;
+
+CREATE TABLE dst (pk INT PRIMARY KEY, value INT) ENGINE=INNODB;
+ALTER TABLE dst DISCARD TABLESPACE;
+
+--copy_file $MYSQLD_DATADIR/test/tmp.ibd $MYSQLD_DATADIR/test/dst.ibd
+--copy_file $MYSQLD_DATADIR/test/tmp.cfg $MYSQLD_DATADIR/test/dst.cfg
+
+--error ER_NOT_KEYFILE
+ALTER TABLE dst IMPORT TABLESPACE;
+
+DROP TABLE dst;
+
+--remove_file $MYSQLD_DATADIR/test/tmp.ibd
+--remove_file $MYSQLD_DATADIR/test/tmp.cfg
diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc
index 81ecdbf0481..bfd129ee2da 100644
--- a/storage/innobase/fil/fil0crypt.cc
+++ b/storage/innobase/fil/fil0crypt.cc
@@ -767,24 +767,19 @@ fil_space_encrypt(
 @param[in]	crypt_data		crypt_data
 @param[in]	tmp_frame		Temporary buffer
 @param[in,out]	src_frame		Page to decrypt
-@param[out]	err			DB_SUCCESS or DB_DECRYPTION_FAILED
-@return true if page decrypted, false if not.*/
-static bool fil_space_decrypt_full_crc32(
+@return DB_SUCCESS or error */
+static dberr_t fil_space_decrypt_full_crc32(
 	ulint			space,
 	fil_space_crypt_t*	crypt_data,
 	byte*			tmp_frame,
-	byte*			src_frame,
-	dberr_t*		err)
+	byte*			src_frame)
 {
 	uint key_version = mach_read_from_4(
 		src_frame + FIL_PAGE_FCRC32_KEY_VERSION);
 	lsn_t lsn = mach_read_from_8(src_frame + FIL_PAGE_LSN);
 	uint offset = mach_read_from_4(src_frame + FIL_PAGE_OFFSET);
-	*err = DB_SUCCESS;
 
-	if (key_version == ENCRYPTION_KEY_NOT_ENCRYPTED) {
-		return false;
-	}
+	ut_a(key_version != ENCRYPTION_KEY_NOT_ENCRYPTED);
 
 	ut_ad(crypt_data);
 	ut_ad(crypt_data->is_encrypted());
@@ -798,9 +793,7 @@ static bool fil_space_decrypt_full_crc32(
 	bool corrupted = false;
 	uint size = buf_page_full_crc32_size(src_frame, NULL, &corrupted);
 	if (UNIV_UNLIKELY(corrupted)) {
-fail:
-		*err = DB_DECRYPTION_FAILED;
-		return false;
+		return DB_DECRYPTION_FAILED;
 	}
 
 	uint srclen = size - (FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION
@@ -812,7 +805,7 @@ fail:
 
 	if (rc != MY_AES_OK || dstlen != srclen) {
 		if (rc == -1) {
-			goto fail;
+			return DB_DECRYPTION_FAILED;
 		}
 
 		ib::fatal() << "Unable to decrypt data-block "
@@ -829,7 +822,7 @@ fail:
 
 	srv_stats.pages_decrypted.inc();
 
-	return true; /* page was decrypted */
+	return DB_SUCCESS; /* page was decrypted */
 }
 
 /** Decrypt a page for non full checksum format.
@@ -837,14 +830,12 @@ fail:
 @param[in]	tmp_frame		Temporary buffer
 @param[in]	physical_size		page size
 @param[in,out]	src_frame		Page to decrypt
-@param[out]	err			DB_SUCCESS or DB_DECRYPTION_FAILED
-@return true if page decrypted, false if not.*/
-static bool fil_space_decrypt_for_non_full_checksum(
+@return DB_SUCCESS or error */
+static dberr_t fil_space_decrypt_for_non_full_checksum(
 	fil_space_crypt_t*	crypt_data,
 	byte*			tmp_frame,
 	ulint			physical_size,
-	byte*			src_frame,
-	dberr_t*		err)
+	byte*			src_frame)
 {
 	ulint page_type = mach_read_from_2(src_frame+FIL_PAGE_TYPE);
 	uint key_version = mach_read_from_4(
@@ -856,12 +847,7 @@ static bool fil_space_decrypt_for_non_full_checksum(
 			src_frame + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID);
 	ib_uint64_t lsn = mach_read_from_8(src_frame + FIL_PAGE_LSN);
 
-	*err = DB_SUCCESS;
-
-	if (key_version == ENCRYPTION_KEY_NOT_ENCRYPTED) {
-		return false;
-	}
-
+	ut_a(key_version != ENCRYPTION_KEY_NOT_ENCRYPTED);
 	ut_a(crypt_data != NULL && crypt_data->is_encrypted());
 
 	/* read space & lsn */
@@ -891,8 +877,7 @@ static bool fil_space_decrypt_for_non_full_checksum(
 	if (! ((rc == MY_AES_OK) && ((ulint) dstlen == srclen))) {
 
 		if (rc == -1) {
-			*err = DB_DECRYPTION_FAILED;
-			return false;
+			return DB_DECRYPTION_FAILED;
 		}
 
 		ib::fatal() << "Unable to decrypt data-block "
@@ -917,7 +902,7 @@ static bool fil_space_decrypt_for_non_full_checksum(
 
 	srv_stats.pages_decrypted.inc();
 
-	return true; /* page was decrypted */
+	return DB_SUCCESS; /* page was decrypted */
 }
 
 /** Decrypt a page.
@@ -928,26 +913,25 @@ static bool fil_space_decrypt_for_non_full_checksum(
 @param[in]	fsp_flags		Tablespace flags
 @param[in,out]	src_frame		Page to decrypt
 @param[out]	err			DB_SUCCESS or DB_DECRYPTION_FAILED
-@return true if page decrypted, false if not.*/
+@return DB_SUCCESS or error */
 UNIV_INTERN
-bool
+dberr_t
 fil_space_decrypt(
 	ulint			space_id,
 	fil_space_crypt_t*	crypt_data,
 	byte*			tmp_frame,
 	ulint			physical_size,
 	ulint			fsp_flags,
-	byte*			src_frame,
-	dberr_t*		err)
+	byte*			src_frame)
 {
 	if (fil_space_t::full_crc32(fsp_flags)) {
 		return fil_space_decrypt_full_crc32(
-			space_id, crypt_data, tmp_frame, src_frame, err);
+			space_id, crypt_data, tmp_frame, src_frame);
 	}
 
 	return fil_space_decrypt_for_non_full_checksum(crypt_data, tmp_frame,
-						       physical_size, src_frame,
-						       err);
+						       physical_size,
+						       src_frame);
 }
 
 /**
@@ -964,29 +948,22 @@ fil_space_decrypt(
 	byte*		tmp_frame,
 	byte*		src_frame)
 {
-	dberr_t err = DB_SUCCESS;
-	byte* res = NULL;
 	const ulint physical_size = space->physical_size();
 
 	ut_ad(space->crypt_data != NULL && space->crypt_data->is_encrypted());
 	ut_ad(space->pending_io());
 
-	bool encrypted = fil_space_decrypt(space->id, space->crypt_data,
-					   tmp_frame, physical_size,
-					   space->flags,
-					   src_frame, &err);
-
-	if (err == DB_SUCCESS) {
-		if (encrypted) {
-			/* Copy the decrypted page back to page buffer, not
-			really any other options. */
-			memcpy(src_frame, tmp_frame, physical_size);
-		}
-
-		res = src_frame;
+	if (DB_SUCCESS != fil_space_decrypt(space->id, space->crypt_data,
+					    tmp_frame, physical_size,
+					    space->flags, src_frame)) {
+		return nullptr;
 	}
 
-	return res;
+	/* Copy the decrypted page back to page buffer, not
+	really any other options. */
+	memcpy(src_frame, tmp_frame, physical_size);
+
+	return src_frame;
 }
 
 /**
@@ -2901,7 +2878,10 @@ encrypted, or corrupted.
 @return true if page is encrypted AND OK, false otherwise */
 bool fil_space_verify_crypt_checksum(const byte* page, ulint zip_size)
 {
-	ut_ad(mach_read_from_4(page + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION));
+	if (ENCRYPTION_KEY_NOT_ENCRYPTED == mach_read_from_4(
+			page + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION)) {
+		return false;
+	}
 
 	/* Compressed and encrypted pages do not have checksum. Assume not
 	corrupted. Page verification happens after decompression in
diff --git a/storage/innobase/include/fil0crypt.h b/storage/innobase/include/fil0crypt.h
index e8183d4aca8..8a3023ae134 100644
--- a/storage/innobase/include/fil0crypt.h
+++ b/storage/innobase/include/fil0crypt.h
@@ -358,18 +358,16 @@ fil_space_encrypt(
 @param[in]	physical_size		page size
 @param[in]	fsp_flags		Tablespace flags
 @param[in,out]	src_frame		Page to decrypt
-@param[out]	err			DB_SUCCESS or DB_DECRYPTION_FAILED
-@return true if page decrypted, false if not.*/
+@return DB_SUCCESS or error */
 UNIV_INTERN
-bool
+dberr_t
 fil_space_decrypt(
 	ulint			space_id,
 	fil_space_crypt_t*	crypt_data,
 	byte*			tmp_frame,
 	ulint			physical_size,
 	ulint			fsp_flags,
-	byte*			src_frame,
-	dberr_t*		err);
+	byte*			src_frame);
 
 /******************************************************************
 Decrypt a page
diff --git a/storage/innobase/row/row0import.cc b/storage/innobase/row/row0import.cc
index d2236d1e400..fa79f223637 100644
--- a/storage/innobase/row/row0import.cc
+++ b/storage/innobase/row/row0import.cc
@@ -3122,10 +3122,9 @@ static dberr_t decrypt_decompress(fil_space_crypt_t *space_crypt,
     if (!buf_page_verify_crypt_checksum(data, space_flags))
       return DB_CORRUPTION;
 
-    dberr_t err;
-    if (!fil_space_decrypt(space_id, space_crypt, data, page.size(),
-                           space_flags, data, &err))
-      return err ? err : DB_CORRUPTION;
+    if (dberr_t err= fil_space_decrypt(space_id, space_crypt, data,
+                                       page.size(), space_flags, data))
+      return err;
   }
   else if (fil_page_is_compressed_encrypted(data))
     return DB_CORRUPTION;
@@ -3835,9 +3834,12 @@ page_corrupted:
     if (!buf_page_verify_crypt_checksum(readptr, m_space_flags))
       goto page_corrupted;
 
-    if (!fil_space_decrypt(get_space_id(), iter.crypt_data, readptr,
-                           size, m_space_flags, readptr, &err) ||
-        err != DB_SUCCESS)
+    if (ENCRYPTION_KEY_NOT_ENCRYPTED ==
+        buf_page_get_key_version(readptr, m_space_flags))
+      goto page_corrupted;
+
+    if ((err= fil_space_decrypt(get_space_id(), iter.crypt_data, readptr, size,
+                                m_space_flags, readptr)))
       goto func_exit;
   }
 
@@ -3989,7 +3991,6 @@ page_corrupted:
 
 			if (!encrypted) {
 			} else if (!key_version) {
-not_encrypted:
 				if (block->page.id.page_no() == 0
 				    && block->page.zip.data) {
 					block->page.zip.data = src;
@@ -4008,21 +4009,16 @@ not_encrypted:
 					goto page_corrupted;
 				}
 
-				decrypted = fil_space_decrypt(
+				if ((err = fil_space_decrypt(
 					actual_space_id,
 					iter.crypt_data, dst,
 					callback.physical_size(),
 					callback.get_space_flags(),
-					src, &err);
-
-				if (err != DB_SUCCESS) {
+					src))) {
 					goto func_exit;
 				}
 
-				if (!decrypted) {
-					goto not_encrypted;
-				}
-
+				decrypted = true;
 				updated = true;
 			}