Bug 1353956 - P2. Do not use SHA-256 while loading the V4 prefix files. r=gcp
authordlee <dlee@mozilla.com>
Thu, 07 Mar 2019 14:40:28 +0000
changeset 520889 c7a253aed4508a8df4f1309d889dbfd669cd1693
parent 520888 c2331373e10707aa5259eb3d8436f65c8d59c684
child 520890 e083106dc24f6f562eb2d72aa02871a27a681ebe
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 - P2. Do not use SHA-256 while loading the V4 prefix files. r=gcp SHA256 is an expensive operation, we should avoid using them if possible. SafeBrowsing prefix files are loaded during startup and verify integrity with SHA256 which may affect the performance especially on the low-end device. This patch simply removes the SHA256 integrity check. CRC32 version integrity check will be introduced in the other patch. This patch also changes the behavior of recording "Telemetry::URLCLASSIFIER_VLPS_LOAD_CORRUPT" a little bit. It used to records only once per session(during startup, the first time we load prefix set), now it records per update. Differential Revision: https://phabricator.services.mozilla.com/D21461
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/LookupCacheV4.cpp
toolkit/components/url-classifier/LookupCacheV4.h
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -1574,16 +1574,18 @@ nsresult Classifier::LoadMetadata(nsIFil
       }
     }
     if (!lookupCacheV4) {
       continue;
     }
 
     nsCString state, sha256;
     rv = lookupCacheV4->LoadMetadata(state, sha256);
+    Telemetry::Accumulate(Telemetry::URLCLASSIFIER_VLPS_METADATA_CORRUPT,
+                          rv == NS_ERROR_FILE_CORRUPTED);
     if (NS_FAILED(rv)) {
       LOG(("Failed to get metadata for table %s", tableName.get()));
       continue;
     }
 
     // The state might include '\n' so that we have to encode.
     nsAutoCString stateBase64;
     rv = Base64Encode(state, stateBase64);
--- a/toolkit/components/url-classifier/LookupCacheV4.cpp
+++ b/toolkit/components/url-classifier/LookupCacheV4.cpp
@@ -163,34 +163,17 @@ nsresult LookupCacheV4::ClearPrefixes() 
   return mVLPrefixSet->SetPrefixes(map);
 }
 
 nsresult LookupCacheV4::StoreToFile(nsCOMPtr<nsIFile>& aFile) {
   return mVLPrefixSet->StoreToFile(aFile);
 }
 
 nsresult LookupCacheV4::LoadFromFile(nsCOMPtr<nsIFile>& aFile) {
-  nsresult rv = mVLPrefixSet->LoadFromFile(aFile);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  nsCString state, sha256;
-  rv = LoadMetadata(state, sha256);
-  Telemetry::Accumulate(Telemetry::URLCLASSIFIER_VLPS_METADATA_CORRUPT,
-                        rv == NS_ERROR_FILE_CORRUPTED);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  rv = VerifySHA256(sha256);
-  Telemetry::Accumulate(Telemetry::URLCLASSIFIER_VLPS_LOAD_CORRUPT,
-                        rv == NS_ERROR_FILE_CORRUPTED);
-  Unused << NS_WARN_IF(NS_FAILED(rv));
-  return rv;
+  return mVLPrefixSet->LoadFromFile(aFile);
 }
 
 size_t LookupCacheV4::SizeOfPrefixSet() const {
   return mVLPrefixSet->SizeOfIncludingThis(moz_malloc_size_of);
 }
 
 static nsresult AppendPrefixToMap(PrefixStringMap& prefixes,
                                   const nsACString& prefix) {
@@ -365,47 +348,16 @@ nsresult LookupCacheV4::ApplyUpdate(RefP
 
 nsresult LookupCacheV4::AddFullHashResponseToCache(
     const FullHashResponseMap& aResponseMap) {
   CopyClassHashTable<FullHashResponseMap>(aResponseMap, mFullHashCache);
 
   return NS_OK;
 }
 
-nsresult LookupCacheV4::VerifySHA256(const nsACString& aSHA256) {
-  nsCOMPtr<nsICryptoHash> crypto;
-  nsresult rv = InitCrypto(crypto);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  PrefixStringMap map;
-  mVLPrefixSet->GetPrefixes(map);
-
-  VLPrefixSet loadPSet(map);
-  uint32_t index = loadPSet.Count() + 1;
-  for (; index > 0; index--) {
-    nsAutoCString prefix;
-    if (!loadPSet.GetSmallestPrefix(prefix)) {
-      break;
-    }
-    UpdateSHA256(crypto, prefix);
-  }
-
-  nsAutoCString sha256;
-  crypto->Finish(false, sha256);
-
-  if (sha256 != aSHA256) {
-    LOG(("Sha256 hash mismatch when loading prefixes from file."));
-    return NS_ERROR_FILE_CORRUPTED;
-  }
-
-  return NS_OK;
-}
-
 //////////////////////////////////////////////////////////////////////////
 // A set of lightweight functions for reading/writing value from/to file.
 
 namespace {
 
 template <typename T>
 struct ValueTraits {
   static_assert(sizeof(T) <= LookupCacheV4::MAX_METADATA_VALUE_LENGTH,
--- a/toolkit/components/url-classifier/LookupCacheV4.h
+++ b/toolkit/components/url-classifier/LookupCacheV4.h
@@ -51,17 +51,15 @@ class LookupCacheV4 final : public Looku
   virtual nsresult LoadFromFile(nsCOMPtr<nsIFile>& aFile) override;
   virtual size_t SizeOfPrefixSet() const override;
 
  private:
   ~LookupCacheV4() {}
 
   virtual int Ver() const override { return VER; }
 
-  nsresult VerifySHA256(const nsACString& aSHA256);
-
   RefPtr<VariableLengthPrefixSet> mVLPrefixSet;
 };
 
 }  // namespace safebrowsing
 }  // namespace mozilla
 
 #endif