MDEV-18129 Backup fails for encrypted tables: mariabackup: Database page...

MDEV-18129 Backup fails for encrypted tables: mariabackup: Database page corruption detected at page 1

If an encrypted table is created during backup, then
mariabackup --backup could wrongly fail.

This caused a failure of the test mariabackup.huge_lsn once on buildbot.

This is due to the way how InnoDB creates .ibd files. It would first
write a dummy page 0 with no encryption information. Due to this,
xb_fil_cur_open() could wrongly interpret that the table is not encrypted.
Subsequently, page_is_corrupted() would compare the computed page
checksum to the wrong checksum. (There are both "before" and "after"
checksums for encrypted pages.)

To work around this problem, we introduce a Boolean option
--backup-encrypted that is enabled by default. With this option,
Mariabackup will assume that a nonzero key_version implies that the
page is encrypted. We need this option in order to be able to copy
encrypted tables from MariaDB 10.1 or 10.2, because unencrypted pages
that were originally created before MySQL 5.1.48 could contain nonzero
garbage in the fields that were repurposed for encryption.

Later, MDEV-18128 would clean up the way how .ibd files are created,
to remove the need for this option.

page_is_corrupted(): Add missing const qualifiers, and do not check
space->crypt_data unless --skip-backup-encrypted has been specified.

xb_fil_cur_read(): After a failed page read, output a page dump.

extra/mariabackup/fil_cur.cc

@@ -265,7 +265,9 @@ xb_fil_cur_open(
 	return(XB_FIL_CUR_SUCCESS);
 }
 
-static bool page_is_corrupted(byte *page, ulint page_no, xb_fil_cur_t *cursor, fil_space_t *space)
+static bool page_is_corrupted(const byte *page, ulint page_no,
+			      const xb_fil_cur_t *cursor,
+			      const fil_space_t *space)
 {
 	byte	tmp_frame[UNIV_PAGE_SIZE_MAX];
 	byte	tmp_page[UNIV_PAGE_SIZE_MAX];
@@ -293,8 +295,8 @@ static bool page_is_corrupted(byte *page, ulint page_no, xb_fil_cur_t *cursor, f
 		from the start of each file.)
 
 		The first 38 and last 8 bytes are never encrypted. */
-		const ulint* p = reinterpret_cast<ulint*>(page);
-		const ulint* const end = reinterpret_cast<ulint*>(
+		const ulint* p = reinterpret_cast<const ulint*>(page);
+		const ulint* const end = reinterpret_cast<const ulint*>(
 			page + cursor->page_size);
 		do {
 			if (*p++) {
@@ -314,8 +316,9 @@ static bool page_is_corrupted(byte *page, ulint page_no, xb_fil_cur_t *cursor, f
 	page_no first. */
 	if (page_no
 	    && mach_read_from_4(page + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION)
-	    && space->crypt_data
-	    && space->crypt_data->type != CRYPT_SCHEME_UNENCRYPTED) {
+	    && (opt_backup_encrypted
+		|| (space->crypt_data
+		    && space->crypt_data->type != CRYPT_SCHEME_UNENCRYPTED))) {
 
 		if (!fil_space_verify_crypt_checksum(page, cursor->zip_size))
 			return true;
@@ -327,7 +330,10 @@ static bool page_is_corrupted(byte *page, ulint page_no, xb_fil_cur_t *cursor, f
 		memcpy(tmp_page, page, cursor->page_size);
 
 		bool decrypted = false;
-		if (!fil_space_decrypt(space, tmp_frame,tmp_page, &decrypted)) {
+		if (!space->crypt_data
+		    || space->crypt_data->type == CRYPT_SCHEME_UNENCRYPTED
+		    || !fil_space_decrypt(space, tmp_frame, tmp_page,
+					  &decrypted)) {
 			return true;
 		}
 
@@ -444,6 +450,7 @@ xb_fil_cur_read(
 				    "corrupted.\n", cursor->thread_n,
 				    cursor->abs_path);
 				ret = XB_FIL_CUR_ERROR;
+				buf_page_print(page, cursor->page_size);
 				break;
 			}
 			msg("[%02u] mariabackup: "

extra/mariabackup/xtrabackup.cc

@@ -207,6 +207,7 @@ char*	log_ignored_opt				= NULL;
 extern my_bool opt_use_ssl;
 my_bool opt_ssl_verify_server_cert;
 my_bool opt_extended_validation;
+my_bool opt_backup_encrypted;
 
 /* === metadata of backup === */
 #define XTRABACKUP_METADATA_FILENAME "xtrabackup_checkpoints"
@@ -512,6 +513,7 @@ enum options_xtrabackup
   OPT_XTRA_CREATE_IB_LOGFILE,
   OPT_XTRA_PARALLEL,
   OPT_XTRA_EXTENDED_VALIDATION,
+  OPT_XTRA_BACKUP_ENCRYPTED,
   OPT_XTRA_STREAM,
   OPT_XTRA_COMPRESS,
   OPT_XTRA_COMPRESS_THREADS,
@@ -979,12 +981,20 @@ struct my_option xb_server_options[] =
    REQUIRED_ARG, 1, 1, INT_MAX, 0, 0, 0},
 
   {"extended_validation", OPT_XTRA_EXTENDED_VALIDATION,
-   "Enable extended validation for Innodb data pages during backup phase."
-   "Will slow down backup considerably, in case encryption is used.",
+   "Enable extended validation for Innodb data pages during backup phase. "
+   "Will slow down backup considerably, in case encryption is used. "
+   "May fail if tables are created during the backup.",
    (G_PTR*)&opt_extended_validation,
    (G_PTR*)&opt_extended_validation,
    0, GET_BOOL, NO_ARG, FALSE, 0, 0, 0, 0, 0},
 
+  {"backup_encrypted", OPT_XTRA_BACKUP_ENCRYPTED,
+   "In --backup, assume that nonzero key_version implies that the page"
+   " is encrypted. Use --backup --skip-backup-encrypted to allow"
+   " copying unencrypted that were originally created before MySQL 5.1.48.",
+   (G_PTR*)&opt_backup_encrypted,
+   (G_PTR*)&opt_backup_encrypted,
+   0, GET_BOOL, NO_ARG, TRUE, 0, 0, 0, 0, 0},
 
    {"log", OPT_LOG, "Ignored option for MySQL option compatibility",
    (G_PTR*) &log_ignored_opt, (G_PTR*) &log_ignored_opt, 0,

extra/mariabackup/xtrabackup.h

@@ -129,6 +129,7 @@ extern my_bool		opt_no_backup_locks;
 extern my_bool		opt_decompress;
 extern my_bool		opt_remove_original;
 extern my_bool		opt_extended_validation;
+extern my_bool		opt_backup_encrypted;
 
 extern char		*opt_incremental_history_name;
 extern char		*opt_incremental_history_uuid;