From 5957417b1f85e0921dae56e88ef68f3d5ce88e9f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 4 Feb 2024 18:09:32 +0100 Subject: [PATCH] Apply changelog entry / documentation improvements from review --- changelog/unreleased/issue-4529 | 22 +++++++++++++--------- cmd/restic/global.go | 2 +- doc/047_tuning_backup_parameters.rst | 15 +++++++++------ internal/pack/pack.go | 3 ++- internal/repository/repository.go | 6 ++++-- 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/changelog/unreleased/issue-4529 b/changelog/unreleased/issue-4529 index c3ec69510..fed726d2d 100644 --- a/changelog/unreleased/issue-4529 +++ b/changelog/unreleased/issue-4529 @@ -1,14 +1,18 @@ -Enhancement: Verify data integrity before upload +Enhancement: Add extra verification of data integrity before upload -Hardware issues or a bug in restic could cause restic to create corrupted files -that were then uploaded to the repository. Detecting such corruption usually -required explicitly running the `check --read-data` command. +Hardware issues, or a bug in restic or its dependencies, could previously cause +corruption in the files restic created and stored in the repository. Detecting +such corruption previously required explicitly running the `check --read-data` +or `check --read-data-subset` commands. -To prevent the upload of corrupted data to the repository, restic now -additionally verifies that files can be decoded and contain the correct data -beforehand. This increases the CPU usage during backups. If absolutely -necessary, you can disable the verification using the option -`--no-extra-verify`. +To further ensure data integrity, even in the case of hardware issues or +software bugs, restic now performs additional verification of the files about +to be uploaded to the repository. + +These extra checks will increase CPU usage during backups. They can therefore, +if absolutely necessary, be disabled using the `--no-extra-verify` global +option. Please note that this should be combined with more active checking +using the previously mentioned check commands. https://github.com/restic/restic/issues/4529 https://github.com/restic/restic/pull/4681 diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 08342435a..da01aa732 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -140,7 +140,7 @@ func init() { f.BoolVar(&globalOptions.InsecureTLS, "insecure-tls", false, "skip TLS certificate verification when connecting to the repository (insecure)") f.BoolVar(&globalOptions.CleanupCache, "cleanup-cache", false, "auto remove old cache directories") f.Var(&globalOptions.Compression, "compression", "compression mode (only available for repository format version 2), one of (auto|off|max) (default: $RESTIC_COMPRESSION)") - f.BoolVar(&globalOptions.NoExtraVerify, "no-extra-verify", false, "skip verification of data before upload") + f.BoolVar(&globalOptions.NoExtraVerify, "no-extra-verify", false, "skip additional verification of data before upload (see documentation)") f.IntVar(&globalOptions.Limits.UploadKb, "limit-upload", 0, "limits uploads to a maximum `rate` in KiB/s. (default: unlimited)") f.IntVar(&globalOptions.Limits.DownloadKb, "limit-download", 0, "limits downloads to a maximum `rate` in KiB/s. (default: unlimited)") f.UintVar(&globalOptions.PackSize, "pack-size", 0, "set target pack `size` in MiB, created pack files may be larger (default: $RESTIC_PACK_SIZE)") diff --git a/doc/047_tuning_backup_parameters.rst b/doc/047_tuning_backup_parameters.rst index 5773ac161..d8fb2c9b6 100644 --- a/doc/047_tuning_backup_parameters.rst +++ b/doc/047_tuning_backup_parameters.rst @@ -63,12 +63,15 @@ variable ``RESTIC_COMPRESSION``. Data Verification ================= -To prevent the upload of corrupted data to the repository, restic verifies that files can -be decoded and contain the correct data beforehand. This increases the CPU usage during -backups. If necessary, you can disable this verification using the option ``--no-extra-verify``. -However, in this case you should verify the repository integrity more actively using -``restic check --read-data``. Otherwise, data corruption due to hardware issues or software -bugs might go unnoticed. +To prevent the upload of corrupted data to the repository, which can happen due +to hardware issues or software bugs, restic verifies that generated files can +be decoded and contain the correct data beforehand. This increases the CPU usage +during backups. If necessary, you can disable this verification using the +``--no-extra-verify`` option of the ``backup`` command. However, in this case +you should verify the repository integrity more actively using +``restic check --read-data`` (or the similar ``--read-data-subset`` option). +Otherwise, data corruption due to hardware issues or software bugs might go +unnoticed. File Read Concurrency diff --git a/internal/pack/pack.go b/internal/pack/pack.go index 04042744e..cd118ab03 100644 --- a/internal/pack/pack.go +++ b/internal/pack/pack.go @@ -87,7 +87,8 @@ func (p *Packer) Finalize() error { encryptedHeader = binary.LittleEndian.AppendUint32(encryptedHeader, uint32(len(encryptedHeader))) if err := verifyHeader(p.k, encryptedHeader, p.blobs); err != nil { - return fmt.Errorf("detected data corruption while writing pack-file header: %w\nCorrupted data is either caused by hardware problems or bugs in restic. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting", err) + //nolint:revive // ignore linter warnings about error message spelling + return fmt.Errorf("Detected data corruption while writing pack-file header: %w\nCorrupted data is either caused by hardware issues or software bugs. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting.", err) } // append the header diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 917b7318f..a20f71ab1 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -425,7 +425,8 @@ func (r *Repository) saveAndEncrypt(ctx context.Context, t restic.BlobType, data ciphertext = r.key.Seal(ciphertext, nonce, data, nil) if err := r.verifyCiphertext(ciphertext, uncompressedLength, id); err != nil { - return 0, fmt.Errorf("detected data corruption while saving blob %v: %w\nCorrupted blobs are either caused by hardware problems or bugs in restic. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting", id, err) + //nolint:revive // ignore linter warnings about error message spelling + return 0, fmt.Errorf("Detected data corruption while saving blob %v: %w\nCorrupted blobs are either caused by hardware issues or software bugs. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting.", id, err) } // find suitable packer and add blob @@ -521,7 +522,8 @@ func (r *Repository) SaveUnpacked(ctx context.Context, t restic.FileType, buf [] ciphertext = r.key.Seal(ciphertext, nonce, p, nil) if err := r.verifyUnpacked(ciphertext, t, buf); err != nil { - return restic.ID{}, fmt.Errorf("detected data corruption while saving file of type %v: %w\nCorrupted data is either caused by hardware problems or bugs in restic. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting", t, err) + //nolint:revive // ignore linter warnings about error message spelling + return restic.ID{}, fmt.Errorf("Detected data corruption while saving file of type %v: %w\nCorrupted data is either caused by hardware issues or software bugs. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting.", t, err) } if t == restic.ConfigFile {