Bug 1353956 - P4. Add header and CRC32 checksum to SafeBrowsing V4 prefix files. r=gcp
authordlee <dlee@mozilla.com>
Thu, 07 Mar 2019 14:41:25 +0000
changeset 520891 3b5da75b9c7b7912e31007b9bac31b4a1ff4a558
parent 520890 e083106dc24f6f562eb2d72aa02871a27a681ebe
child 520892 aaba7c25b72b9485fc6c71b879cbc4096f3f4f2b
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1353956
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1353956 - P4. Add header and CRC32 checksum to SafeBrowsing V4 prefix files. r=gcp After this patch, we may have the following files in SafeBrowsing directory: - (v2) .sbstore : Store V2 chunkdata, for update, MD5 integrity check while load - (v2) .pset : Store V2 prefixset, for lookup, load upon startup, no integrity check - (v4) .metadata : Store V4 state, for update, no integrity check - (v4) .vlpset : Store V4 prefixset, for lookup, load upon startup, CRC32 integrity check - (v4) .pset : V4 prefix set before this patch, should be removed The magic string is also added to ".vlpset" header so we can add a telemetry to see if sanity check is good enough for prefix set integrity check (The telemetry is not yet added). If yes, we can remove the CRC32 in the future for even better performance. Differential Revision: https://phabricator.services.mozilla.com/D21463
testing/firefox-ui/tests/functional/safebrowsing/test_initial_download.py
toolkit/components/url-classifier/LookupCache.cpp
toolkit/components/url-classifier/LookupCache.h
toolkit/components/url-classifier/LookupCacheV4.cpp
toolkit/components/url-classifier/LookupCacheV4.h
toolkit/components/url-classifier/nsCheckSummedOutputStream.cpp
toolkit/components/url-classifier/nsCheckSummedOutputStream.h
toolkit/components/url-classifier/tests/gtest/TestFailUpdate.cpp
toolkit/components/url-classifier/tests/gtest/TestUrlClassifierTableUpdateV4.cpp
--- a/testing/firefox-ui/tests/functional/safebrowsing/test_initial_download.py
+++ b/testing/firefox-ui/tests/functional/safebrowsing/test_initial_download.py
@@ -12,17 +12,17 @@ from marionette_harness import Marionett
 class TestSafeBrowsingInitialDownload(PuppeteerMixin, MarionetteTestCase):
 
     v2_file_extensions = [
         'pset',
         'sbstore',
     ]
 
     v4_file_extensions = [
-        'pset',
+        'vlpset',
         'metadata',
     ]
 
     prefs_download_lists = [
         'urlclassifier.blockedTable',
         'urlclassifier.downloadAllowTable',
         'urlclassifier.downloadBlockTable',
         'urlclassifier.malwareTable',
--- a/toolkit/components/url-classifier/LookupCache.cpp
+++ b/toolkit/components/url-classifier/LookupCache.cpp
@@ -26,19 +26,16 @@
 // PrefixSet stores and provides lookups for 4-byte prefixes.
 // mUpdateCompletions contains 32-byte completions which were
 // contained in updates. They are retrieved from HashStore/.sbtore
 // on startup.
 // mGetHashCache contains 32-byte completions which were
 // returned from the gethash server. They are not serialized,
 // only cached until the next update.
 
-// Name of the persistent PrefixSet storage
-#define PREFIXSET_SUFFIX ".pset"
-
 #define V2_CACHE_DURATION_SEC (15 * 60)
 
 // MOZ_LOG=UrlClassifierDbService:5
 extern mozilla::LazyLogModule gUrlClassifierDbServiceLog;
 #define LOG(args) \
   MOZ_LOG(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug, args)
 #define LOG_ENABLED() \
   MOZ_LOG_TEST(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug)
@@ -117,17 +114,17 @@ nsresult LookupCache::WriteFile() {
   if (nsUrlClassifierDBService::ShutdownHasStarted()) {
     return NS_ERROR_ABORT;
   }
 
   nsCOMPtr<nsIFile> psFile;
   nsresult rv = mStoreDirectory->Clone(getter_AddRefs(psFile));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = psFile->AppendNative(mTableName + NS_LITERAL_CSTRING(PREFIXSET_SUFFIX));
+  rv = psFile->AppendNative(mTableName + GetPrefixSetSuffix());
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = StoreToFile(psFile);
   NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "failed to store the prefixset");
 
   return NS_OK;
 }
 
