Commit 02da78d1 authored by cfroussios's avatar cfroussios Committed by Commit bot

Migrate Libsecret for OSCrypt to a new schema

Schemas for gnome-keyring and libsecret are not 100% interchangeable. To
support gnome-keyring, we need to use equivalent schemas in both
libraries. This will allow a user to upgrade their machine from gnome-keyring
to libsecret, without chrome losing access to OSCrypt's encryption key.

We correct the unfortunate initial choice for a schema by copying entries
to the new schema.

As a bonus, this will correct the mislabeling of some entries (see crbug/640603)

BUG=639298

Review-Url: https://codereview.chromium.org/2273723002
Cr-Commit-Position: refs/heads/master@{#415254}
parent fe3a12df
...@@ -11,42 +11,61 @@ ...@@ -11,42 +11,61 @@
namespace { namespace {
const SecretSchema kKeystoreSchema = { #if defined(GOOGLE_CHROME_BUILD)
const char kApplicationName[] = "chrome";
#else
const char kApplicationName[] = "chromium";
#endif
// Deprecated in M55 (crbug.com/639298)
const SecretSchema kKeystoreSchemaV1 = {
"chrome_libsecret_os_crypt_password", "chrome_libsecret_os_crypt_password",
SECRET_SCHEMA_NONE, SECRET_SCHEMA_NONE,
{ {
{nullptr, SECRET_SCHEMA_ATTRIBUTE_STRING}, {nullptr, SECRET_SCHEMA_ATTRIBUTE_STRING},
}}; }};
const SecretSchema kKeystoreSchemaV2 = {
"chrome_libsecret_os_crypt_password_v2",
SECRET_SCHEMA_DONT_MATCH_NAME,
{
{"application", SECRET_SCHEMA_ATTRIBUTE_STRING},
{nullptr, SECRET_SCHEMA_ATTRIBUTE_STRING},
}};
} // namespace } // namespace
std::string KeyStorageLibsecret::AddRandomPasswordInLibsecret() { std::string KeyStorageLibsecret::AddRandomPasswordInLibsecret() {
std::string password; std::string password;
base::Base64Encode(base::RandBytesAsString(16), &password); base::Base64Encode(base::RandBytesAsString(16), &password);
GError* error = nullptr; GError* error = nullptr;
LibsecretLoader::secret_password_store_sync( bool success = LibsecretLoader::secret_password_store_sync(
&kKeystoreSchema, nullptr, KeyStorageLinux::kKey, password.c_str(), &kKeystoreSchemaV2, nullptr, KeyStorageLinux::kKey, password.c_str(),
nullptr, &error, nullptr); nullptr, &error, "application", kApplicationName, nullptr);
if (error || !success) {
if (error) {
VLOG(1) << "Libsecret lookup failed: " << error->message; VLOG(1) << "Libsecret lookup failed: " << error->message;
return std::string(); return std::string();
} }
VLOG(1) << "OSCrypt generated a new password.";
return password; return password;
} }
std::string KeyStorageLibsecret::GetKey() { std::string KeyStorageLibsecret::GetKey() {
GError* error = nullptr; GError* error = nullptr;
LibsecretAttributesBuilder attrs; LibsecretAttributesBuilder attrs;
attrs.Append("application", kApplicationName);
SecretValue* password_libsecret = LibsecretLoader::secret_service_lookup_sync( SecretValue* password_libsecret = LibsecretLoader::secret_service_lookup_sync(
nullptr, &kKeystoreSchema, attrs.Get(), nullptr, &error); nullptr, &kKeystoreSchemaV2, attrs.Get(), nullptr, &error);
if (error) { if (error) {
VLOG(1) << "Libsecret lookup failed: " << error->message; VLOG(1) << "Libsecret lookup failed: " << error->message;
g_error_free(error); g_error_free(error);
return std::string(); return std::string();
} }
if (!password_libsecret) { if (!password_libsecret) {
std::string password = Migrate();
if (!password.empty())
return password;
return AddRandomPasswordInLibsecret(); return AddRandomPasswordInLibsecret();
} }
std::string password( std::string password(
...@@ -58,3 +77,34 @@ std::string KeyStorageLibsecret::GetKey() { ...@@ -58,3 +77,34 @@ std::string KeyStorageLibsecret::GetKey() {
bool KeyStorageLibsecret::Init() { bool KeyStorageLibsecret::Init() {
return LibsecretLoader::EnsureLibsecretLoaded(); return LibsecretLoader::EnsureLibsecretLoaded();
} }
std::string KeyStorageLibsecret::Migrate() {
GError* error = nullptr;
LibsecretAttributesBuilder attrs;
// Detect old entry.
SecretValue* password_libsecret = LibsecretLoader::secret_service_lookup_sync(
nullptr, &kKeystoreSchemaV1, attrs.Get(), nullptr, &error);
if (error || !password_libsecret)
return std::string();
VLOG(1) << "OSCrypt detected a deprecated password in Libsecret.";
std::string password(
LibsecretLoader::secret_value_get_text(password_libsecret));
// Create new entry.
bool success = LibsecretLoader::secret_password_store_sync(
&kKeystoreSchemaV2, nullptr, KeyStorageLinux::kKey, password.c_str(),
nullptr, &error, "application", kApplicationName, nullptr);
if (error || !success)
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);
VLOG(1) << "OSCrypt migrated from deprecated password.";
return password;
}
...@@ -26,6 +26,12 @@ class KeyStorageLibsecret : public KeyStorageLinux { ...@@ -26,6 +26,12 @@ class KeyStorageLibsecret : public KeyStorageLinux {
private: private:
std::string AddRandomPasswordInLibsecret(); 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); DISALLOW_COPY_AND_ASSIGN(KeyStorageLibsecret);
}; };
......
...@@ -16,6 +16,21 @@ namespace { ...@@ -16,6 +16,21 @@ namespace {
// because we don't use anything else from it. // because we don't use anything else from it.
using MockSecretValue = std::string; using MockSecretValue = std::string;
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,
{
{"application", SECRET_SCHEMA_ATTRIBUTE_STRING},
{nullptr, SECRET_SCHEMA_ATTRIBUTE_STRING},
}};
// Replaces some of LibsecretLoader's methods with mocked ones. // Replaces some of LibsecretLoader's methods with mocked ones.
class MockLibsecretLoader : public LibsecretLoader { class MockLibsecretLoader : public LibsecretLoader {
public: public:
...@@ -29,6 +44,10 @@ class MockLibsecretLoader : public LibsecretLoader { ...@@ -29,6 +44,10 @@ class MockLibsecretLoader : public LibsecretLoader {
// Releases memory and restores LibsecretLoader to an uninitialized state. // Releases memory and restores LibsecretLoader to an uninitialized state.
static void TearDown(); static void TearDown();
// Set whether there is an old password that needs to be migrated from the
// deprecated schema. Null means no such password. See crbug.com/639298
static void SetDeprecatedOSCryptPassword(const char* value);
private: private:
// These methods are used to redirect calls through LibsecretLoader // These methods are used to redirect calls through LibsecretLoader
static const gchar* mock_secret_value_get_text(MockSecretValue* value); static const gchar* mock_secret_value_get_text(MockSecretValue* value);
...@@ -57,11 +76,18 @@ class MockLibsecretLoader : public LibsecretLoader { ...@@ -57,11 +76,18 @@ class MockLibsecretLoader : public LibsecretLoader {
GCancellable* cancellable, GCancellable* cancellable,
GError** error); GError** error);
// MockLibsecretLoader owns this object. static gboolean mock_secret_password_clear_sync(const SecretSchema* schema,
GCancellable* cancellable,
GError** error,
...);
// MockLibsecretLoader owns these objects.
static MockSecretValue* stored_password_mock_ptr_; static MockSecretValue* stored_password_mock_ptr_;
static MockSecretValue* deprecated_password_mock_ptr_;
}; };
MockSecretValue* MockLibsecretLoader::stored_password_mock_ptr_ = nullptr; MockSecretValue* MockLibsecretLoader::stored_password_mock_ptr_ = nullptr;
MockSecretValue* MockLibsecretLoader::deprecated_password_mock_ptr_ = nullptr;
const gchar* MockLibsecretLoader::mock_secret_value_get_text( const gchar* MockLibsecretLoader::mock_secret_value_get_text(
MockSecretValue* value) { MockSecretValue* value) {
...@@ -77,6 +103,7 @@ gboolean MockLibsecretLoader::mock_secret_password_store_sync( ...@@ -77,6 +103,7 @@ gboolean MockLibsecretLoader::mock_secret_password_store_sync(
GCancellable* cancellable, GCancellable* cancellable,
GError** error, GError** error,
...) { ...) {
EXPECT_STREQ(kKeystoreSchemaV2.name, schema->name);
delete stored_password_mock_ptr_; delete stored_password_mock_ptr_;
stored_password_mock_ptr_ = new MockSecretValue(password); stored_password_mock_ptr_ = new MockSecretValue(password);
return true; return true;
...@@ -89,7 +116,17 @@ MockSecretValue* MockLibsecretLoader::mock_secret_service_lookup_sync( ...@@ -89,7 +116,17 @@ MockSecretValue* MockLibsecretLoader::mock_secret_service_lookup_sync(
GHashTable* attributes, GHashTable* attributes,
GCancellable* cancellable, GCancellable* cancellable,
GError** error) { GError** error) {
return stored_password_mock_ptr_; bool is_known_schema = strcmp(schema->name, kKeystoreSchemaV2.name) == 0 ||
strcmp(schema->name, kKeystoreSchemaV1.name) == 0;
EXPECT_TRUE(is_known_schema);
if (strcmp(schema->name, kKeystoreSchemaV2.name) == 0)
return stored_password_mock_ptr_;
else if (strcmp(schema->name, kKeystoreSchemaV1.name) == 0)
return deprecated_password_mock_ptr_;
NOTREACHED();
return nullptr;
} }
// static // static
...@@ -107,9 +144,21 @@ GList* MockLibsecretLoader::mock_secret_service_search_sync( ...@@ -107,9 +144,21 @@ GList* MockLibsecretLoader::mock_secret_service_search_sync(
return nullptr; return nullptr;
} }
// static
gboolean MockLibsecretLoader::mock_secret_password_clear_sync(
const SecretSchema* schema,
GCancellable* cancellable,
GError** error,
...) {
EXPECT_STREQ(kKeystoreSchemaV1.name, schema->name);
delete deprecated_password_mock_ptr_;
deprecated_password_mock_ptr_ = nullptr;
return true;
}
// static // static
bool MockLibsecretLoader::ResetForOSCrypt() { bool MockLibsecretLoader::ResetForOSCrypt() {
// 4 methods used by KeyStorageLibsecret::GetKey() // 4 methods used by KeyStorageLibsecret
secret_password_store_sync = secret_password_store_sync =
&MockLibsecretLoader::mock_secret_password_store_sync; &MockLibsecretLoader::mock_secret_password_store_sync;
secret_value_get_text = (decltype(&::secret_value_get_text)) & secret_value_get_text = (decltype(&::secret_value_get_text)) &
...@@ -118,6 +167,9 @@ bool MockLibsecretLoader::ResetForOSCrypt() { ...@@ -118,6 +167,9 @@ bool MockLibsecretLoader::ResetForOSCrypt() {
secret_service_lookup_sync = secret_service_lookup_sync =
(decltype(&::secret_service_lookup_sync)) & (decltype(&::secret_service_lookup_sync)) &
MockLibsecretLoader::mock_secret_service_lookup_sync; MockLibsecretLoader::mock_secret_service_lookup_sync;
// Used by Migrate()
secret_password_clear_sync =
&MockLibsecretLoader::mock_secret_password_clear_sync;
// 1 method used by LibsecretLoader::EnsureLibsecretLoaded() // 1 method used by LibsecretLoader::EnsureLibsecretLoaded()
secret_service_search_sync = secret_service_search_sync =
&MockLibsecretLoader::mock_secret_service_search_sync; &MockLibsecretLoader::mock_secret_service_search_sync;
...@@ -135,6 +187,12 @@ void MockLibsecretLoader::SetOSCryptPassword(const char* value) { ...@@ -135,6 +187,12 @@ void MockLibsecretLoader::SetOSCryptPassword(const char* value) {
stored_password_mock_ptr_ = new MockSecretValue(value); stored_password_mock_ptr_ = new MockSecretValue(value);
} }
// static
void MockLibsecretLoader::SetDeprecatedOSCryptPassword(const char* value) {
delete deprecated_password_mock_ptr_;
deprecated_password_mock_ptr_ = new MockSecretValue(value);
}
// static // static
void MockLibsecretLoader::TearDown() { void MockLibsecretLoader::TearDown() {
delete stored_password_mock_ptr_; delete stored_password_mock_ptr_;
...@@ -174,4 +232,12 @@ TEST_F(LibsecretTest, LibsecretCreatesRandomised) { ...@@ -174,4 +232,12 @@ TEST_F(LibsecretTest, LibsecretCreatesRandomised) {
EXPECT_NE(password, password_new); EXPECT_NE(password, password_new);
} }
TEST_F(LibsecretTest, LibsecretMigratesFromSchemaV1ToV2) {
KeyStorageLibsecret libsecret;
MockLibsecretLoader::ResetForOSCrypt();
MockLibsecretLoader::SetDeprecatedOSCryptPassword("swallow");
std::string password = libsecret.GetKey();
EXPECT_EQ("swallow", password);
}
} // namespace } // namespace
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment