Reland "[sync::test] Add two client integration test for custom passphrase"

This is a reland of 48c7a842

Original change's description:
> [sync::test] Add two client integration test for custom passphrase
> 
> Add a Sync integration test which exercises custom passphrase flows in
> an end-to-end manner with two clients. The test ensures that two clients
> can sync data properly in the presence of custom passphrase-based
> encryption and different key derivation methods.
> 
> Refactor SetScryptFeatureState() from the single-client test to a scoped
> feature toggler that resides in encryption_helper, because the
> functionality now needs to also be used from the two-client test.
> 
> Bug: 894148
> Change-Id: Ia211b221f89a99092d3b19690ecae6bb4fa77d02
> Reviewed-on: https://chromium-review.googlesource.com/c/1277648
> Commit-Queue: David Davidović <davidovic@google.com>
> Reviewed-by: vitaliii <vitaliii@chromium.org>
> Reviewed-by: Marc Treib <treib@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599929}

Bug: 894148
Change-Id: I425db59e1427f2c3ebf1cb9cd8b2ee440fd31ce1
Reviewed-on: https://chromium-review.googlesource.com/c/1283012Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: David Davidović <davidovic@google.com>
Cr-Commit-Position: refs/heads/master@{#599945}

chrome/browser/sync/test/integration/encryption_helper.cc

@@ -9,7 +9,9 @@
 #include "chrome/browser/sync/test/integration/encryption_helper.h"
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/sync/base/passphrase_enums.h"
+#include "components/sync/base/sync_base_switches.h"
 #include "components/sync/base/system_encryptor.h"
+#include "components/sync/engine/sync_engine_switches.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace encryption_helper {
@@ -174,3 +176,24 @@ std::string PassphraseRequiredStateChecker::GetDebugMessage() const {
   return "Waiting until decryption passphrase is " +
          std::string(desired_state_ ? "required" : "not required");
 }
+
+ScopedScryptFeatureToggler::ScopedScryptFeatureToggler(
+    bool force_disabled,
+    bool use_for_new_passphrases) {
+  std::vector<base::Feature> enabled_features;
+  std::vector<base::Feature> disabled_features;
+  if (force_disabled) {
+    enabled_features.push_back(
+        switches::kSyncForceDisableScryptForCustomPassphrase);
+  } else {
+    disabled_features.push_back(
+        switches::kSyncForceDisableScryptForCustomPassphrase);
+  }
+  if (use_for_new_passphrases) {
+    enabled_features.push_back(switches::kSyncUseScryptForNewCustomPassphrases);
+  } else {
+    disabled_features.push_back(
+        switches::kSyncUseScryptForNewCustomPassphrases);
+  }
+  feature_list_.InitWithFeatures(enabled_features, disabled_features);
+}

chrome/browser/sync/test/integration/encryption_helper.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
 #include "components/sync/base/cryptographer.h"
 #include "components/sync/protocol/nigori_specifics.pb.h"
@@ -76,4 +77,12 @@ class PassphraseRequiredStateChecker : public SingleClientStatusChangeChecker {
   bool desired_state_;
 };
 
+class ScopedScryptFeatureToggler {
+ public:
+  ScopedScryptFeatureToggler(bool force_disabled, bool use_for_new_passphrases);
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
 #endif  // CHROME_BROWSER_SYNC_TEST_INTEGRATION_ENCRYPTION_HELPER_H_

chrome/browser/sync/test/integration/single_client_custom_passphrase_sync_test.cc

@@ -8,9 +8,7 @@
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/sync/base/cryptographer.h"
 #include "components/sync/base/passphrase_enums.h"
-#include "components/sync/base/sync_base_switches.h"
 #include "components/sync/base/system_encryptor.h"
-#include "components/sync/engine/sync_engine_switches.h"
 
 namespace {
 
@@ -162,27 +160,6 @@ class SingleClientCustomPassphraseSyncTest : public SyncTest {
     return cryptographer;
   }
 
-  void SetScryptFeaturesState(bool force_disabled,
-                              bool use_for_new_passphrases) {
-    std::vector<base::Feature> enabled_features;
-    std::vector<base::Feature> disabled_features;
-    if (force_disabled) {
-      enabled_features.push_back(
-          switches::kSyncForceDisableScryptForCustomPassphrase);
-    } else {
-      disabled_features.push_back(
-          switches::kSyncForceDisableScryptForCustomPassphrase);
-    }
-    if (use_for_new_passphrases) {
-      enabled_features.push_back(
-          switches::kSyncUseScryptForNewCustomPassphrases);
-    } else {
-      disabled_features.push_back(
-          switches::kSyncUseScryptForNewCustomPassphrases);
-    }
-    feature_list_.InitWithFeatures(enabled_features, disabled_features);
-  }
-
   void InjectEncryptedServerBookmark(const std::string& title,
                                      const GURL& url,
                                      const KeyParams& key_params) {
@@ -195,7 +172,6 @@ class SingleClientCustomPassphraseSyncTest : public SyncTest {
 
  private:
   SystemEncryptor system_encryptor_;
-  base::test::ScopedFeatureList feature_list_;
 
   DISALLOW_COPY_AND_ASSIGN(SingleClientCustomPassphraseSyncTest);
 };
@@ -219,8 +195,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        CommitsEncryptedDataUsingPbkdf2WhenScryptDisabled) {
-  SetScryptFeaturesState(/*force_disabled=*/false,
-                         /*use_for_new_passphrases=*/false);
+  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
+                                     /*use_for_new_passphrases=*/false);
   SetEncryptionPassphraseForClient(/*index=*/0, "hunter2");
   ASSERT_TRUE(SetupSync());
   ASSERT_TRUE(AddURL(/*profile=*/0, "PBKDF2 encrypted",
@@ -238,8 +214,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        CommitsEncryptedDataUsingScryptWhenScryptEnabled) {
-  SetScryptFeaturesState(/*force_disabled=*/false,
-                         /*use_for_new_passphrases=*/true);
+  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
+                                     /*use_for_new_passphrases=*/true);
   SetEncryptionPassphraseForClient(/*index=*/0, "hunter2");
   ASSERT_TRUE(SetupSync());
 
@@ -274,8 +250,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        CanDecryptScryptKeyEncryptedDataWhenScryptNotDisabled) {
-  SetScryptFeaturesState(/*force_disabled=*/false,
-                         /*used_for_new_passphrases_=*/false);
+  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
+                                     /*use_for_new_passphrases_=*/false);
   KeyParams key_params = {
       KeyDerivationParams::CreateForScrypt("someConstantSalt"), "hunter2"};
   InjectEncryptedServerBookmark("scypt-encrypted bookmark",
@@ -301,8 +277,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                                 key_params);
   // Can only set feature state now because creating a Nigori and injecting an
   // encrypted bookmark both require key derivation using scrypt.
-  SetScryptFeaturesState(/*force_disabled=*/true,
-                         /*used_for_new_passphrases_=*/false);
+  ScopedScryptFeatureToggler toggler(/*force_disabled=*/true,
+                                     /*use_for_new_passphrases_=*/false);
   SetNigoriInFakeServer(GetFakeServer(), nigori);
   SetDecryptionPassphraseForClient(/*index=*/0, "hunter2");
 
@@ -313,8 +289,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        DoesNotLeakUnencryptedData) {
-  SetScryptFeaturesState(/*force_disabled=*/false,
-                         /*use_for_new_passphrases=*/false);
+  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
+                                     /*use_for_new_passphrases=*/false);
   SetEncryptionPassphraseForClient(/*index=*/0, "hunter2");
   DatatypeCommitCountingFakeServerObserver observer(GetFakeServer());
   ASSERT_TRUE(SetupSync());
@@ -338,8 +314,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
 
 IN_PROC_BROWSER_TEST_F(SingleClientCustomPassphraseSyncTest,
                        ReencryptsDataWhenPassphraseIsSet) {
-  SetScryptFeaturesState(/*force_disabled=*/false,
-                         /*use_for_new_passphrases=*/false);
+  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
+                                     /*use_for_new_passphrases=*/false);
   ASSERT_TRUE(SetupSync());
   ASSERT_TRUE(WaitForNigori(PassphraseType::KEYSTORE_PASSPHRASE));
   ASSERT_TRUE(AddURL(/*profile=*/0, "Re-encryption is great",

chrome/browser/sync/test/integration/two_client_custom_passphrase_sync_test.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sync/test/integration/bookmarks_helper.h"
+#include "chrome/browser/sync/test/integration/encryption_helper.h"
+#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
+#include "chrome/browser/sync/test/integration/sync_test.h"
+#include "components/sync/base/sync_base_switches.h"
+#include "components/sync/engine/sync_engine_switches.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+static const int kEncryptingClientId = 0;
+static const int kDecryptingClientId = 1;
+
+using bookmarks_helper::AddURL;
+using bookmarks_helper::AllModelsMatchVerifier;
+
+// These tests consider the client as a black-box; they are not concerned with
+// whether the data is committed to the server correctly encrypted. Rather, they
+// test the end-to-end behavior of two clients when a custom passphrase is set,
+// i.e. whether the second client can see data that was committed by the first
+// client. To test proper encryption behavior, a separate single-client test is
+// used.
+class TwoClientCustomPassphraseSyncTest : public SyncTest {
+ public:
+  TwoClientCustomPassphraseSyncTest() : SyncTest(TWO_CLIENT) {}
+  ~TwoClientCustomPassphraseSyncTest() override {}
+
+  bool WaitForBookmarksToMatchVerifier() {
+    return BookmarksMatchVerifierChecker().Wait();
+  }
+
+  bool WaitForPassphraseRequiredState(int index, bool desired_state) {
+    return PassphraseRequiredStateChecker(GetSyncService(index), desired_state)
+        .Wait();
+  }
+
+  void AddTestBookmarksToClient(int index) {
+    ASSERT_TRUE(AddURL(index, 0, "What are you syncing about?",
+                       GURL("https://google.com/synced-bookmark-1")));
+    ASSERT_TRUE(AddURL(index, 1, "Test bookmark",
+                       GURL("https://google.com/synced-bookmark-2")));
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TwoClientCustomPassphraseSyncTest);
+};
+
+IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest,
+                       DecryptionFailsWhenIncorrectPassphraseProvided) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  GetSyncService(kEncryptingClientId)->SetEncryptionPassphrase("hunter2");
+  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
+                                             /*desired_state=*/true));
+  EXPECT_FALSE(GetSyncService(kDecryptingClientId)
+                   ->SetDecryptionPassphrase("incorrect passphrase"));
+  EXPECT_TRUE(
+      GetSyncService(kDecryptingClientId)->IsPassphraseRequiredForDecryption());
+}
+
+IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest, ClientsCanSyncData) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  GetSyncService(kEncryptingClientId)->SetEncryptionPassphrase("hunter2");
+  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
+                                             /*desired_state=*/true));
+  EXPECT_TRUE(
+      GetSyncService(kDecryptingClientId)->SetDecryptionPassphrase("hunter2"));
+  EXPECT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
+                                             /*desired_state=*/false));
+  AddTestBookmarksToClient(kEncryptingClientId);
+
+  ASSERT_TRUE(
+      GetClient(kEncryptingClientId)
+          ->AwaitMutualSyncCycleCompletion(GetClient(kDecryptingClientId)));
+  EXPECT_TRUE(WaitForBookmarksToMatchVerifier());
+}
+
+IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest,
+                       ClientsCanSyncDataWhenScryptEncryptionNotEnabled) {
+  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
+                                     /*use_for_new_passphrases=*/false);
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  GetSyncService(kEncryptingClientId)->SetEncryptionPassphrase("hunter2");
+  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
+                                             /*desired_state=*/true));
+  EXPECT_TRUE(
+      GetSyncService(kDecryptingClientId)->SetDecryptionPassphrase("hunter2"));
+  EXPECT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
+                                             /*desired_state=*/false));
+  AddTestBookmarksToClient(kEncryptingClientId);
+
+  ASSERT_TRUE(
+      GetClient(kEncryptingClientId)
+          ->AwaitMutualSyncCycleCompletion(GetClient(kDecryptingClientId)));
+  EXPECT_TRUE(WaitForBookmarksToMatchVerifier());
+}
+
+IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest,
+                       ClientsCanSyncDataWhenScryptEncryptionEnabledInBoth) {
+  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
+                                     /*use_for_new_passphrases=*/true);
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  GetSyncService(kEncryptingClientId)->SetEncryptionPassphrase("hunter2");
+  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
+                                             /*desired_state=*/true));
+  EXPECT_TRUE(
+      GetSyncService(kDecryptingClientId)->SetDecryptionPassphrase("hunter2"));
+  EXPECT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
+                                             /*desired_state=*/false));
+  AddTestBookmarksToClient(kEncryptingClientId);
+
+  ASSERT_TRUE(
+      GetClient(kEncryptingClientId)
+          ->AwaitMutualSyncCycleCompletion(GetClient(kDecryptingClientId)));
+  EXPECT_TRUE(WaitForBookmarksToMatchVerifier());
+}
+
+IN_PROC_BROWSER_TEST_F(TwoClientCustomPassphraseSyncTest,
+                       ClientsCanSyncDataWhenScryptEncryptionEnabledInOne) {
+  ScopedScryptFeatureToggler toggler(/*force_disabled=*/false,
+                                     /*use_for_new_passphrases=*/false);
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AllModelsMatchVerifier());
+
+  {
+    ScopedScryptFeatureToggler temporary_toggler(
+        /*force_disabled=*/false, /*use_for_new_passphrases=*/true);
+    GetSyncService(kEncryptingClientId)->SetEncryptionPassphrase("hunter2");
+  }
+  ASSERT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
+                                             /*desired_state=*/true));
+  EXPECT_TRUE(
+      GetSyncService(kDecryptingClientId)->SetDecryptionPassphrase("hunter2"));
+  EXPECT_TRUE(WaitForPassphraseRequiredState(kDecryptingClientId,
+                                             /*desired_state=*/false));
+  AddTestBookmarksToClient(kEncryptingClientId);
+
+  ASSERT_TRUE(
+      GetClient(kEncryptingClientId)
+          ->AwaitMutualSyncCycleCompletion(GetClient(kDecryptingClientId)));
+  EXPECT_TRUE(WaitForBookmarksToMatchVerifier());
+}
+
+}  // namespace

chrome/test/BUILD.gn

@@ -5362,6 +5362,7 @@ if (!is_android && !is_fuchsia) {
       "../browser/sync/test/integration/two_client_autocomplete_sync_test.cc",
       "../browser/sync/test/integration/two_client_autofill_sync_test.cc",
       "../browser/sync/test/integration/two_client_bookmarks_sync_test.cc",
+      "../browser/sync/test/integration/two_client_custom_passphrase_sync_test.cc",
       "../browser/sync/test/integration/two_client_dictionary_sync_test.cc",
       "../browser/sync/test/integration/two_client_extension_settings_and_app_settings_sync_test.cc",
       "../browser/sync/test/integration/two_client_extensions_sync_test.cc",