@@ -436,17 +433,17 @@ nsresult LookupCache::GetHostKeys(const 
   return NS_OK;
 }
 
 nsresult LookupCache::LoadPrefixSet() {
   nsCOMPtr<nsIFile> psFile;
   nsresult rv = mStoreDirectory->Clone(getter_AddRefs(psFile));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = psFile->AppendNative(mTableName + NS_LITERAL_CSTRING(PREFIXSET_SUFFIX));
+  rv = psFile->AppendNative(mTableName + GetPrefixSetSuffix());
   NS_ENSURE_SUCCESS(rv, rv);
 
   bool exists;
   rv = psFile->Exists(&exists);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (exists) {
     LOG(("stored PrefixSet exists, loading from disk"));
@@ -721,16 +718,20 @@ nsresult LookupCacheV2::LoadFromFile(nsC
 
   return NS_OK;
 }
 
 size_t LookupCacheV2::SizeOfPrefixSet() const {
   return mPrefixSet->SizeOfIncludingThis(moz_malloc_size_of);
 }
 
+nsCString LookupCacheV2::GetPrefixSetSuffix() const {
+  return NS_LITERAL_CSTRING(".pset");
+}
+
 #ifdef DEBUG
 template <class T>
 static void EnsureSorted(T* aArray) {
   typename T::elem_type* start = aArray->Elements();
   typename T::elem_type* end = aArray->Elements() + aArray->Length();
   typename T::elem_type* iter = start;
   typename T::elem_type* previous = start;
 
--- a/toolkit/components/url-classifier/LookupCache.h
+++ b/toolkit/components/url-classifier/LookupCache.h
@@ -249,16 +249,17 @@ class LookupCache {
                 ? reinterpret_cast<const T*>(aThat)
                 : nullptr);
   }
 
  private:
   nsresult LoadPrefixSet();
 
   virtual size_t SizeOfPrefixSet() const = 0;
+  virtual nsCString GetPrefixSetSuffix() const = 0;
 
   virtual int Ver() const = 0;
 
  protected:
   virtual ~LookupCache() {}
 
   static const uint32_t MAX_BUFFER_SIZE;
 
@@ -316,16 +317,17 @@ class LookupCacheV2 final : public Looku
 
   static const int VER;
 
  protected:
   nsresult ReadCompletions();
 
   virtual nsresult ClearPrefixes() override;
   virtual size_t SizeOfPrefixSet() const override;
+  virtual nsCString GetPrefixSetSuffix() const override;
 
  private:
   ~LookupCacheV2() {}
 
   virtual int Ver() const override { return VER; }
 
   // Construct a Prefix Set with known prefixes.
   // This will Clear() aAddPrefixes when done.
--- a/toolkit/components/url-classifier/LookupCacheV4.cpp
+++ b/toolkit/components/url-classifier/LookupCacheV4.cpp
@@ -1,33 +1,126 @@
 //* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "LookupCacheV4.h"
 #include "HashStore.h"
 #include "mozilla/Unused.h"
+#include "nsCheckSummedOutputStream.h"
+#include "crc32c.h"
 #include <string>
 
 // MOZ_LOG=UrlClassifierDbService:5
 extern mozilla::LazyLogModule gUrlClassifierDbServiceLog;
 #define LOG(args) \
   MOZ_LOG(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug, args)
 #define LOG_ENABLED() \
   MOZ_LOG_TEST(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug)
 
 #define METADATA_SUFFIX NS_LITERAL_CSTRING(".metadata")
+namespace {
+
+static const uint64_t STREAM_BUFFER_SIZE = 4096;
+
+//////////////////////////////////////////////////////////////////////////
+// A set of lightweight functions for reading/writing value from/to file.
+template <typename T>
+struct ValueTraits {
+  static_assert(sizeof(T) <= LookupCacheV4::MAX_METADATA_VALUE_LENGTH,
+                "LookupCacheV4::MAX_METADATA_VALUE_LENGTH is too small.");
+  static uint32_t Length(const T& aValue) { return sizeof(T); }
+  static char* WritePtr(T& aValue, uint32_t aLength) { return (char*)&aValue; }
+  static const char* ReadPtr(const T& aValue) { return (char*)&aValue; }
+  static bool IsFixedLength() { return true; }
+};
+
+template <>
+struct ValueTraits<nsACString> {
+  static bool IsFixedLength() { return false; }
+
+  static uint32_t Length(const nsACString& aValue) { return aValue.Length(); }
+
+  static char* WritePtr(nsACString& aValue, uint32_t aLength) {
+    aValue.SetLength(aLength);
+    return aValue.BeginWriting();
+  }
+
+  static const char* ReadPtr(const nsACString& aValue) {
+    return aValue.BeginReading();
+  }
+};
+
+template <typename T>
+static nsresult WriteValue(nsIOutputStream* aOutputStream, const T& aValue) {
+  uint32_t writeLength = ValueTraits<T>::Length(aValue);
+  MOZ_ASSERT(writeLength <= LookupCacheV4::MAX_METADATA_VALUE_LENGTH,
+             "LookupCacheV4::MAX_METADATA_VALUE_LENGTH is too small.");
+  if (!ValueTraits<T>::IsFixedLength()) {
+    // We need to write out the variable value length.
+    nsresult rv = WriteValue(aOutputStream, writeLength);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  // Write out the value.
+  auto valueReadPtr = ValueTraits<T>::ReadPtr(aValue);
+  uint32_t written;
+  nsresult rv = aOutputStream->Write(valueReadPtr, writeLength, &written);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(written != writeLength)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  return rv;
+}
+
+template <typename T>
+static nsresult ReadValue(nsIInputStream* aInputStream, T& aValue) {
+  nsresult rv;
+
+  uint32_t readLength;
+  if (ValueTraits<T>::IsFixedLength()) {
+    readLength = ValueTraits<T>::Length(aValue);
+  } else {
+    // Read the variable value length from file.
+    nsresult rv = ReadValue(aInputStream, readLength);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  // Sanity-check the readLength in case of disk corruption
+  // (see bug 1433636).
+  if (readLength > LookupCacheV4::MAX_METADATA_VALUE_LENGTH) {
+    return NS_ERROR_FILE_CORRUPTED;
+  }
+
+  // Read the value.
+  uint32_t read;
+  auto valueWritePtr = ValueTraits<T>::WritePtr(aValue, readLength);
+  rv = aInputStream->Read(valueWritePtr, readLength, &read);
+  if (NS_FAILED(rv) || read != readLength) {
+    LOG(("Failed to read the value."));
+    return NS_FAILED(rv) ? rv : NS_ERROR_FAILURE;
+  }
+
+  return rv;
+}
+
+}  // end of unnamed namespace.
+////////////////////////////////////////////////////////////////////////
 
 namespace mozilla {
 namespace safebrowsing {
 
 const int LookupCacheV4::VER = 4;
 const uint32_t LookupCacheV4::MAX_METADATA_VALUE_LENGTH = 256;
 
+const uint32_t VLPSET_MAGIC = 0x36044a35;
+const uint32_t VLPSET_VERSION = 1;
+
 // Prefixes coming from updates and VLPrefixSet are both stored in the HashTable
 // where the (key, value) pair is a prefix size and a lexicographic-sorted
 // string. The difference is prefixes from updates use std:string(to avoid
 // additional copies) and prefixes from VLPrefixSet use nsCString. This class
 // provides a common interface for the partial update algorithm to make it
 // easier to operate on two different kind prefix string map..
 class VLPrefixSet {
  public:
@@ -161,88 +254,153 @@ nsresult LookupCacheV4::ClearPrefixes() 
   // Clear by seting a empty map
   PrefixStringMap map;
   return mVLPrefixSet->SetPrefixes(map);
 }
 
 nsresult LookupCacheV4::StoreToFile(nsCOMPtr<nsIFile>& aFile) {
   NS_ENSURE_ARG_POINTER(aFile);
 
+  uint32_t fileSize = sizeof(Header) +
+                      mVLPrefixSet->CalculatePreallocateSize() +
+                      nsCrc32CheckSumedOutputStream::CHECKSUM_SIZE;
+
   nsCOMPtr<nsIOutputStream> localOutFile;
   nsresult rv =
-      NS_NewLocalFileOutputStream(getter_AddRefs(localOutFile), aFile,
-                                  PR_WRONLY | PR_TRUNCATE | PR_CREATE_FILE);
-  NS_ENSURE_SUCCESS(rv, rv);
+      NS_NewSafeLocalFileOutputStream(getter_AddRefs(localOutFile), aFile,
+                                      PR_WRONLY | PR_TRUNCATE | PR_CREATE_FILE);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
-  uint32_t fileSize = 0;
   // Preallocate the file storage
   {
     nsCOMPtr<nsIFileOutputStream> fos(do_QueryInterface(localOutFile));
     Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_VLPS_FALLOCATE_TIME> timer;
 
-    fileSize = mVLPrefixSet->CalculatePreallocateSize();
-
     Unused << fos->Preallocate(fileSize);
   }
 
-  // Convert to buffered stream
   nsCOMPtr<nsIOutputStream> out;
-  rv = NS_NewBufferedOutputStream(getter_AddRefs(out), localOutFile.forget(),
-                                  std::min(fileSize, MAX_BUFFER_SIZE));
-  NS_ENSURE_SUCCESS(rv, rv);
+  rv = NS_NewCrc32OutputStream(getter_AddRefs(out), localOutFile.forget(),
+                               std::min(fileSize, MAX_BUFFER_SIZE));
+
+  // Write header
+  Header header = {.magic = VLPSET_MAGIC, .version = VLPSET_VERSION};
+  rv = WriteValue(out, header);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
+  // Write prefixes
   rv = mVLPrefixSet->WritePrefixes(out);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  // Write checksum
+  nsCOMPtr<nsISafeOutputStream> safeOut = do_QueryInterface(out, &rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  rv = safeOut->Finish();
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   LOG(("[%s] Storing PrefixSet successful", mTableName.get()));
   return NS_OK;
 }
 
 nsresult LookupCacheV4::LoadFromFile(nsCOMPtr<nsIFile>& aFile) {
+  NS_ENSURE_ARG_POINTER(aFile);
+
   Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_VLPS_FILELOAD_TIME> timer;
 
   nsCOMPtr<nsIInputStream> localInFile;
   nsresult rv = NS_NewLocalFileInputStream(getter_AddRefs(localInFile), aFile,
                                            PR_RDONLY | nsIFile::OS_READAHEAD);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   // Calculate how big the file is, make sure our read buffer isn't bigger
   // than the file itself which is just wasting memory.
   int64_t fileSize;
   rv = aFile->GetFileSize(&fileSize);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   if (fileSize < 0 || fileSize > UINT32_MAX) {
     return NS_ERROR_FAILURE;
   }
 
   uint32_t bufferSize =
       std::min<uint32_t>(static_cast<uint32_t>(fileSize), MAX_BUFFER_SIZE);
 
   // Convert to buffered stream
   nsCOMPtr<nsIInputStream> in;
   rv = NS_NewBufferedInputStream(getter_AddRefs(in), localInFile.forget(),
                                  bufferSize);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
+  // Load header
+  Header header;
+  rv = ReadValue(in, header);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  rv = SanityCheck(header);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  // Load data
   rv = mVLPrefixSet->LoadPrefixes(in);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
+  // Load crc32 checksum and verify
+  rv = VerifyCRC32(in);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
   mPrimed = true;
+
   LOG(("[%s] Loading PrefixSet successful", mTableName.get()));
+  return NS_OK;
+}
+
+nsresult LookupCacheV4::SanityCheck(const Header& aHeader) {
+  if (aHeader.magic != VLPSET_MAGIC) {
+    return NS_ERROR_FILE_CORRUPTED;
+  }
+
+  if (aHeader.version != VLPSET_VERSION) {
+    return NS_ERROR_FAILURE;
+  }
 
   return NS_OK;
 }
 
 size_t LookupCacheV4::SizeOfPrefixSet() const {
   return mVLPrefixSet->SizeOfIncludingThis(moz_malloc_size_of);
 }
 
+nsCString LookupCacheV4::GetPrefixSetSuffix() const {
+  return NS_LITERAL_CSTRING(".vlpset");
+}
+
 static nsresult AppendPrefixToMap(PrefixStringMap& prefixes,
                                   const nsACString& prefix) {
   uint32_t len = prefix.Length();
   MOZ_ASSERT(len >= PREFIX_SIZE && len <= COMPLETE_SIZE);
   if (!len) {
     return NS_OK;
   }
 
@@ -411,104 +569,65 @@ nsresult LookupCacheV4::ApplyUpdate(RefP
 
 nsresult LookupCacheV4::AddFullHashResponseToCache(
     const FullHashResponseMap& aResponseMap) {
   CopyClassHashTable<FullHashResponseMap>(aResponseMap, mFullHashCache);
 
   return NS_OK;
 }
 
-//////////////////////////////////////////////////////////////////////////
-// A set of lightweight functions for reading/writing value from/to file.
-
-namespace {
+// This function assumes CRC32 checksum is in the end of the input stream
+nsresult LookupCacheV4::VerifyCRC32(nsCOMPtr<nsIInputStream>& aIn) {
+  nsCOMPtr<nsISeekableStream> seekIn = do_QueryInterface(aIn);
+  nsresult rv = seekIn->Seek(nsISeekableStream::NS_SEEK_SET, 0);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
-template <typename T>
-struct ValueTraits {
-  static_assert(sizeof(T) <= LookupCacheV4::MAX_METADATA_VALUE_LENGTH,
-                "LookupCacheV4::MAX_METADATA_VALUE_LENGTH is too small.");
-  static uint32_t Length(const T& aValue) { return sizeof(T); }
-  static char* WritePtr(T& aValue, uint32_t aLength) { return (char*)&aValue; }
-  static const char* ReadPtr(const T& aValue) { return (char*)&aValue; }
-  static bool IsFixedLength() { return true; }
-};
-
-template <>
-struct ValueTraits<nsACString> {
-  static bool IsFixedLength() { return false; }
-
-  static uint32_t Length(const nsACString& aValue) { return aValue.Length(); }
-
-  static char* WritePtr(nsACString& aValue, uint32_t aLength) {
-    aValue.SetLength(aLength);
-    return aValue.BeginWriting();
+  uint64_t len;
+  rv = aIn->Available(&len);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
   }
 
-  static const char* ReadPtr(const nsACString& aValue) {
-    return aValue.BeginReading();
-  }
-};
+  uint32_t calculateCrc32 = ~0;
+
+  // We don't want to include the checksum itself
+  len = len - nsCrc32CheckSumedOutputStream::CHECKSUM_SIZE;
+
+  char buffer[STREAM_BUFFER_SIZE];
+  while (len) {
+    uint32_t read;
+    uint64_t readLimit = std::min<uint64_t>(STREAM_BUFFER_SIZE, len);
 
-template <typename T>
-static nsresult WriteValue(nsIOutputStream* aOutputStream, const T& aValue) {
-  uint32_t writeLength = ValueTraits<T>::Length(aValue);
-  MOZ_ASSERT(writeLength <= LookupCacheV4::MAX_METADATA_VALUE_LENGTH,
-             "LookupCacheV4::MAX_METADATA_VALUE_LENGTH is too small.");
-  if (!ValueTraits<T>::IsFixedLength()) {
-    // We need to write out the variable value length.
-    nsresult rv = WriteValue(aOutputStream, writeLength);
-    NS_ENSURE_SUCCESS(rv, rv);
+    rv = aIn->Read(buffer, readLimit, &read);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+
+    calculateCrc32 = ComputeCrc32c(
+        calculateCrc32, reinterpret_cast<const uint8_t*>(buffer), read);
+
+    len -= read;
   }
 
-  // Write out the value.
-  auto valueReadPtr = ValueTraits<T>::ReadPtr(aValue);
-  uint32_t written;
-  nsresult rv = aOutputStream->Write(valueReadPtr, writeLength, &written);
-  NS_ENSURE_SUCCESS(rv, rv);
-  if (NS_WARN_IF(written != writeLength)) {
-    return NS_ERROR_FAILURE;
+  // Now read the CRC32
+  uint32_t crc32;
+  ReadValue(aIn, crc32);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
   }
 
-  return rv;
-}
-
-template <typename T>
-static nsresult ReadValue(nsIInputStream* aInputStream, T& aValue) {
-  nsresult rv;
-
-  uint32_t readLength;
-  if (ValueTraits<T>::IsFixedLength()) {
-    readLength = ValueTraits<T>::Length(aValue);
-  } else {
-    // Read the variable value length from file.
-    nsresult rv = ReadValue(aInputStream, readLength);
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
-  // Sanity-check the readLength in case of disk corruption
-  // (see bug 1433636).
-  if (readLength > LookupCacheV4::MAX_METADATA_VALUE_LENGTH) {
+  if (crc32 != calculateCrc32) {
     return NS_ERROR_FILE_CORRUPTED;
   }
 
-  // Read the value.
-  uint32_t read;
-  auto valueWritePtr = ValueTraits<T>::WritePtr(aValue, readLength);
-  rv = aInputStream->Read(valueWritePtr, readLength, &read);
-  if (NS_FAILED(rv) || read != readLength) {
-    LOG(("Failed to read the value."));
-    return NS_FAILED(rv) ? rv : NS_ERROR_FAILURE;
-  }
-
-  return rv;
+  return NS_OK;
 }
 
-}  // end of unnamed namespace.
-////////////////////////////////////////////////////////////////////////
-
 nsresult LookupCacheV4::WriteMetadata(
     RefPtr<const TableUpdateV4> aTableUpdate) {
   NS_ENSURE_ARG_POINTER(aTableUpdate);
   if (nsUrlClassifierDBService::ShutdownHasStarted()) {
     return NS_ERROR_ABORT;
   }
 
   nsCOMPtr<nsIFile> metaFile;
--- a/toolkit/components/url-classifier/LookupCacheV4.h
+++ b/toolkit/components/url-classifier/LookupCacheV4.h
@@ -46,21 +46,31 @@ class LookupCacheV4 final : public Looku
   nsresult LoadMetadata(nsACString& aState, nsACString& aChecksum);
 
   static const int VER;
   static const uint32_t MAX_METADATA_VALUE_LENGTH;
 
  protected:
   virtual nsresult ClearPrefixes() override;
   virtual size_t SizeOfPrefixSet() const override;
+  virtual nsCString GetPrefixSetSuffix() const override;
+  nsCString GetMetadataSuffix() const;
 
  private:
   ~LookupCacheV4() {}
 
   virtual int Ver() const override { return VER; }
 
+  struct Header {
+    uint32_t magic;
+    uint32_t version;
+  };
+
+  nsresult SanityCheck(const Header& aHeader);
+  nsresult VerifyCRC32(nsCOMPtr<nsIInputStream>& aIn);
+
   RefPtr<VariableLengthPrefixSet> mVLPrefixSet;
 };
 
 }  // namespace safebrowsing
 }  // namespace mozilla
 
 #endif
--- a/toolkit/components/url-classifier/nsCheckSummedOutputStream.cpp
+++ b/toolkit/components/url-classifier/nsCheckSummedOutputStream.cpp
@@ -2,16 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsCRT.h"
 #include "nsIFile.h"
 #include "nsISupportsImpl.h"
 #include "nsCheckSummedOutputStream.h"
+#include "crc32c.h"
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsCheckSummedOutputStream
 
 NS_IMPL_ISUPPORTS_INHERITED(nsCheckSummedOutputStream, nsBufferedOutputStream,
                             nsISafeOutputStream)
 
 NS_IMETHODIMP
@@ -46,8 +47,41 @@ nsCheckSummedOutputStream::Write(const c
                                  uint32_t *result) {
   nsresult rv = mHash->Update(reinterpret_cast<const uint8_t *>(buf), count);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return nsBufferedOutputStream::Write(buf, count, result);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// nsCrc32CheckSumedOutputStream
+NS_IMPL_ISUPPORTS_INHERITED(nsCrc32CheckSumedOutputStream,
+                            nsBufferedOutputStream, nsISafeOutputStream)
+
+NS_IMETHODIMP
+nsCrc32CheckSumedOutputStream::Init(nsIOutputStream *stream,
+                                    uint32_t bufferSize) {
+  mCheckSum = ~0;
+
+  return nsBufferedOutputStream::Init(stream, bufferSize);
+}
+
+NS_IMETHODIMP
+nsCrc32CheckSumedOutputStream::Finish() {
+  uint32_t written;
+  nsresult rv = nsBufferedOutputStream::Write(
+      reinterpret_cast<const char *>(&mCheckSum), sizeof(mCheckSum), &written);
+  NS_ASSERTION(written == sizeof(mCheckSum), "Error writing stream checksum");
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return nsBufferedOutputStream::Finish();
+}
+
+NS_IMETHODIMP
+nsCrc32CheckSumedOutputStream::Write(const char *buf, uint32_t count,
+                                     uint32_t *result) {
+  mCheckSum =
+      ComputeCrc32c(mCheckSum, reinterpret_cast<const uint8_t *>(buf), count);
+
+  return nsBufferedOutputStream::Write(buf, count, result);
+}
+
+////////////////////////////////////////////////////////////////////////////////
--- a/toolkit/components/url-classifier/nsCheckSummedOutputStream.h
+++ b/toolkit/components/url-classifier/nsCheckSummedOutputStream.h
@@ -48,9 +48,41 @@ inline nsresult NS_NewCheckSummedOutputS
   nsCOMPtr<nsIBufferedOutputStream> out = new nsCheckSummedOutputStream();
   rv = out->Init(localOutFile, nsCheckSummedOutputStream::CHECKSUM_SIZE);
   if (NS_SUCCEEDED(rv)) {
     out.forget(result);
   }
   return rv;
 }
 
+class nsCrc32CheckSumedOutputStream : public nsBufferedOutputStream {
+ public:
+  NS_DECL_ISUPPORTS_INHERITED
+
+  static const uint32_t CHECKSUM_SIZE = 4;
+
+  nsCrc32CheckSumedOutputStream() = default;
+
+  NS_IMETHOD Finish() override;
+  NS_IMETHOD Write(const char *buf, uint32_t count, uint32_t *result) override;
+  NS_IMETHOD Init(nsIOutputStream *stream, uint32_t bufferSize) override;
+
+ protected:
+  virtual ~nsCrc32CheckSumedOutputStream() { nsBufferedOutputStream::Close(); }
+
+  uint32_t mCheckSum;
+};
+
+inline nsresult NS_NewCrc32OutputStream(
+    nsIOutputStream **aResult, already_AddRefed<nsIOutputStream> aOutput,
+    uint32_t aBufferSize) {
+  nsCOMPtr<nsIOutputStream> out = std::move(aOutput);
+
+  nsCOMPtr<nsIBufferedOutputStream> bufferOutput =
+      new nsCrc32CheckSumedOutputStream();
+  nsresult rv = bufferOutput->Init(out, aBufferSize);
+  if (NS_SUCCEEDED(rv)) {
+    bufferOutput.forget(aResult);
+  }
+  return rv;
+}
+
 #endif
--- a/toolkit/components/url-classifier/tests/gtest/TestFailUpdate.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestFailUpdate.cpp
@@ -3,17 +3,17 @@
 #include "string.h"
 #include "gtest/gtest.h"
 #include "mozilla/Unused.h"
 
 using namespace mozilla;
 using namespace mozilla::safebrowsing;
 
 static const char* kFilesInV2[] = {".pset", ".sbstore"};
-static const char* kFilesInV4[] = {".pset", ".metadata"};
+static const char* kFilesInV4[] = {".vlpset", ".metadata"};
 
 #define V2_TABLE "gtest-malware-simple"
 #define V4_TABLE1 "goog-malware-proto"
 #define V4_TABLE2 "goog-phish-proto"
 
 #define ROOT_DIR NS_LITERAL_STRING("safebrowsing")
 #define SB_FILE(x, y) NS_ConvertUTF8toUTF16(nsPrintfCString("%s%s", x, y))
 
@@ -58,17 +58,17 @@ TEST(UrlClassifierFailUpdate, CheckTable
   // Apply V4 update for table1
   {
     RefPtr<TableUpdateV4> update =
         new TableUpdateV4(NS_LITERAL_CSTRING(V4_TABLE1));
     func(update, FULL_UPDATE, "test_prefix");
 
     ApplyUpdate(update);
 
-    // A successful V4 update should create .pset & .metadata files
+    // A successful V4 update should create .vlpset & .metadata files
     CheckFileExist(V4_TABLE1, kFilesInV4, true);
   }
 
   // Apply V4 update for table2
   {
     RefPtr<TableUpdateV4> update =
         new TableUpdateV4(NS_LITERAL_CSTRING(V4_TABLE2));
     func(update, FULL_UPDATE, "test_prefix");
--- a/toolkit/components/url-classifier/tests/gtest/TestUrlClassifierTableUpdateV4.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestUrlClassifierTableUpdateV4.cpp
@@ -16,17 +16,17 @@
 using namespace mozilla;
 using namespace mozilla::safebrowsing;
 
 typedef nsCString _Prefix;
 typedef nsTArray<_Prefix> _PrefixArray;
 
 #define GTEST_SAFEBROWSING_DIR NS_LITERAL_CSTRING("safebrowsing")
 #define GTEST_TABLE NS_LITERAL_CSTRING("gtest-malware-proto")
-#define GTEST_PREFIXFILE NS_LITERAL_CSTRING("gtest-malware-proto.pset")
+#define GTEST_PREFIXFILE NS_LITERAL_CSTRING("gtest-malware-proto.vlpset")
 
 // This function removes common elements of inArray and outArray from
 // outArray. This is used by partial update testcase to ensure partial update
 // data won't contain prefixes we already have.
 static void RemoveIntersection(const _PrefixArray& inArray,
                                _PrefixArray& outArray) {
   for (uint32_t i = 0; i < inArray.Length(); i++) {
     int32_t idx = outArray.BinaryIndexOf(inArray[i]);