Bug 1353956 - P2. Do not use SHA-256 while loading the V4 prefix files. r=gcp
☠☠ backed out by 4f6e4cbf401f ☠ ☠
authordlee <dlee@mozilla.com>
Thu, 28 Feb 2019 08:18:46 +0000
changeset 520660 6b8412db5a05f12bf44b5abc1bf553ed356da41f
parent 520659 3d326cfcd002b1c9598151ae5d39bbc9051524f3
child 520661 bc0b91abce9bcb43ef501dfa7bca03ea678f9783
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