Issue 1028127: Cleanup gnome-keyring migration from bad schema

In August 2016 browser migrated from old schema to a new one, this patch
removes the migration code as most active installations probably already
migrated.

Bug: 1028127
Change-Id: I586fa49422daef12c7a27b4b32fddc40cc0651a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144186Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758822}

components/os_crypt/key_storage_libsecret.cc

@@ -19,14 +19,6 @@ const char kApplicationName[] = "chrome";
 const char kApplicationName[] = "chromium";
 #endif
 
-// Deprecated in M55 (crbug.com/639298)
-const SecretSchema kKeystoreSchemaV1 = {
-    "chrome_libsecret_os_crypt_password",
-    SECRET_SCHEMA_NONE,
-    {
-        {nullptr, SECRET_SCHEMA_ATTRIBUTE_STRING},
-    }};
-
 const SecretSchema kKeystoreSchemaV2 = {
     "chrome_libsecret_os_crypt_password_v2",
     SECRET_SCHEMA_DONT_MATCH_NAME,
@@ -106,9 +98,6 @@ std::string KeyStorageLibsecret::GetKeyImpl() {
 
   SecretValue* password_libsecret = ToSingleSecret(helper.results());
   if (!password_libsecret) {
-    std::string password = Migrate();
-    if (!password.empty())
-      return password;
     return AddRandomPasswordInLibsecret();
   }
   AnalyseKeyHistory(helper.results());
@@ -124,52 +113,3 @@ bool KeyStorageLibsecret::Init() {
     LibsecretLoader::EnsureKeyringUnlocked();
   return loaded;
 }
-
-std::string KeyStorageLibsecret::Migrate() {
-  LibsecretAttributesBuilder attrs;
-
-  // Detect old entry.
-  LibsecretLoader::SearchHelper helper;
-  helper.Search(&kKeystoreSchemaV1, attrs.Get(),
-                SECRET_SEARCH_UNLOCK | SECRET_SEARCH_LOAD_SECRETS);
-  if (!helper.success())
-    return std::string();
-
-  SecretValue* password_libsecret = ToSingleSecret(helper.results());
-  if (!password_libsecret)
-    return std::string();
-
-  VLOG(1) << "OSCrypt detected a deprecated password in Libsecret.";
-  std::string password(
-      LibsecretLoader::secret_value_get_text(password_libsecret));
-  LibsecretLoader::secret_value_unref(password_libsecret);
-
-  // Create new entry.
-  GError* error = nullptr;
-  bool success = LibsecretLoader::secret_password_store_sync(
-      &kKeystoreSchemaV2, nullptr, KeyStorageLinux::kKey, password.c_str(),
-      nullptr, &error, "application", kApplicationName, nullptr);
-  if (error) {
-    VLOG(1) << "Failed to store migrated password. " << error->message;
-    g_error_free(error);
-    return std::string();
-  }
-  if (!success) {
-    VLOG(1) << "Failed to store migrated password.";
-    return std::string();
-  }
-
-  // Delete old entry.
-  // Even if deletion failed, we have to use the password that we created.
-  success = LibsecretLoader::secret_password_clear_sync(
-      &kKeystoreSchemaV1, nullptr, &error, nullptr);
-  if (error) {
-    VLOG(1) << "OSCrypt failed to delete deprecated password. "
-            << error->message;
-    g_error_free(error);
-  }
-
-  VLOG(1) << "OSCrypt migrated from deprecated password.";
-
-  return password;
-}

components/os_crypt/key_storage_libsecret.h

@@ -25,12 +25,6 @@ class COMPONENT_EXPORT(OS_CRYPT) KeyStorageLibsecret : public KeyStorageLinux {
  private:
   std::string AddRandomPasswordInLibsecret();
 
-  // TODO(crbug.com/639298) Older Chromium releases stored passwords with a
-  // problematic schema. Detect password entries with the old schema and migrate
-  // them to the new schema. Returns the migrated password or an empty string if
-  // none we migrated.
-  std::string Migrate();
-
   DISALLOW_COPY_AND_ASSIGN(KeyStorageLibsecret);
 };
 

components/os_crypt/key_storage_libsecret_unittest.cc

@@ -14,13 +14,6 @@
 
 namespace {
 
-const SecretSchema kKeystoreSchemaV1 = {
-    "chrome_libsecret_os_crypt_password",
-    SECRET_SCHEMA_NONE,
-    {
-        {nullptr, SECRET_SCHEMA_ATTRIBUTE_STRING},
-    }};
-
 const SecretSchema kKeystoreSchemaV2 = {
     "chrome_libsecret_os_crypt_password_v2",
     SECRET_SCHEMA_DONT_MATCH_NAME,
@@ -41,8 +34,7 @@ class MockPasswordStore {
  public:
   void Reset() {
     mapping_.clear();
-    ClearV1Password();
-    ClearV2Password();
+    ClearPassword();
     for (GObject* o : objects_returned_to_caller_) {
       ASSERT_EQ(o->ref_count, 1u);
       g_object_unref(o);
@@ -50,31 +42,18 @@ class MockPasswordStore {
     objects_returned_to_caller_.clear();
   }
 
-  void ClearV1Password() {
-    if (v1_password_) {
-      ASSERT_EQ(v1_password_->ref_count, 1u);
-      g_object_unref(v1_password_);
-      v1_password_ = nullptr;
+  void ClearPassword() {
+    if (password_) {
+      ASSERT_EQ(password_->ref_count, 1u);
+      g_object_unref(password_);
+      password_ = nullptr;
     }
   }
-  void ClearV2Password() {
-    if (v2_password_) {
-      ASSERT_EQ(v2_password_->ref_count, 1u);
-      g_object_unref(v2_password_);
-      v2_password_ = nullptr;
-    }
-  }
-
-  void SetV1Password(const std::string& password) {
-    ASSERT_FALSE(v1_password_);
-    v1_password_ = static_cast<GObject*>(g_object_new(G_TYPE_OBJECT, nullptr));
-    mapping_[v1_password_] = password;
-  }
 
-  void SetV2Password(const std::string& password) {
-    ASSERT_FALSE(v2_password_);
-    v2_password_ = static_cast<GObject*>(g_object_new(G_TYPE_OBJECT, nullptr));
-    mapping_[v2_password_] = password;
+  void SetPassword(const std::string& password) {
+    ASSERT_FALSE(password_);
+    password_ = static_cast<GObject*>(g_object_new(G_TYPE_OBJECT, nullptr));
+    mapping_[password_] = password;
   }
 
   GObject* MakeTempObject(const std::string& value) {
@@ -92,13 +71,11 @@ class MockPasswordStore {
     return mapping_[static_cast<GObject*>(opaque_id)].c_str();
   }
 
-  GObject* v1_password() { return v1_password_; }
-  GObject* v2_password() { return v2_password_; }
+  GObject* password() { return password_; }
 
   std::unordered_map<GObject*, std::string> mapping_;
   std::vector<GObject*> objects_returned_to_caller_;
-  GObject* v1_password_ = nullptr;
-  GObject* v2_password_ = nullptr;
+  GObject* password_ = nullptr;
 };
 base::LazyInstance<MockPasswordStore>::Leaky g_password_store =
     LAZY_INSTANCE_INITIALIZER;
@@ -137,11 +114,6 @@ class MockLibsecretLoader : public LibsecretLoader {
                                                 GCancellable* cancellable,
                                                 GError** error);
 
-  static gboolean mock_secret_password_clear_sync(const SecretSchema* schema,
-                                                  GCancellable* cancellable,
-                                                  GError** error,
-                                                  ...);
-
   static SecretValue* mock_secret_item_get_secret(SecretItem* item);
 
   static guint64 mock_secret_item_get_created(SecretItem* item);
@@ -168,7 +140,7 @@ gboolean MockLibsecretLoader::mock_secret_password_store_sync(
     return true;
 
   EXPECT_STREQ(kKeystoreSchemaV2.name, schema->name);
-  g_password_store.Pointer()->SetV2Password(password);
+  g_password_store.Pointer()->SetPassword(password);
   return true;
 }
 
@@ -185,24 +157,16 @@ GList* MockLibsecretLoader::mock_secret_service_search_sync(
     SecretSearchFlags flags,
     GCancellable* cancellable,
     GError** error) {
-  bool is_known_schema = strcmp(schema->name, kKeystoreSchemaV2.name) == 0 ||
-                         strcmp(schema->name, kKeystoreSchemaV1.name) == 0;
-  EXPECT_TRUE(is_known_schema);
+  EXPECT_STREQ(kKeystoreSchemaV2.name, schema->name);
 
   EXPECT_TRUE(flags & SECRET_SEARCH_UNLOCK);
   EXPECT_TRUE(flags & SECRET_SEARCH_LOAD_SECRETS);
 
   GObject* item = nullptr;
   MockPasswordStore* store = g_password_store.Pointer();
-  if (strcmp(schema->name, kKeystoreSchemaV2.name) == 0) {
-    GObject* password = store->v2_password();
-    if (password)
-      item = store->MakeTempObject(store->GetString(password));
-  } else if (strcmp(schema->name, kKeystoreSchemaV1.name) == 0) {
-    GObject* password = store->v1_password();
-    if (password)
-      item = store->MakeTempObject(store->GetString(password));
-  }
+  GObject* password = store->password();
+  if (password)
+    item = store->MakeTempObject(store->GetString(password));
 
   if (!item) {
     return nullptr;
@@ -214,18 +178,6 @@ GList* MockLibsecretLoader::mock_secret_service_search_sync(
   return result;
 }
 
-// static
-gboolean MockLibsecretLoader::mock_secret_password_clear_sync(
-    const SecretSchema* schema,
-    GCancellable* cancellable,
-    GError** error,
-    ...) {
-  // We would only delete entries in the deprecated schema.
-  EXPECT_STREQ(kKeystoreSchemaV1.name, schema->name);
-  g_password_store.Pointer()->ClearV1Password();
-  return true;
-}
-
 // static
 SecretValue* MockLibsecretLoader::mock_secret_item_get_secret(
     SecretItem* item) {
@@ -254,9 +206,6 @@ bool MockLibsecretLoader::ResetForOSCrypt() {
   secret_service_search_sync =
       &MockLibsecretLoader::mock_secret_service_search_sync;
   secret_item_get_secret = &MockLibsecretLoader::mock_secret_item_get_secret;
-  // Used by Migrate()
-  secret_password_clear_sync =
-      &MockLibsecretLoader::mock_secret_password_clear_sync;
   secret_item_get_created = &MockLibsecretLoader::mock_secret_item_get_created;
   secret_item_get_modified =
       &MockLibsecretLoader::mock_secret_item_get_modified;
@@ -290,7 +239,7 @@ class LibsecretTest : public testing::Test {
 TEST_F(LibsecretTest, LibsecretRepeats) {
   KeyStorageLibsecret libsecret;
   MockLibsecretLoader::ResetForOSCrypt();
-  g_password_store.Pointer()->SetV2Password("initial password");
+  g_password_store.Pointer()->SetPassword("initial password");
   std::string password = libsecret.GetKey();
   EXPECT_FALSE(password.empty());
   std::string password_repeat = libsecret.GetKey();
@@ -306,13 +255,4 @@ TEST_F(LibsecretTest, LibsecretCreatesRandomised) {
   EXPECT_NE(password, password_new);
 }
 
-TEST_F(LibsecretTest, LibsecretMigratesFromSchemaV1ToV2) {
-  KeyStorageLibsecret libsecret;
-  MockLibsecretLoader::ResetForOSCrypt();
-  g_password_store.Pointer()->SetV1Password("swallow");
-  g_password_store.Pointer()->ClearV2Password();
-  std::string password = libsecret.GetKey();
-  EXPECT_EQ("swallow", password);
-}
-
 }  // namespace