diff --git a/Makefile.am b/Makefile.am index 3806fb0a..30247f51 100644 --- a/Makefile.am +++ b/Makefile.am @@ -83,7 +83,9 @@ EXTRA_DIST = \ tests/bug128-parfiles.tar.gz \ tests/bug190.tar.gz \ tests/block-count-wrap.tar.gz \ + tests/full-hash-mismatch.tar.gz \ tests/generate-block-count-wrap-fixture.py \ + tests/generate-full-hash-mismatch-fixture.py \ tests/flatdata-filelist.txt \ tests/subdirdata-filelist.txt \ tests/subdirdata-partial-filelist.txt \ @@ -184,6 +186,8 @@ EXTRA_DIST = \ tests/test46 \ tests/test47 \ tests/test47.ps1 \ + tests/test48 \ + tests/test48.ps1 \ tests/unit_tests \ tests/unit_tests.ps1 @@ -271,6 +275,7 @@ TESTS = \ tests/test45 \ tests/test46 \ tests/test47 \ + tests/test48 \ tests/utf8_test \ tests/unit_tests diff --git a/man/par2.1 b/man/par2.1 index 804ef825..a3b96a11 100644 --- a/man/par2.1 +++ b/man/par2.1 @@ -77,6 +77,9 @@ Rename-only mode (skip files that are not perfect matches, useful for quickly fi .B \-N data skipping (find badly mispositioned data blocks) .TP +.B \-\-force\-full\-hash\-verify +Also check the hash of the whole of each file, not only the hash of each of its blocks +.TP .B \-S Skip leaway (distance +/\- from expected block position, default 64) .SH OPTIONS create diff --git a/src/commandline.cpp b/src/commandline.cpp index 729042be..450bbc7b 100644 --- a/src/commandline.cpp +++ b/src/commandline.cpp @@ -55,6 +55,7 @@ CommandLine::CommandLine(void) , operation(opNone) , purgefiles(false) , renameonly(false) +, forcefullhashverify(false) , skipdata(false) , skipleaway(0) , blockcount(0) @@ -130,6 +131,8 @@ void CommandLine::usage(void) " -O : Rename-only mode (skip files that are not perfect matches,\n" " useful for quickly fixing renamed files)\n" " -N : Data skipping (find badly mispositioned data blocks)\n" + " --force-full-hash-verify :\n" + " Also check the hash of the whole of each file, not only its blocks\n" " -S : Skip leaway (distance +/- from expected block position, default 64)\n" "Options: (create)\n" " -b : Set the Block-Count (default 2000)\n" @@ -862,6 +865,16 @@ bool CommandLine::ReadArgs(int argc, const char * const *argv) case '-': { + if (argv[0] == std::string("--force-full-hash-verify")) { + if (operation == opCreate) + { + std::cerr << "Cannot specify a full hash verify unless repairing or verifying." << std::endl; + return false; + } + forcefullhashverify = true; + break; + } + if (argv[0] != std::string("--")) { std::cerr << "Unknown option: " << argv[0] << std::endl; std::cerr << " (Options must appear after create, repair or verify.)" << std::endl; diff --git a/src/commandline.h b/src/commandline.h index 1e639161..f52ca62c 100644 --- a/src/commandline.h +++ b/src/commandline.h @@ -98,6 +98,7 @@ class CommandLine bool GetRecursive(void) const {return recursive;} bool GetFollowLinks(void) const {return followlinks;} bool GetSkipData(void) const {return skipdata;} + bool GetForceFullHashVerify(void) const {return forcefullhashverify;} u64 GetSkipLeaway(void) const {return skipleaway;} #ifdef _OPENMP u32 GetNumThreads(void) {return nthreads;} @@ -176,6 +177,8 @@ class CommandLine // recovery bool renameonly; // Only attempt to repair via rename, skip // files that are not perfect matches + bool forcefullhashverify; // Whether to check the hash of the whole of each + // file as well as the hash of each of its blocks bool skipdata; // Whether we should assume that all good // data blocks are within +/- bytes of // where we expect to find them and should diff --git a/src/filechecksummer.cpp b/src/filechecksummer.cpp index 82c8632d..ceadc70c 100644 --- a/src/filechecksummer.cpp +++ b/src/filechecksummer.cpp @@ -46,8 +46,7 @@ FileCheckSummer::FileCheckSummer(DiskFile *_diskfile, , tailpointer(0) , readoffset(0) , checksum(0) -, contextfull() -, context16k() +, filehasher() { buffer = new char[(size_t)blocksize*2]; } @@ -159,7 +158,7 @@ bool FileCheckSummer::Fill(bool longfill) return false; if (computefilehashes) - UpdateHashes(readoffset, tailpointer, want); + filehasher.Update(readoffset, tailpointer, want); readoffset += want; tailpointer += want; } @@ -175,8 +174,8 @@ bool FileCheckSummer::Fill(bool longfill) return true; } -// Update the full file hash and the 16k hash using the new data -void FileCheckSummer::UpdateHashes(u64 offset, const void *buffer, size_t length) +// Add the next part of the file +void FileHasher::Update(u64 offset, const void *buffer, size_t length) { // Are we already beyond the first 16k if (offset >= 16384) @@ -210,6 +209,12 @@ void FileCheckSummer::GetFileHashes(MD5Hash &hashfull, MD5Hash &hash16k) const { assert(computefilehashes); + filehasher.GetHashes(filesize, hashfull, hash16k); +} + +// Return the full file hash and the 16k file hash +void FileHasher::GetHashes(u64 filesize, MD5Hash &hashfull, MD5Hash &hash16k) const +{ // Compute the hash of the first 16k MD5Context context = context16k; context.Final(hash16k); diff --git a/src/filechecksummer.h b/src/filechecksummer.h index 574eec6f..44617a34 100644 --- a/src/filechecksummer.h +++ b/src/filechecksummer.h @@ -34,6 +34,22 @@ // the object also computes the MD5 Hash of the whole file and of // the first 16k of the file for later tests. +// Computes the hash of the whole of a file and of its first 16k from the data +// of the file supplied in order +class FileHasher +{ +public: + // Add the next part of the file + void Update(u64 offset, const void *buffer, size_t length); + + // Return the full file hash and the 16k file hash + void GetHashes(u64 filesize, MD5Hash &hashfull, MD5Hash &hash16k) const; + +protected: + MD5Context contextfull; + MD5Context context16k; +}; + class FileCheckSummer { public: @@ -100,14 +116,10 @@ class FileCheckSummer u32 checksum; // MD5 hash of whole file and of first 16k - MD5Context contextfull; - MD5Context context16k; + FileHasher filehasher; protected: - //void ComputeCurrentCRC(void); - void UpdateHashes(u64 offset, const void *buffer, size_t length); - - //// Fill the buffers with more data from disk + // Fill the buffers with more data from disk // Set longfill = true to force fill the whole buffer bool Fill(bool longfill = false); diff --git a/src/libpar2.cpp b/src/libpar2.cpp index b3956d84..ec8aad11 100644 --- a/src/libpar2.cpp +++ b/src/libpar2.cpp @@ -72,7 +72,8 @@ Result par2repair(std::ostream &sout, const bool purgefiles, const bool renameonly, const bool skipdata, - const u64 skipleaway + const u64 skipleaway, + const bool forcefullhashverify ) { Par2Repairer repairer(sout, serr, noiselevel); @@ -89,7 +90,8 @@ Result par2repair(std::ostream &sout, purgefiles, renameonly, skipdata, - skipleaway); + skipleaway, + forcefullhashverify); return result; } diff --git a/src/libpar2.h b/src/libpar2.h index 0366ed61..7ab5784c 100644 --- a/src/libpar2.h +++ b/src/libpar2.h @@ -195,7 +195,8 @@ Result par2repair(std::ostream &sout, const bool purgefiles, const bool renameonly, const bool skipdata, - const u64 skipleaway + const u64 skipleaway, + const bool forcefullhashverify = false ); diff --git a/src/par2cmdline.cpp b/src/par2cmdline.cpp index 93b459d3..e87eda63 100644 --- a/src/par2cmdline.cpp +++ b/src/par2cmdline.cpp @@ -135,7 +135,8 @@ int main(int argc, char* argv[]) commandline->GetPurgeFiles(), commandline->GetRenameOnly(), commandline->GetSkipData(), - commandline->GetSkipLeaway()); + commandline->GetSkipLeaway(), + commandline->GetForceFullHashVerify()); break; default: break; diff --git a/src/par2repairer.cpp b/src/par2repairer.cpp index 73084dcc..9afe37f6 100644 --- a/src/par2repairer.cpp +++ b/src/par2repairer.cpp @@ -147,9 +147,13 @@ Result Par2Repairer::Process( const bool purgefiles, const bool renameonly, const bool _skipdata, - const u64 _skipleaway + const u64 _skipleaway, + const bool _forcefullhashverify ) { + // Should the whole of each file be hashed as well as its blocks + forcefullhashverify = _forcefullhashverify; + // Should we skip data whilst scanning files skipdata = _skipdata; @@ -1572,7 +1576,9 @@ bool Par2Repairer::ScanDataFileAligned(DiskFile *diskfile, // [i ProgressMeter &progress, // [in] Par2RepairerSourceFile *sourcefile, // [in] std::vector &matched, // [out] - u32 &matchcount) // [out] + u32 &matchcount, // [out] + MD5Hash &hashfull, // [out] + MD5Hash &hash16k) // [out] { matchcount = 0; @@ -1620,6 +1626,8 @@ bool Par2Repairer::ScanDataFileAligned(DiskFile *diskfile, // [i bool readfailed = false; + FileHasher filehasher; + // The blocks of a batch are next to each other, so they are read in one go. // Only the last block of a file can be short, and its entry covers it padded // out to the full block size with zeroes @@ -1636,6 +1644,9 @@ bool Par2Repairer::ScanDataFileAligned(DiskFile *diskfile, // [i return; } + if (forcefullhashverify) + filehasher.Update(offset, &into[0], length); + if (length < span) memset(&into[length], 0, span - length); }; @@ -1690,6 +1701,9 @@ bool Par2Repairer::ScanDataFileAligned(DiskFile *diskfile, // [i if (readfailed) return false; + if (forcefullhashverify) + filehasher.GetHashes(filesize, hashfull, hash16k); + for (u32 b=0; b alignedmatch; u32 alignedcount = 0; const bool aligned = ScanDataFileAligned(diskfile, progress, sourcefile, - alignedmatch, alignedcount); + alignedmatch, alignedcount, + hashfull, hash16k); // The parts of the file which still have to be searched a byte at a time std::vector > searchranges; @@ -1860,7 +1875,7 @@ bool Par2Repairer::ScanDataFile(DiskFile *diskfile, // [in] // The MD5 hash of the whole file is only needed to match against source // files which have no verification packet, and only when no block at all is // found. That can only happen when the whole of the file is being searched. - const bool computefilehashes = !unverifiablesourcefiles.empty() + const bool computefilehashes = ((forcefullhashverify && !aligned) || !unverifiablesourcefiles.empty()) && 1 == searchranges.size() && 0 == searchranges[0].first && filesize == searchranges[0].second; @@ -2088,7 +2103,10 @@ bool Par2Repairer::ScanDataFile(DiskFile *diskfile, // [in] // hash of the block to be verified. if (matchtype != eFullMatch || count != sourcefile->GetVerificationPacket()->BlockCount() || - diskfile->FileSize() != sourcefile->GetDescriptionPacket()->FileSize()) + diskfile->FileSize() != sourcefile->GetDescriptionPacket()->FileSize() || + (forcefullhashverify && + (hashfull != sourcefile->GetDescriptionPacket()->HashFull() || + hash16k != sourcefile->GetDescriptionPacket()->Hash16k()))) { matchtype = ePartialMatch; diff --git a/src/par2repairer.h b/src/par2repairer.h index 0ae6b666..e04849f0 100644 --- a/src/par2repairer.h +++ b/src/par2repairer.h @@ -39,7 +39,8 @@ class Par2Repairer const bool purgefiles, const bool renameonly, const bool skipdata, - const u64 skipleaway + const u64 skipleaway, + const bool forcefullhashverify ); protected: @@ -108,7 +109,9 @@ class Par2Repairer ProgressMeter &progress, // [in] Par2RepairerSourceFile *sourcefile, // [in] The file it should match std::vector &matched, // [out] One entry per block - u32 &matchcount); // [out] + u32 &matchcount, // [out] + MD5Hash &hashfull, // [out] Only set when the whole hash is wanted + MD5Hash &hash16k); // [out] // Perform a sliding window scan of the DiskFile looking for blocks of data that // might belong to any of the source files (for which a verification packet was @@ -180,6 +183,7 @@ class Par2Repairer static u32 filethreads; // Number of threads for file processing #endif + bool forcefullhashverify; // Should the whole of each file be hashed too bool skipdata; // Should we skip data whilst scanning u64 skipleaway; // The leaway +/- we should allow whilst scanning diff --git a/tests/full-hash-mismatch.tar.gz b/tests/full-hash-mismatch.tar.gz new file mode 100644 index 00000000..fb71b4b8 Binary files /dev/null and b/tests/full-hash-mismatch.tar.gz differ diff --git a/tests/generate-full-hash-mismatch-fixture.py b/tests/generate-full-hash-mismatch-fixture.py new file mode 100755 index 00000000..33d2cbdb --- /dev/null +++ b/tests/generate-full-hash-mismatch-fixture.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +"""Generate full-hash-mismatch.tar.gz from a freshly created PAR2 file. + +Every block of the data file still matches its verification entry, but the +whole file hash recorded in the file description packets does not match the +file. Verification only notices with --force-full-hash-verify. +""" + +import glob +import hashlib +import os +import pathlib +import struct +import subprocess +import sys +import tarfile +import tempfile + +MAGIC = b"PAR2\0PKT" +DESCRIPTION = b"PAR 2.0\0FileDesc" + + +def break_whole_file_hash(path): + data = bytearray(path.read_bytes()) + off = 0 + altered = 0 + + while off < len(data): + if data[off:off + 8] != MAGIC: + raise SystemExit(f"{path}: no packet magic at offset {off}") + length = struct.unpack_from(" len(data): + raise SystemExit(f"{path}: bad packet length at offset {off}") + + if data[off + 48:off + 64] == DESCRIPTION: + # header 64, then fileid 16, then the hash of the whole file + data[off + 80] ^= 0xFF + # the packet carries its own hash, over everything from the set id on + digest = hashlib.md5(bytes(data[off + 32:off + length])).digest() + data[off + 16:off + 32] = digest + altered += 1 + + off += length + + path.write_bytes(data) + return altered + + +def main(): + if len(sys.argv) != 3: + raise SystemExit( + "usage: generate-full-hash-mismatch-fixture.py PAR2_BINARY OUTPUT_TAR_GZ" + ) + + par2 = pathlib.Path(sys.argv[1]).resolve() + output = pathlib.Path(sys.argv[2]).resolve() + + with tempfile.TemporaryDirectory() as temp_name: + temp = pathlib.Path(temp_name) + data = temp / "data.bin" + data.write_bytes(bytes(range(256)) * 128) + + subprocess.run( + [str(par2), "c", "-q", "-s1024", "-c4", "recovery.par2", "data.bin"], + cwd=temp, + check=True, + ) + + altered = 0 + for name in sorted(glob.glob(str(temp / "*.par2"))): + altered += break_whole_file_hash(pathlib.Path(name)) + + if altered == 0: + raise SystemExit("no file description packets were found") + + with tarfile.open(output, "w:gz") as archive: + archive.add(data, arcname="data.bin") + for name in sorted(glob.glob(str(temp / "*.par2"))): + archive.add(name, arcname=os.path.basename(name)) + + +if __name__ == "__main__": + main() diff --git a/tests/test48 b/tests/test48 new file mode 100755 index 00000000..fedf0dd8 --- /dev/null +++ b/tests/test48 @@ -0,0 +1,72 @@ +#!/bin/sh + +execdir="$PWD" + +if [ -n "${PARVALGRINDOPTS+set}" ] +then + PARBINARY="valgrind $PARVALGRINDOPTS $execdir/par2" +elif [ "`which wine`" != "" ] && [ -f "$execdir/par2.exe" ] +then + PARBINARY="wine $execdir/par2.exe" +else + PARBINARY="$execdir/par2" +fi + +if [ -z "$srcdir" ] || [ "." = "$srcdir" ]; then + srcdir="$PWD" + TESTDATA="$srcdir/tests" +else + srcdir="$PWD/$srcdir" + TESTDATA="$srcdir/tests" +fi + +TESTROOT="$PWD" + +testname=$(basename $0) +rm -f "$testname.log" +rm -rf "run$testname" + +mkdir "run$testname" && cd "run$testname" || { echo "ERROR: Could not change to test directory" ; exit 1; } >&2 + +banner="Checking the hash of the whole of a file when asked to" +dashes=`echo "$banner" | sed s/./-/g` + +echo $dashes +echo $banner +echo $dashes + +tar -xzf "$TESTDATA/full-hash-mismatch.tar.gz" || { echo "ERROR: Could not extract PAR2 test file" ; exit 1; } >&2 + +# Every block of the file matches its verification entry, so the blocks alone +# say the file is intact. +$PARBINARY v -q recovery.par2 > blocks.out 2>&1 +blocks=$? + +if [ $blocks -ne 0 ] +then + echo "ERROR: Expected the file to verify when only its blocks are checked" >&2 + cat blocks.out >&2 + exit 1 +fi + +# The whole file hash recorded for it does not match, which only shows up +# when the whole of the file is hashed as well. A single thread leaves the +# whole file to the byte at a time search, and several threads have the thread +# which reads the file hash it, so both have to notice. +for threads in 1 4 +do + $PARBINARY v -q -t$threads --force-full-hash-verify recovery.par2 > whole.out 2>&1 + whole=$? + + if [ $whole -eq 0 ] + then + echo "ERROR: --force-full-hash-verify with -t$threads did not notice the whole file hash" >&2 + cat whole.out >&2 + exit 1 + fi +done + +cd "$TESTROOT" +rm -rf "run$testname" + +exit 0 diff --git a/tests/test48.ps1 b/tests/test48.ps1 new file mode 100644 index 00000000..1a75c7dc --- /dev/null +++ b/tests/test48.ps1 @@ -0,0 +1,44 @@ +#!/usr/bin/env pwsh +# Test 48: Checking the hash of the whole of a file when asked to + +$ErrorActionPreference = "Stop" + +# Source common test functions +. (Join-Path $PSScriptRoot "testfuncs.ps1") + +$testname = [System.IO.Path]::GetFileNameWithoutExtension($MyInvocation.MyCommand.Name) + +try { + Initialize-Test -TestName $testname + + Expand-TarGz -Archive (Join-Path $TESTDATA "full-hash-mismatch.tar.gz") -Destination "." + + Write-Banner "Checking the hash of the whole of a file when asked to" + + # Every block of the file matches its verification entry, so the blocks + # alone say the file is intact. + $blocks = Invoke-Par2 -Arguments @("v", "-q", "recovery.par2") -ReturnObject + if ($blocks.ExitCode -ne 0) { + Exit-TestWithError "Expected the file to verify when only its blocks are checked" + } + + # The whole file hash recorded for it does not match, which only shows up + # when the whole of the file is hashed as well. + # A single thread leaves the whole file to the byte at a time search, and + # several threads have the thread which reads the file hash it, so both + # have to notice. + foreach ($threads in 1, 4) { + $whole = Invoke-Par2 -Arguments @("v", "-q", "-t$threads", "--force-full-hash-verify", "recovery.par2") -ReturnObject + if ($whole.ExitCode -eq 0) { + Exit-TestWithError "--force-full-hash-verify with -t$threads did not notice the whole file hash" + } + } + + Complete-Test + exit 0 +} +catch { + Write-Host "ERROR: $_" -ForegroundColor Red + Complete-Test + exit 1 +}