From: Joel Rosdahl Date: Sun, 6 Oct 2019 20:21:46 +0000 (+0200) Subject: Let cache entry writer/reader update/verify checksums X-Git-Tag: v4.0~754 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=42e179e82b24a8fd0dd92bd227054dfe2341277e;p=thirdparty%2Fccache.git Let cache entry writer/reader update/verify checksums --- diff --git a/src/CacheEntryReader.cpp b/src/CacheEntryReader.cpp index 82dae950c..8014fe8e1 100644 --- a/src/CacheEntryReader.cpp +++ b/src/CacheEntryReader.cpp @@ -18,7 +18,6 @@ #include "CacheEntryReader.hpp" -#include "Checksum.hpp" #include "Compressor.hpp" #include "Error.hpp" #include "ccache.hpp" @@ -27,9 +26,7 @@ CacheEntryReader::CacheEntryReader(FILE* stream, const uint8_t expected_magic[4], - uint8_t expected_version, - Checksum* checksum) - : m_checksum(checksum) + uint8_t expected_version) { uint8_t header_bytes[15]; if (fread(header_bytes, sizeof(header_bytes), 1, stream) != 1) { @@ -69,10 +66,7 @@ CacheEntryReader::CacheEntryReader(FILE* stream, } } - if (m_checksum) { - m_checksum->update(header_bytes, sizeof(header_bytes)); - } - + m_checksum.update(header_bytes, sizeof(header_bytes)); m_decompressor = Decompressor::create_from_type(m_compression_type, stream); } @@ -92,11 +86,25 @@ void CacheEntryReader::read(void* data, size_t count) { m_decompressor->read(data, count); - m_checksum->update(data, count); + m_checksum.update(data, count); } void CacheEntryReader::finalize() { + uint64_t actual_digest = m_checksum.digest(); + + uint8_t buffer[8]; + read(buffer, sizeof(buffer)); + uint64_t expected_digest; + Util::big_endian_to_int(buffer, expected_digest); + + if (actual_digest != expected_digest) { + throw Error( + fmt::format("Incorrect checksum (actual 0x{:016x}, expected 0x{:016x})", + actual_digest, + expected_digest)); + } + m_decompressor->finalize(); } diff --git a/src/CacheEntryReader.hpp b/src/CacheEntryReader.hpp index 17f4682e4..40a377c0c 100644 --- a/src/CacheEntryReader.hpp +++ b/src/CacheEntryReader.hpp @@ -18,14 +18,13 @@ #pragma once +#include "Checksum.hpp" #include "Decompressor.hpp" #include "Util.hpp" #include #include -class Checksum; - // This class knows how to read a cache entry with a common header and a // payload part that is different depending on the cache entry type (result or // manifest). @@ -38,12 +37,10 @@ public: // - stream: Stream to read header and payload from. // - expected_magic: Expected magic bytes (first four bytes of the file). // - expected_version: Expected file format version. - // - checksum: Checksum state that will be updated with the read bytes, or // nullptr for no checksumming. CacheEntryReader(FILE* stream, const uint8_t expected_magic[4], - uint8_t expected_version, - Checksum* checksum = nullptr); + uint8_t expected_version); // Dump header information in text format. // @@ -74,12 +71,15 @@ public: // entry and throws Error if any integrity issues are found. void finalize(); - // Get size of the content (header + payload). + // Get size of the payload, + uint64_t payload_size() const; + + // Get size of the content (header + payload + checksum). uint64_t content_size() const; private: std::unique_ptr m_decompressor; - Checksum* m_checksum; + Checksum m_checksum; char m_magic[4]; uint8_t m_version; Compression::Type m_compression_type; @@ -96,6 +96,12 @@ CacheEntryReader::read(T& value) Util::big_endian_to_int(buffer, value); } +inline uint64_t +CacheEntryReader::payload_size() const +{ + return m_content_size - 15 - 8; +} + inline uint64_t CacheEntryReader::content_size() const { diff --git a/src/CacheEntryWriter.cpp b/src/CacheEntryWriter.cpp index 156963901..e030a277d 100644 --- a/src/CacheEntryWriter.cpp +++ b/src/CacheEntryWriter.cpp @@ -18,29 +18,26 @@ #include "CacheEntryWriter.hpp" -#include "Checksum.hpp" - CacheEntryWriter::CacheEntryWriter(FILE* stream, const uint8_t magic[4], uint8_t version, Compression::Type compression_type, int8_t compression_level, - uint64_t content_size, - Checksum& checksum) + uint64_t payload_size) : m_compressor( - Compressor::create_from_type(compression_type, stream, compression_level)), - m_checksum(checksum) + Compressor::create_from_type(compression_type, stream, compression_level)) { uint8_t header_bytes[15]; memcpy(header_bytes, magic, 4); header_bytes[4] = version; header_bytes[5] = static_cast(compression_type); header_bytes[6] = m_compressor->actual_compression_level(); + uint64_t content_size = 15 + payload_size + 8; Util::int_to_big_endian(content_size, header_bytes + 7); if (fwrite(header_bytes, sizeof(header_bytes), 1, stream) != 1) { throw Error("Failed to write cache entry header"); } - checksum.update(header_bytes, sizeof(header_bytes)); + m_checksum.update(header_bytes, sizeof(header_bytes)); } void @@ -53,5 +50,8 @@ CacheEntryWriter::write(const void* data, size_t count) void CacheEntryWriter::finalize() { + uint8_t buffer[8]; + Util::int_to_big_endian(m_checksum.digest(), buffer); + m_compressor->write(buffer, sizeof(buffer)); m_compressor->finalize(); } diff --git a/src/CacheEntryWriter.hpp b/src/CacheEntryWriter.hpp index aa02a01b5..338aa173b 100644 --- a/src/CacheEntryWriter.hpp +++ b/src/CacheEntryWriter.hpp @@ -18,14 +18,13 @@ #pragma once +#include "Checksum.hpp" #include "Compressor.hpp" #include "Util.hpp" #include #include -class Checksum; - // This class knows how to write a cache entry with a common header and a // payload part that is different depending on the cache entry type (result or // manifest). @@ -40,15 +39,13 @@ public: // - version: File format version. // - compression_type: Compression type to use. // - compression_level: Compression level to use. - // - content_size: Content size. - // - checksum: Checksum state that will be updated with the written bytes. + // - payload_size: Payload size. CacheEntryWriter(FILE* stream, const uint8_t magic[4], uint8_t version, Compression::Type compression_type, int8_t compression_level, - uint64_t content_size, - Checksum& checksum); + uint64_t payload_size); // Write data to the payload from a buffer. // @@ -75,7 +72,7 @@ public: private: std::unique_ptr m_compressor; - Checksum& m_checksum; + Checksum m_checksum; }; template diff --git a/src/manifest.cpp b/src/manifest.cpp index 36d81163d..07658ccbe 100644 --- a/src/manifest.cpp +++ b/src/manifest.cpp @@ -264,9 +264,7 @@ read_manifest(const std::string& path, FILE* dump_stream = nullptr) return {}; } - Checksum checksum; - CacheEntryReader reader( - file.get(), k_manifest_magic, k_manifest_version, &checksum); + CacheEntryReader reader(file.get(), k_manifest_magic, k_manifest_version); if (dump_stream) { reader.dump_header(dump_stream); @@ -313,16 +311,6 @@ read_manifest(const std::string& path, FILE* dump_stream = nullptr) reader.read(entry.name.bytes, DIGEST_SIZE); } - uint64_t actual_checksum = checksum.digest(); - uint64_t expected_checksum; - reader.read(expected_checksum); - if (actual_checksum != expected_checksum) { - throw Error( - fmt::format("Incorrect checksum (actual 0x{:016x}, expected 0x{:016x})", - actual_checksum, - expected_checksum)); - } - reader.finalize(); return mf; } @@ -330,30 +318,27 @@ read_manifest(const std::string& path, FILE* dump_stream = nullptr) static bool write_manifest(const std::string& path, const ManifestData& mf) { - uint64_t content_size = 15; - content_size += 4; // n_files + uint64_t payload_size = 0; + payload_size += 4; // n_files for (size_t i = 0; i < mf.files.size(); ++i) { - content_size += 2 + mf.files[i].length(); + payload_size += 2 + mf.files[i].length(); } - content_size += 4; // n_file_infos - content_size += mf.file_infos.size() * (4 + DIGEST_SIZE + 8 + 8 + 8); - content_size += 4; // n_results + payload_size += 4; // n_file_infos + payload_size += mf.file_infos.size() * (4 + DIGEST_SIZE + 8 + 8 + 8); + payload_size += 4; // n_results for (size_t i = 0; i < mf.results.size(); ++i) { - content_size += 4; // n_file_info_indexes - content_size += mf.results[i].file_info_indexes.size() * 4; - content_size += DIGEST_SIZE; + payload_size += 4; // n_file_info_indexes + payload_size += mf.results[i].file_info_indexes.size() * 4; + payload_size += DIGEST_SIZE; } - content_size += 8; // checksum - Checksum checksum; AtomicFile atomic_manifest_file(path, AtomicFile::Mode::binary); CacheEntryWriter writer(atomic_manifest_file.stream(), k_manifest_magic, k_manifest_version, Compression::type_from_config(), Compression::level_from_config(), - content_size, - checksum); + payload_size); writer.write(mf.files.size()); for (uint32_t i = 0; i < mf.files.size(); ++i) { writer.write(mf.files[i].length()); @@ -378,7 +363,6 @@ write_manifest(const std::string& path, const ManifestData& mf) writer.write(mf.results[i].name.bytes, DIGEST_SIZE); } - writer.write(checksum.digest()); writer.finalize(); atomic_manifest_file.commit(); return true; diff --git a/src/result.cpp b/src/result.cpp index 18c959689..15b0a18c2 100644 --- a/src/result.cpp +++ b/src/result.cpp @@ -21,7 +21,6 @@ #include "AtomicFile.hpp" #include "CacheEntryReader.hpp" #include "CacheEntryWriter.hpp" -#include "Checksum.hpp" #include "Config.hpp" #include "Error.hpp" #include "File.hpp" @@ -280,9 +279,7 @@ read_result(const std::string& path, return false; } - Checksum checksum; - CacheEntryReader reader( - file.get(), k_result_magic, k_result_version, &checksum); + CacheEntryReader reader(file.get(), k_result_magic, k_result_version); if (dump_stream) { reader.dump_header(dump_stream); @@ -319,16 +316,6 @@ read_result(const std::string& path, fmt::format("Too few entries (read {}, expected {})", i, n_entries)); } - uint64_t actual_checksum = checksum.digest(); - uint64_t expected_checksum; - reader.read(expected_checksum); - if (actual_checksum != expected_checksum) { - throw Error( - fmt::format("Incorrect checksum (actual 0x{:016x}, expected 0x{:016x})", - actual_checksum, - expected_checksum)); - } - reader.finalize(); return true; } @@ -449,8 +436,8 @@ should_store_raw_file(const std::string& suffix) static void write_result(const std::string& path, const ResultFileMap& result_file_map) { - uint64_t content_size = 15; - content_size += 1; // n_entries + uint64_t payload_size = 0; + payload_size += 1; // n_entries for (const auto& pair : result_file_map) { const auto& suffix = pair.first; const auto& result_file = pair.second; @@ -459,23 +446,20 @@ write_result(const std::string& path, const ResultFileMap& result_file_map) throw Error( fmt::format("Failed to stat {}: {}", result_file, strerror(errno))); } - content_size += 1; // embedded_file_marker - content_size += 1; // suffix_len - content_size += suffix.length(); // suffix - content_size += 8; // data_len - content_size += source_file_size; // data + payload_size += 1; // embedded_file_marker + payload_size += 1; // suffix_len + payload_size += suffix.length(); // suffix + payload_size += 8; // data_len + payload_size += source_file_size; // data } - content_size += 8; // checksum - Checksum checksum; AtomicFile atomic_result_file(path, AtomicFile::Mode::binary); CacheEntryWriter writer(atomic_result_file.stream(), k_result_magic, k_result_version, Compression::type_from_config(), Compression::level_from_config(), - content_size, - checksum); + payload_size); writer.write(result_file_map.size()); @@ -489,7 +473,6 @@ write_result(const std::string& path, const ResultFileMap& result_file_map) ++entry_number; } - writer.write(checksum.digest()); writer.finalize(); atomic_result_file.commit(); }