Hi All, With the attached patch, the backup manifest will have a new key item as "System-Identifier" 64-bit integer whose value is derived from pg_control while generating it, and the manifest version bumps to 2.
This helps to identify the correct database server and/or backup for the subsequent backup operations. pg_verifybackup validates the manifest system identifier against the backup control file and fails if they don’t match. Similarly, pg_basebackup increment backup will fail if the manifest system identifier does not match with the server system identifier. The pg_combinebackup is already a bit smarter -- checks the system identifier from the pg_control of all the backups, with this patch the manifest system identifier also validated. For backward compatibility, the manifest system identifier validation will be skipped for version 1. -- Regards, Amul Sul EDB: http://www.enterprisedb.com
From 03d21efd7b03d17a55fc1dd1159e0838777f548a Mon Sep 17 00:00:00 2001 From: Amul Sul <amul.sul@enterprisedb.com> Date: Wed, 17 Jan 2024 16:12:19 +0530 Subject: [PATCH v1] Add system identifier to backup manifest The backup manifest will be having a new item as "System-Identifier" and its value is from "ControlFileData.system_identifier" when manifest generated. This help identify the correct database server and/or backup while taking subsequent backup. --- doc/src/sgml/backup-manifest.sgml | 13 +++- doc/src/sgml/ref/pg_verifybackup.sgml | 3 +- src/backend/backup/backup_manifest.c | 9 ++- src/backend/backup/basebackup.c | 3 +- src/backend/backup/basebackup_incremental.c | 29 ++++++++ src/bin/pg_basebackup/t/010_pg_basebackup.pl | 20 +++++ src/bin/pg_combinebackup/load_manifest.c | 73 ++++++++++++++++--- src/bin/pg_combinebackup/load_manifest.h | 6 +- src/bin/pg_combinebackup/pg_combinebackup.c | 16 ++-- src/bin/pg_combinebackup/t/005_integrity.pl | 14 ++++ src/bin/pg_combinebackup/write_manifest.c | 8 +- src/bin/pg_combinebackup/write_manifest.h | 3 +- src/bin/pg_verifybackup/pg_verifybackup.c | 59 ++++++++++++++- src/bin/pg_verifybackup/t/003_corruption.pl | 16 +++- src/bin/pg_verifybackup/t/004_options.pl | 2 +- src/bin/pg_verifybackup/t/005_bad_manifest.pl | 7 +- src/common/parse_manifest.c | 36 ++++++++- src/include/backup/backup_manifest.h | 3 +- src/include/common/parse_manifest.h | 4 + 19 files changed, 286 insertions(+), 38 deletions(-) diff --git a/doc/src/sgml/backup-manifest.sgml b/doc/src/sgml/backup-manifest.sgml index 771be1310a..d0fc20ed0f 100644 --- a/doc/src/sgml/backup-manifest.sgml +++ b/doc/src/sgml/backup-manifest.sgml @@ -37,7 +37,18 @@ <term><literal>PostgreSQL-Backup-Manifest-Version</literal></term> <listitem> <para> - The associated value is always the integer 1. + The associated value is the integer, either 1 or 2. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>System-Identifier</literal></term> + <listitem> + <para> + The associated integer value is an unique system identifier to ensure + file match up with the installation that produced them. Available only + when <literal>PostgreSQL-Backup-Manifest-Version</literal> is 2 and later. </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/pg_verifybackup.sgml b/doc/src/sgml/ref/pg_verifybackup.sgml index 36335e0a18..3051517d92 100644 --- a/doc/src/sgml/ref/pg_verifybackup.sgml +++ b/doc/src/sgml/ref/pg_verifybackup.sgml @@ -53,7 +53,8 @@ PostgreSQL documentation Backup verification proceeds in four stages. First, <literal>pg_verifybackup</literal> reads the <literal>backup_manifest</literal> file. If that file - does not exist, cannot be read, is malformed, or fails verification + does not exist, cannot be read, is malformed, fails to match the system + identifier with pg_control of the backup directory or fails verification against its own internal checksum, <literal>pg_verifybackup</literal> will terminate with a fatal error. </para> diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c index 2c34e59752..612ff3add2 100644 --- a/src/backend/backup/backup_manifest.c +++ b/src/backend/backup/backup_manifest.c @@ -56,7 +56,8 @@ IsManifestEnabled(backup_manifest_info *manifest) void InitializeBackupManifest(backup_manifest_info *manifest, backup_manifest_option want_manifest, - pg_checksum_type manifest_checksum_type) + pg_checksum_type manifest_checksum_type, + uint64 system_identifier) { memset(manifest, 0, sizeof(backup_manifest_info)); manifest->checksum_type = manifest_checksum_type; @@ -79,8 +80,10 @@ InitializeBackupManifest(backup_manifest_info *manifest, if (want_manifest != MANIFEST_OPTION_NO) AppendToManifest(manifest, - "{ \"PostgreSQL-Backup-Manifest-Version\": 1,\n" - "\"Files\": ["); + "{ \"PostgreSQL-Backup-Manifest-Version\": 2,\n" + "\"System-Identifier\": " UINT64_FORMAT ",\n" + "\"Files\": [", + system_identifier); } /* diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index d5b8ca21b7..315efc7536 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -256,7 +256,8 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, backup_started_in_recovery = RecoveryInProgress(); InitializeBackupManifest(&manifest, opt->manifest, - opt->manifest_checksum_type); + opt->manifest_checksum_type, + GetSystemIdentifier()); total_checksum_failures = 0; diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c index 0504c465db..41a4fb4cb6 100644 --- a/src/backend/backup/basebackup_incremental.c +++ b/src/backend/backup/basebackup_incremental.c @@ -112,6 +112,9 @@ struct IncrementalBackupInfo BlockRefTable *brtab; }; +static void manifest_process_identity(JsonManifestParseContext *context, + int manifest_version, + uint64 manifest_system_identifier); static void manifest_process_file(JsonManifestParseContext *context, char *pathname, size_t size, @@ -198,6 +201,7 @@ FinalizeIncrementalManifest(IncrementalBackupInfo *ib) /* Parse the manifest. */ context.private_data = ib; + context.identity_cb = manifest_process_identity; context.per_file_cb = manifest_process_file; context.per_wal_range_cb = manifest_process_wal_range; context.error_cb = manifest_report_error; @@ -910,6 +914,31 @@ hash_string_pointer(const char *s) return hash_bytes(ss, strlen(s)); } +/* + * This callback to validate the manifest system identifier against the current + * database server. + */ +static void +manifest_process_identity(JsonManifestParseContext *context, + int manifest_version, + uint64 manifest_system_identifier) +{ + uint64 system_identifier; + + /* Manifest system identifier available in version 2 or later */ + if (manifest_version == 1) + return; + + /* Get system identifier of current system */ + system_identifier = GetSystemIdentifier(); + + if (manifest_system_identifier != system_identifier) + context->error_cb(context, + "manifest is from different database system: manifest database system identifier is %llu, pg_control database system identifier is %llu", + (unsigned long long) manifest_system_identifier, + (unsigned long long) system_identifier); + } + /* * This callback is invoked for each file mentioned in the backup manifest. * diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 42a09d0da3..ec663dfcd4 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -949,4 +949,24 @@ $backupdir = $node->backup_dir . '/backup3'; my @dst_tblspc = glob "$backupdir/pg_tblspc/$tblspc_oid/PG_*"; is(@dst_tblspc, 1, 'tblspc directory copied'); +# Can't take backup with referring manifest of different cluster +# +# Set up another new database instance. We don't want to use the cached +# INITDB_TEMPLATE for this, because we want it to be a separate cluster +# with a different system ID. +my $node2; +{ + local $ENV{'INITDB_TEMPLATE'} = undef; + + $node2 = PostgreSQL::Test::Cluster->new('node2'); + $node2->init(has_archiving => 1, allows_streaming => 1); + $node2->append_conf('postgresql.conf', 'summarize_wal = on'); + $node2->start; +} +$node2->command_fails_like( + [ @pg_basebackup_defs, '-D', "$tempdir" . '/diff_sysid', + '--incremental', "$backupdir" . '/backup_manifest' ], + qr/manifest is from different database system/, + "pg_basebackup fails with different database system manifest"); + done_testing(); diff --git a/src/bin/pg_combinebackup/load_manifest.c b/src/bin/pg_combinebackup/load_manifest.c index 2b8e74fcf3..40d3949543 100644 --- a/src/bin/pg_combinebackup/load_manifest.c +++ b/src/bin/pg_combinebackup/load_manifest.c @@ -50,6 +50,19 @@ static uint32 hash_string_pointer(char *s); #define SH_DEFINE #include "lib/simplehash.h" +/* + * Details we need in callbacks that occur while parsing a backup manifest. + */ +typedef struct parser_context +{ + manifest_data *manifest; + char *backup_directory; + uint64 system_identifier; +} parser_context; + +static void combinebackup_identity_cb(JsonManifestParseContext *context, + int manifest_version, + uint64 manifest_system_identifier); static void combinebackup_per_file_cb(JsonManifestParseContext *context, char *pathname, size_t size, pg_checksum_type checksum_type, @@ -71,14 +84,16 @@ static void report_manifest_error(JsonManifestParseContext *context, * contain NULL entries. */ manifest_data ** -load_backup_manifests(int n_backups, char **backup_directories) +load_backup_manifests(int n_backups, char **backup_directories, + uint64 system_identifier) { manifest_data **result; int i; result = pg_malloc(sizeof(manifest_data *) * n_backups); for (i = 0; i < n_backups; ++i) - result[i] = load_backup_manifest(backup_directories[i]); + result[i] = load_backup_manifest(backup_directories[i], + system_identifier); return result; } @@ -93,7 +108,7 @@ load_backup_manifests(int n_backups, char **backup_directories) * fatal. */ manifest_data * -load_backup_manifest(char *backup_directory) +load_backup_manifest(char *backup_directory, uint64 system_identifier) { char pathname[MAXPGPATH]; int fd; @@ -104,7 +119,7 @@ load_backup_manifest(char *backup_directory) char *buffer; int rc; JsonManifestParseContext context; - manifest_data *result; + parser_context private_context; /* Open the manifest file. */ snprintf(pathname, MAXPGPATH, "%s/backup_manifest", backup_directory); @@ -150,9 +165,12 @@ load_backup_manifest(char *backup_directory) close(fd); /* Parse the manifest. */ - result = pg_malloc0(sizeof(manifest_data)); - result->files = ht; - context.private_data = result; + private_context.manifest = pg_malloc0(sizeof(manifest_data)); + private_context.manifest->files = ht; + private_context.backup_directory = backup_directory; + private_context.system_identifier = system_identifier; + context.private_data = &private_context; + context.identity_cb = combinebackup_identity_cb; context.per_file_cb = combinebackup_per_file_cb; context.per_wal_range_cb = combinebackup_per_wal_range_cb; context.error_cb = report_manifest_error; @@ -160,7 +178,7 @@ load_backup_manifest(char *backup_directory) /* All done. */ pfree(buffer); - return result; + return private_context.manifest; } /* @@ -181,6 +199,39 @@ report_manifest_error(JsonManifestParseContext *context, const char *fmt,...) exit(1); } +/* + * This callback to validate the manifest system identifier against the backup. + */ +static void +combinebackup_identity_cb(JsonManifestParseContext *context, + int manifest_version, + uint64 manifest_system_identifier) +{ + parser_context *private_context = context->private_data; + uint64 system_identifier = private_context->system_identifier; + + /* Manifest system identifier available in version 2 or later */ + if (manifest_version == 1) + return; + + if (manifest_system_identifier != system_identifier) + { + char *controlpath; + + /* + * check_control_files() ensures the given system identifier through + * private context belongs to the backup pg_control + */ + controlpath = psprintf("%s/global/pg_control", + private_context->backup_directory); + + pg_fatal("manifest is from different database system: manifest database system identifier is %llu, %s system identifier is %llu", + (unsigned long long) manifest_system_identifier, + controlpath, + (unsigned long long) system_identifier); + } +} + /* * Record details extracted from the backup manifest for one file. */ @@ -190,7 +241,8 @@ combinebackup_per_file_cb(JsonManifestParseContext *context, pg_checksum_type checksum_type, int checksum_length, uint8 *checksum_payload) { - manifest_data *manifest = context->private_data; + parser_context *private_context = context->private_data; + manifest_data *manifest = private_context->manifest; manifest_file *m; bool found; @@ -214,7 +266,8 @@ combinebackup_per_wal_range_cb(JsonManifestParseContext *context, TimeLineID tli, XLogRecPtr start_lsn, XLogRecPtr end_lsn) { - manifest_data *manifest = context->private_data; + parser_context *private_context = context->private_data; + manifest_data *manifest = private_context->manifest; manifest_wal_range *range; /* Allocate and initialize a struct describing this WAL range. */ diff --git a/src/bin/pg_combinebackup/load_manifest.h b/src/bin/pg_combinebackup/load_manifest.h index 9163e071af..2a2414799a 100644 --- a/src/bin/pg_combinebackup/load_manifest.h +++ b/src/bin/pg_combinebackup/load_manifest.h @@ -60,8 +60,10 @@ typedef struct manifest_data manifest_wal_range *last_wal_range; } manifest_data; -extern manifest_data *load_backup_manifest(char *backup_directory); +extern manifest_data *load_backup_manifest(char *backup_directory, + uint64 system_identifier); extern manifest_data **load_backup_manifests(int n_backups, - char **backup_directories); + char **backup_directories, + uint64 system_identifier); #endif /* LOAD_MANIFEST_H */ diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index 31ead7f405..f2934e90ab 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -92,7 +92,7 @@ cb_cleanup_dir *cleanup_dir_list = NULL; static void add_tablespace_mapping(cb_options *opt, char *arg); static StringInfo check_backup_label_files(int n_backups, char **backup_dirs); -static void check_control_files(int n_backups, char **backup_dirs); +static uint64 check_control_files(int n_backups, char **backup_dirs); static void check_input_dir_permissions(char *dir); static void cleanup_directories_atexit(void); static void create_output_directory(char *dirname, cb_options *opt); @@ -139,6 +139,7 @@ main(int argc, char *argv[]) int n_backups; int n_prior_backups; int version; + uint64 system_identifier; char **prior_backup_dirs; cb_options opt; cb_tablespace *tablespaces; @@ -216,7 +217,7 @@ main(int argc, char *argv[]) /* Sanity-check control files. */ n_backups = argc - optind; - check_control_files(n_backups, argv + optind); + system_identifier = check_control_files(n_backups, argv + optind); /* Sanity-check backup_label files, and get the contents of the last one. */ last_backup_label = check_backup_label_files(n_backups, argv + optind); @@ -229,7 +230,8 @@ main(int argc, char *argv[]) prior_backup_dirs = argv + optind; /* Load backup manifests. */ - manifests = load_backup_manifests(n_backups, prior_backup_dirs); + manifests = load_backup_manifests(n_backups, prior_backup_dirs, + system_identifier); /* Figure out which tablespaces are going to be included in the output. */ last_input_dir = argv[argc - 1]; @@ -256,7 +258,7 @@ main(int argc, char *argv[]) /* If we need to write a backup_manifest, prepare to do so. */ if (!opt.dry_run && !opt.no_manifest) { - mwriter = create_manifest_writer(opt.output); + mwriter = create_manifest_writer(opt.output, system_identifier); /* * Verify that we have a backup manifest for the final backup; else we @@ -517,9 +519,9 @@ check_backup_label_files(int n_backups, char **backup_dirs) } /* - * Sanity check control files. + * Sanity check control files and return system_identifier. */ -static void +static uint64 check_control_files(int n_backups, char **backup_dirs) { int i; @@ -564,6 +566,8 @@ check_control_files(int n_backups, char **backup_dirs) */ pg_log_debug("system identifier is %llu", (unsigned long long) system_identifier); + + return system_identifier; } /* diff --git a/src/bin/pg_combinebackup/t/005_integrity.pl b/src/bin/pg_combinebackup/t/005_integrity.pl index 3b445d0e30..875ffbf525 100644 --- a/src/bin/pg_combinebackup/t/005_integrity.pl +++ b/src/bin/pg_combinebackup/t/005_integrity.pl @@ -8,6 +8,7 @@ use strict; use warnings FATAL => 'all'; use File::Compare; use File::Path qw(rmtree); +use File::Copy; use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; @@ -85,6 +86,19 @@ $node1->command_fails_like( qr/expected system identifier.*but found/, "can't combine backups from different nodes"); +# Can't combine when different manifest system identifier +rename("$backup2path/backup_manifest", "$backup2path/backup_manifest.orig") + or BAIL_OUT "could not move $backup2path/backup_manifest"; +copy("$backupother2path/backup_manifest", "$backup2path/backup_manifest") + or BAIL_OUT "could not copy $backupother2path/backup_manifest"; +$node1->command_fails_like( + [ 'pg_combinebackup', $backup1path, $backup2path, $backup3path, '-o', $resultpath ], + qr/manifest is from different database system/, + "can't combine backups with different manifest system identifier "); +# Restore the backup state +move("$backup2path/backup_manifest.orig", "$backup2path/backup_manifest") + or BAIL_OUT "could not move $backup2path/backup_manifest"; + # Can't omit a required backup. $node1->command_fails_like( [ 'pg_combinebackup', $backup1path, $backup3path, '-o', $resultpath ], diff --git a/src/bin/pg_combinebackup/write_manifest.c b/src/bin/pg_combinebackup/write_manifest.c index 01deb82cc9..7a2065e1db 100644 --- a/src/bin/pg_combinebackup/write_manifest.c +++ b/src/bin/pg_combinebackup/write_manifest.c @@ -45,7 +45,7 @@ static size_t hex_encode(const uint8 *src, size_t len, char *dst); * in the specified directory. */ manifest_writer * -create_manifest_writer(char *directory) +create_manifest_writer(char *directory, uint64 system_identifier) { manifest_writer *mwriter = pg_malloc(sizeof(manifest_writer)); @@ -57,8 +57,10 @@ create_manifest_writer(char *directory) pg_checksum_init(&mwriter->manifest_ctx, CHECKSUM_TYPE_SHA256); appendStringInfo(&mwriter->buf, - "{ \"PostgreSQL-Backup-Manifest-Version\": 1,\n" - "\"Files\": ["); + "{ \"PostgreSQL-Backup-Manifest-Version\": 2,\n" + "\"System-Identifier\": " UINT64_FORMAT ",\n" + "\"Files\": [", + system_identifier); return mwriter; } diff --git a/src/bin/pg_combinebackup/write_manifest.h b/src/bin/pg_combinebackup/write_manifest.h index de0f742779..ebc4f9441a 100644 --- a/src/bin/pg_combinebackup/write_manifest.h +++ b/src/bin/pg_combinebackup/write_manifest.h @@ -19,7 +19,8 @@ struct manifest_wal_range; struct manifest_writer; typedef struct manifest_writer manifest_writer; -extern manifest_writer *create_manifest_writer(char *directory); +extern manifest_writer *create_manifest_writer(char *directory, + uint64 system_identifier); extern void add_file_to_manifest(manifest_writer *mwriter, const char *manifest_path, size_t size, time_t mtime, diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c index ae8c18f373..93dbc1b327 100644 --- a/src/bin/pg_verifybackup/pg_verifybackup.c +++ b/src/bin/pg_verifybackup/pg_verifybackup.c @@ -18,6 +18,7 @@ #include <sys/stat.h> #include <time.h> +#include "common/controldata_utils.h" #include "common/hashfn.h" #include "common/logging.h" #include "common/parse_manifest.h" @@ -101,6 +102,7 @@ typedef struct parser_context manifest_files_hash *ht; manifest_wal_range *first_wal_range; manifest_wal_range *last_wal_range; + char *backup_directory; } parser_context; /* @@ -117,8 +119,11 @@ typedef struct verifier_context static void parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p, - manifest_wal_range **first_wal_range_p); - + manifest_wal_range **first_wal_range_p, + char *backup_directory); +static void verifybackup_identity_cb(JsonManifestParseContext *context, + int manifest_version, + uint64 manifest_system_identifier); static void verifybackup_per_file_cb(JsonManifestParseContext *context, char *pathname, size_t size, pg_checksum_type checksum_type, @@ -338,7 +343,8 @@ main(int argc, char **argv) * the manifest as fatal; there doesn't seem to be much point in trying to * verify the backup directory against a corrupted manifest. */ - parse_manifest_file(manifest_path, &context.ht, &first_wal_range); + parse_manifest_file(manifest_path, &context.ht, &first_wal_range, + context.backup_directory); /* * Now scan the files in the backup directory. At this stage, we verify @@ -387,7 +393,8 @@ main(int argc, char **argv) */ static void parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p, - manifest_wal_range **first_wal_range_p) + manifest_wal_range **first_wal_range_p, + char *backup_directory) { int fd; struct stat statbuf; @@ -439,7 +446,9 @@ parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p, private_context.ht = ht; private_context.first_wal_range = NULL; private_context.last_wal_range = NULL; + private_context.backup_directory = backup_directory; context.private_data = &private_context; + context.identity_cb = verifybackup_identity_cb; context.per_file_cb = verifybackup_per_file_cb; context.per_wal_range_cb = verifybackup_per_wal_range_cb; context.error_cb = report_manifest_error; @@ -471,6 +480,48 @@ report_manifest_error(JsonManifestParseContext *context, const char *fmt,...) exit(1); } +/* + * Validate the manifest system identifier against the pg_control of the backup + * directory. + */ +static void +verifybackup_identity_cb(JsonManifestParseContext *context, + int manifest_version, + uint64 manifest_system_identifier) +{ + parser_context *parseContext = (parser_context *) context->private_data; + ControlFileData *control_file; + char *controlpath; + bool crc_ok; + + /* Manifest system identifier available in version 2 or later */ + if (manifest_version == 1) + return; + + /* Get the system identifier from the pg_cotrol file */ + controlpath = psprintf("%s/global/pg_control", + parseContext->backup_directory); + pg_log_debug("reading \"%s\"", controlpath); + control_file = get_controlfile(parseContext->backup_directory, &crc_ok); + + /* Control file contents not meaningful if CRC is bad. */ + if (!crc_ok) + report_fatal_error("%s: CRC is incorrect", controlpath); + + /* Can't interpret control file if not current version. */ + if (control_file->pg_control_version != PG_CONTROL_VERSION) + report_fatal_error("%s: unexpected control file version", + controlpath); + + if (manifest_system_identifier != control_file->system_identifier) + report_fatal_error("manifest is from different database system: manifest database system identifier is %llu, %s system identifier is %llu", + (unsigned long long) manifest_system_identifier, + controlpath, + (unsigned long long) control_file->system_identifier); + pfree(control_file); + pfree(controlpath); +} + /* * Record details extracted from the backup manifest for one file. */ diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl index 11bd577081..fcaaa5eae7 100644 --- a/src/bin/pg_verifybackup/t/003_corruption.pl +++ b/src/bin/pg_verifybackup/t/003_corruption.pl @@ -68,6 +68,11 @@ my @scenario = ( 'mutilate' => \&mutilate_replace_file, 'fails_like' => qr/checksum mismatch for file/ }, + { + 'name' => 'system_identifier', + 'mutilate' => \&mutilate_system_identifier, + 'fails_like' => qr/manifest is from different database system/ + }, { 'name' => 'bad_manifest', 'mutilate' => \&mutilate_bad_manifest, @@ -216,7 +221,7 @@ sub mutilate_append_to_file sub mutilate_truncate_file { my ($backup_path) = @_; - my $pathname = "$backup_path/global/pg_control"; + my $pathname = "$backup_path/pg_hba.conf"; open(my $fh, '>', $pathname) || die "open $pathname: $!"; close($fh); return; @@ -236,6 +241,15 @@ sub mutilate_replace_file return; } +# Change System-Identifier from the backup manifest. +sub mutilate_system_identifier +{ + my ($backup_path) = @_; + string_replace_file("$backup_path/backup_manifest", + '"System-Identifier": .*,' , '"System-Identifier": "12345678900",'); + return; +} + # Corrupt the backup manifest. sub mutilate_bad_manifest { diff --git a/src/bin/pg_verifybackup/t/004_options.pl b/src/bin/pg_verifybackup/t/004_options.pl index 8ed2214408..970b7dcd56 100644 --- a/src/bin/pg_verifybackup/t/004_options.pl +++ b/src/bin/pg_verifybackup/t/004_options.pl @@ -111,7 +111,7 @@ command_fails_like( 'pg_verifybackup', '-m', "$backup_path/backup_manifest", "$backup_path/fake" ], - qr/could not open directory/, + qr/could not open file/, 'nonexistent backup directory'); done_testing(); diff --git a/src/bin/pg_verifybackup/t/005_bad_manifest.pl b/src/bin/pg_verifybackup/t/005_bad_manifest.pl index e278ccea5a..9bd87feecc 100644 --- a/src/bin/pg_verifybackup/t/005_bad_manifest.pl +++ b/src/bin/pg_verifybackup/t/005_bad_manifest.pl @@ -34,11 +34,14 @@ test_parse_error('unexpected manifest version', <<EOM); EOM test_parse_error('unexpected scalar', <<EOM); -{"PostgreSQL-Backup-Manifest-Version": 1, "Files": true} +{"PostgreSQL-Backup-Manifest-Version": 2, "Files": true} EOM +# Note that, we don't have any specific check for System-Identifier at parsing. test_parse_error('unrecognized top-level field', <<EOM); -{"PostgreSQL-Backup-Manifest-Version": 1, "Oops": 1} +{"PostgreSQL-Backup-Manifest-Version": 2, + "System-Identifier": "7320280665284567118", + "Oops": 1} EOM test_parse_error('unexpected object start', <<EOM); diff --git a/src/common/parse_manifest.c b/src/common/parse_manifest.c index 92a97714f3..269165cd88 100644 --- a/src/common/parse_manifest.c +++ b/src/common/parse_manifest.c @@ -25,6 +25,7 @@ typedef enum JM_EXPECT_TOPLEVEL_END, JM_EXPECT_TOPLEVEL_FIELD, JM_EXPECT_VERSION_VALUE, + JM_EXPECT_SYSTEM_IDENTIFIER_VALUE, JM_EXPECT_FILES_START, JM_EXPECT_FILES_NEXT, JM_EXPECT_THIS_FILE_FIELD, @@ -85,6 +86,8 @@ typedef struct /* Miscellaneous other stuff. */ bool saw_version_field; + int manifest_version; + uint64 manifest_system_identifier; char *manifest_checksum; } JsonManifestParseState; @@ -100,6 +103,7 @@ static void json_manifest_finalize_file(JsonManifestParseState *parse); static void json_manifest_finalize_wal_range(JsonManifestParseState *parse); static void verify_manifest_checksum(JsonManifestParseState *parse, char *buffer, size_t size); +static void verify_manifest_system_identifier(JsonManifestParseState *parse); static void json_manifest_parse_failure(JsonManifestParseContext *context, char *msg); @@ -151,6 +155,9 @@ json_parse_manifest(JsonManifestParseContext *context, char *buffer, if (parse.state != JM_EXPECT_EOF) json_manifest_parse_failure(context, "manifest ended unexpectedly"); + /* Verify the manifest system identifier. */ + verify_manifest_system_identifier(&parse); + /* Verify the manifest checksum. */ verify_manifest_checksum(&parse, buffer, size); @@ -312,6 +319,13 @@ json_manifest_object_field_start(void *state, char *fname, bool isnull) break; } + /* Is this the system identifier? */ + if (strcmp(fname, "System-Identifier") == 0) + { + parse->state = JM_EXPECT_SYSTEM_IDENTIFIER_VALUE; + break; + } + /* Is this the list of files? */ if (strcmp(fname, "Files") == 0) { @@ -404,12 +418,19 @@ json_manifest_scalar(void *state, char *token, JsonTokenType tokentype) switch (parse->state) { case JM_EXPECT_VERSION_VALUE: - if (strcmp(token, "1") != 0) + parse->manifest_version = atoi(token); + if (parse->manifest_version != 1 && parse->manifest_version != 2) json_manifest_parse_failure(parse->context, "unexpected manifest version"); parse->state = JM_EXPECT_TOPLEVEL_FIELD; break; + case JM_EXPECT_SYSTEM_IDENTIFIER_VALUE: + Assert(parse->manifest_version == 2); + parse->manifest_system_identifier = strtou64(token, NULL, 10); + parse->state = JM_EXPECT_TOPLEVEL_FIELD; + break; + case JM_EXPECT_THIS_FILE_VALUE: switch (parse->file_field) { @@ -691,6 +712,19 @@ verify_manifest_checksum(JsonManifestParseState *parse, char *buffer, pg_cryptohash_free(manifest_ctx); } +/* + * Validate manifest system identifier against the database server system + * identifier. + */ +static void +verify_manifest_system_identifier(JsonManifestParseState *parse) +{ + JsonManifestParseContext *context = parse->context; + + context->identity_cb(context, parse->manifest_version, + parse->manifest_system_identifier); +} + /* * Report a parse error. * diff --git a/src/include/backup/backup_manifest.h b/src/include/backup/backup_manifest.h index 3853d2ca90..4a21102506 100644 --- a/src/include/backup/backup_manifest.h +++ b/src/include/backup/backup_manifest.h @@ -37,7 +37,8 @@ typedef struct backup_manifest_info extern void InitializeBackupManifest(backup_manifest_info *manifest, backup_manifest_option want_manifest, - pg_checksum_type manifest_checksum_type); + pg_checksum_type manifest_checksum_type, + uint64 system_identifier); extern void AddFileToBackupManifest(backup_manifest_info *manifest, Oid spcoid, const char *pathname, size_t size, diff --git a/src/include/common/parse_manifest.h b/src/include/common/parse_manifest.h index f74be0db35..ea1ee9a5e6 100644 --- a/src/include/common/parse_manifest.h +++ b/src/include/common/parse_manifest.h @@ -21,6 +21,9 @@ struct JsonManifestParseContext; typedef struct JsonManifestParseContext JsonManifestParseContext; +typedef void (*json_manifest_identity_callback) (JsonManifestParseContext *, + int manifest_version, + uint64 manifest_system_identifier); typedef void (*json_manifest_per_file_callback) (JsonManifestParseContext *, char *pathname, size_t size, pg_checksum_type checksum_type, @@ -35,6 +38,7 @@ typedef void (*json_manifest_error_callback) (JsonManifestParseContext *, struct JsonManifestParseContext { void *private_data; + json_manifest_identity_callback identity_cb; json_manifest_per_file_callback per_file_cb; json_manifest_per_wal_range_callback per_wal_range_cb; json_manifest_error_callback error_cb; -- 2.18.0