Skip to content

Commit

Permalink
Forward password-store switch to OSCrypt component
Browse files Browse the repository at this point in the history
Password manager uses a switch to allow the user to override the
auto-detection of the appropriate password store. OSCrypt should
respect this switch as well.

The switch value is read and passed to OSCrypt at a very early
point in Chrome's start, before any of OSCrypt's dependents
use it.

I also reworked OSCrypt's build to make it simpler for chrome to
deduce whether the linux implementation of OSCrypt will be used.
 - Previously, os_crypt_linux was used only if we also decided to
   link at least one linux backend. Otherwise we used os_crypt_posix.
 + Now, we always use the linux implementation for linux. If no
   KeyStorage is linked, the linux implementation defaults to the
   same behavior as for posix.

BUG=602624

Review-Url: https://codereview.chromium.org/2118443002
Cr-Commit-Position: refs/heads/master@{#405114}
  • Loading branch information
Froussios authored and Commit bot committed Jul 13, 2016
1 parent f78ee98 commit 92699e3
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 15 deletions.
12 changes: 12 additions & 0 deletions chrome/browser/chrome_browser_main_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#include <fontconfig/fontconfig.h>

#include <string>

#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
#include "chrome/grit/chromium_strings.h"
Expand All @@ -17,7 +19,10 @@
#include "ui/base/l10n/l10n_util.h"

#if !defined(OS_CHROMEOS)
#include "base/command_line.h"
#include "base/linux_util.h"
#include "chrome/common/chrome_switches.h"
#include "components/os_crypt/os_crypt.h"
#include "content/public/browser/browser_thread.h"
#endif

Expand Down Expand Up @@ -59,6 +64,13 @@ void ChromeBrowserMainPartsLinux::PostProfileInit() {

g_browser_process->metrics_service()->RecordBreakpadRegistration(
breakpad::IsCrashReporterEnabled());

#if !defined(OS_CHROMEOS)
// Forward to os_crypt the flag to use a specific password store.
std::string password_store =
parsed_command_line().GetSwitchValueASCII(switches::kPasswordStore);
OSCrypt::SetStore(password_store);
#endif
}

void ChromeBrowserMainPartsLinux::PostMainMessageLoopStart() {
Expand Down
1 change: 1 addition & 0 deletions components/components_tests.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -1596,6 +1596,7 @@
'os_crypt/os_crypt_util_linux_unittest.cc',
],
'defines': [
'USE_KWALLET',
'USE_LIBSECRET',
],
'include_dirs': [
Expand Down
4 changes: 2 additions & 2 deletions components/os_crypt.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
},
},
}],
['OS=="linux" and chromeos!=1 and (use_glib==1 or use_dbus==1)', {
['OS=="linux" and chromeos!=1', {
'sources': [
'os_crypt/key_storage_linux.cc',
'os_crypt/key_storage_linux.h',
Expand Down Expand Up @@ -117,7 +117,7 @@
'../testing/gtest.gyp:gtest',
],
'conditions': [
['OS=="linux" and chromeos!=1 and (use_glib==1 or use_dbus)', {
['OS=="linux" and chromeos!=1', {
'sources': [
'os_crypt/os_crypt_mocker_linux.cc',
'os_crypt/os_crypt_mocker_linux.h',
Expand Down
6 changes: 3 additions & 3 deletions components/os_crypt/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ source_set("os_crypt") {
libs = [ "crypt32.lib" ]
}

if (is_desktop_linux && (use_glib || use_dbus)) {
if (is_desktop_linux) {
sources -= [ "os_crypt_posix.cc" ]
sources += [
"key_storage_linux.cc",
Expand Down Expand Up @@ -86,7 +86,7 @@ source_set("test_support") {
"//base",
"//testing/gtest",
]
if (is_desktop_linux && (use_glib || use_dbus)) {
if (is_desktop_linux) {
sources += [
"os_crypt_mocker_linux.cc",
"os_crypt_mocker_linux.h",
Expand All @@ -112,7 +112,7 @@ source_set("unit_tests") {
"//testing/gtest",
]

if (is_desktop_linux && (use_glib || use_dbus)) {
if (is_desktop_linux) {
sources += [ "os_crypt_linux_unittest.cc" ]
defines = []

Expand Down
48 changes: 45 additions & 3 deletions components/os_crypt/key_storage_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,56 @@

#include "components/os_crypt/key_storage_linux.h"

#include <string.h>

#include "base/environment.h"
#include "base/lazy_instance.h"
#include "base/logging.h"
#include "base/nix/xdg_util.h"

#if defined(USE_LIBSECRET)
#include "components/os_crypt/key_storage_libsecret.h"
#endif

base::LazyInstance<std::string> g_store_ = LAZY_INSTANCE_INITIALIZER;

// static
void KeyStorageLinux::SetStore(const std::string& store_type) {
g_store_.Get() = store_type;
VLOG(1) << "OSCrypt store set to " << store_type;
}

// static
std::unique_ptr<KeyStorageLinux> KeyStorageLinux::CreateService() {
std::unique_ptr<KeyStorageLinux> key_storage(new KeyStorageLibsecret());
base::nix::DesktopEnvironment used_desktop_env;
if (g_store_.Get() == "kwallet") {
used_desktop_env = base::nix::DESKTOP_ENVIRONMENT_KDE4;
} else if (g_store_.Get() == "kwallet5") {
used_desktop_env = base::nix::DESKTOP_ENVIRONMENT_KDE5;
} else if (g_store_.Get() == "gnome") {
used_desktop_env = base::nix::DESKTOP_ENVIRONMENT_GNOME;
} else if (g_store_.Get() == "basic") {
used_desktop_env = base::nix::DESKTOP_ENVIRONMENT_OTHER;
} else {
std::unique_ptr<base::Environment> env(base::Environment::Create());
used_desktop_env = base::nix::GetDesktopEnvironment(env.get());
}

if (key_storage->Init())
return key_storage;
// Try initializing the appropriate store for our environment.
std::unique_ptr<KeyStorageLinux> key_storage;
if (used_desktop_env == base::nix::DESKTOP_ENVIRONMENT_GNOME ||
used_desktop_env == base::nix::DESKTOP_ENVIRONMENT_UNITY ||
used_desktop_env == base::nix::DESKTOP_ENVIRONMENT_XFCE) {
#if defined(USE_LIBSECRET)
key_storage.reset(new KeyStorageLibsecret());
if (key_storage->Init()) {
VLOG(1) << "OSCrypt using Libsecret as backend.";
return key_storage;
}
#endif
}

// The appropriate store was not available.
VLOG(1) << "OSCrypt could not initialize a backend.";
return nullptr;
}
6 changes: 4 additions & 2 deletions components/os_crypt/key_storage_linux.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ class KeyStorageLinux {
KeyStorageLinux() = default;
virtual ~KeyStorageLinux() = default;

// Tries to load all known key storages. Returns the first that succeeds or
// null if none succeed.
// Force OSCrypt to use a specific linux password store.
static void SetStore(const std::string& store_type);

// Tries to load the appropriate key storage. Returns null if none succeed.
static std::unique_ptr<KeyStorageLinux> CreateService();

// Gets the encryption key from the OS password-managing library. If a key is
Expand Down
16 changes: 11 additions & 5 deletions components/os_crypt/os_crypt.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,23 @@
#include "base/strings/string16.h"
#include "build/build_config.h"

#if defined(USE_LIBSECRET)
#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
#include "components/os_crypt/key_storage_linux.h"
#endif
#endif // defined(OS_LINUX) && !defined(OS_CHROMEOS)

// The OSCrypt class gives access to simple encryption and decryption of
// strings. Note that on Mac, access to the system Keychain is required and
// these calls can block the current thread to collect user input. The same is
// true for Linux, if a password management tool is available.
class OSCrypt {
public:
#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
// If |store_type| is a known password store, we will attempt to use it.
// In any other case, we default to auto-detecting the store.
// This should not be changed after OSCrypt has been used.
static void SetStore(const std::string& store_type);
#endif // defined(OS_LINUX) && !defined(OS_CHROMEOS)

// Encrypt a string16. The output (second argument) is really an array of
// bytes, but we're passing it back as a std::string.
static bool EncryptString16(const base::string16& plaintext,
Expand Down Expand Up @@ -52,15 +59,14 @@ class OSCrypt {
DISALLOW_IMPLICIT_CONSTRUCTORS(OSCrypt);
};

#if (defined(USE_LIBSECRET) || defined(USE_KWALLET)) && defined(UNIT_TEST)
#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(UNIT_TEST)
// For unit testing purposes, inject methods to be used.
// |get_key_storage_mock| provides the desired |KeyStorage| implementation.
// If the provider returns |nullptr|, a hardcoded password will be used.
// |get_password_v11_mock| provides a password to derive the encryption key from
// If both parameters are |nullptr|, the real implementation is restored.
void UseMockKeyStorageForTesting(KeyStorageLinux* (*get_key_storage_mock)(),
std::string* (*get_password_v11_mock)());
#endif // (defined(USE_LIBSECRET) || defined(USE_KWALLET)) &&
// defined(UNIT_TEST)
#endif // defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(UNIT_TEST)

#endif // COMPONENTS_OS_CRYPT_OS_CRYPT_H_
8 changes: 8 additions & 0 deletions components/os_crypt/os_crypt_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,14 @@ bool OSCrypt::DecryptString(const std::string& ciphertext,
return true;
}

// static
void OSCrypt::SetStore(const std::string& store_type) {
// Changing the targeted password store makes no sense after initializing.
DCHECK(!g_cache.Get().is_key_storage_cached);

KeyStorageLinux::SetStore(store_type);
}

void UseMockKeyStorageForTesting(KeyStorageLinux* (*get_key_storage_mock)(),
std::string* (*get_password_v11_mock)()) {
// Save the real implementation to restore it later.
Expand Down

0 comments on commit 92699e3

Please sign in to comment.