New issue
Advanced search Search tips
Starred by 11 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Feature

Blocked on:
issue 606303
issue 639298

Blocking:
issue 571003



Sign in to add a comment

Implement components/os_crypt on Linux

Project Member Reported by vabr@chromium.org, Apr 12 2016

Issue description

Currently, GetEncryptionKey for Linux (components/os_crypt/os_crypt_posix.cc) uses a hard-coded password. Instead, it should use a randomly generated password stored in the OS secure storage if available (Gnome Keyring or KDE Wallet Manager), and only use the hard-coded one as a fallback.

Doing so will mimick what happens on Mac with the Keychain.

Once this encryption is improved, we can implement bug 571003 by using Login DB and this new encryption.

The sample code for accessing KWallet is in chrome/browser/password_manager/native_backend_kwallet_x.cc, similarly for Gnome Keyring (there are two libraries currently used for that, will clarify that in the comments).
 

Comment 1 by vabr@chromium.org, Apr 12 2016

@dvadym -- do we know whether for Gnome Keyring we can just use libsecret, or whether we need to keep also the older interface?

Comment 2 by vabr@chromium.org, Apr 12 2016

Blocking: 571003

Comment 3 by dvadym@chromium.org, Apr 12 2016

We also need keyring library interface since according to UMA still about 10% users don't have libsecret.

Comment 4 by vabr@chromium.org, Apr 15 2016

Thanks, dvadym!
Which histogram tracks that?
Suppose that our users already have encryption from a hardcoded key. After we replace that key with a new generated key, they will no longer have access to their data.

We need to either migrate their data or keep accessing existing data with the old key.

@vabr can you send me documentation on how things like that were migrated in the past?

Comment 6 by dvadym@chromium.org, Apr 18 2016

Histogram is called LinuxBackendStatistics here is place in  code where it's sent https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/password_manager/password_store_factory.cc&sq=package:chromium&q=linuxbackendstatistics&l=358
But now it's broken and it's not shown in UMA dashboards. Probably there is something incorrect in it's implementation. I noticed before that there were no names for enums in the dashboard. It should be fairly easy to fix this metric. About 2-3 months ago the data shows results that I wrote in #3. 

Comment 7 by vabr@chromium.org, Apr 18 2016

To be completely clear, the migration mentioned in #5 would be for users on Linux without either Gnome Keyring or KWallet. For those users nothing should change, unless we figure out how to implement a good way to store a password to encrypt their password store with (recall that they have neither Gnome Keyring not KWallet). So we should not have to migrate them at all.

For the rest, i.e., the migration from Keyring/KWallet to the Login DB, we will ultimately need to recognise the state of the profile data safely (maybe we can use the content of the Keyring/KWallet and Login DB, but more likely we'll have to store some preference, similar to what we did for Mac [1]. However, that step does not block introducing the encryption on Linux, rather the opposite (it is blocked by introducing the encryption).

[1] https://docs.google.com/document/d/1A8ZG16bLuUH1u21K0GoABKz_wpz1kchXMnMlpmq_ecA/edit#heading=h.uedth21l2ca0
Blockedon: 606303
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df99cec1248d89188156bf53aba322c3a9e0f0bb

commit df99cec1248d89188156bf53aba322c3a9e0f0bb
Author: cfroussios <cfroussios@chromium.org>
Date: Fri Apr 29 18:42:50 2016

Changed location of Libsecret utils

Moved classes LibsecretLoader and LibsecretAttributeBuilder
from password_manager to os_crypt. In the future,
password_manager will not interact with libsecret directly,
while os_crypt will. Responsibility is therefore
transfered. For now, password_manager continues to use
those utilites.

Currently, Password Manager stores passwords one by one in
libsecret. With this and future CL(s), we will only store
an encryption key with libsecret and use said key to
protect a local database, managed by the Password Manager.

BUG= 602624 
TBR=caitkp@chromium.org for components/os_crypt.gypi

Review-Url: https://codereview.chromium.org/1929573002
Cr-Commit-Position: refs/heads/master@{#390706}

[modify] https://crrev.com/df99cec1248d89188156bf53aba322c3a9e0f0bb/chrome/browser/password_manager/native_backend_libsecret.cc
[modify] https://crrev.com/df99cec1248d89188156bf53aba322c3a9e0f0bb/chrome/browser/password_manager/native_backend_libsecret.h
[modify] https://crrev.com/df99cec1248d89188156bf53aba322c3a9e0f0bb/chrome/browser/password_manager/native_backend_libsecret_unittest.cc
[modify] https://crrev.com/df99cec1248d89188156bf53aba322c3a9e0f0bb/components/os_crypt.gypi
[modify] https://crrev.com/df99cec1248d89188156bf53aba322c3a9e0f0bb/components/os_crypt/BUILD.gn
[add] https://crrev.com/df99cec1248d89188156bf53aba322c3a9e0f0bb/components/os_crypt/libsecret_util_posix.cc
[add] https://crrev.com/df99cec1248d89188156bf53aba322c3a9e0f0bb/components/os_crypt/libsecret_util_posix.h

Project Member

Comment 10 by bugdroid1@chromium.org, May 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d281ff9e63f782362ef6ea0ccdde6c1238f34431

commit d281ff9e63f782362ef6ea0ccdde6c1238f34431
Author: cfroussios <cfroussios@chromium.org>
Date: Fri May 27 15:37:06 2016

OSCryptMocker

OSCrypt uses OS-level password storage services (e.g.
Keychain). Tests should avoid involving such services.
Currently, tests do that with OS-specific code. Soon,
linux will need such mocking too.

This CL introduces the build target
//components/os_crypt:test_support, which contains an
abstraction for mocking: OSCryptMocker. This abstraction
replaces previous OS-specific mocking instructions in
tests. Additionally, I added it to some
tests which previously omitted mocking entirely.

BUG= 602624 

Review-Url: https://codereview.chromium.org/2010463002
Cr-Commit-Position: refs/heads/master@{#396472}

[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/autofill/autofill_browsertest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/autofill/autofill_interactive_uitest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/password_manager/password_generation_interactive_uitest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/password_manager/password_manager_browsertest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/password_manager/password_store_mac_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/password_manager/password_store_proxy_mac_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/password_manager/simple_password_store_mac_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/signin/local_auth_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/sync/test/integration/sync_test.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/browser/ui/webui/bidi_checker_web_ui_test.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/chrome_tests.gypi
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/test/BUILD.gn
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/chrome/test/base/in_process_browser_test.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/autofill/core/browser/BUILD.gn
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/autofill/core/browser/autofill_merge_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/autofill/core/browser/autofill_test_utils.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/autofill/core/browser/autofill_test_utils.h
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/autofill/core/browser/webdata/autofill_table_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/components_tests.gyp
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/os_crypt.gypi
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/os_crypt/BUILD.gn
[add] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/os_crypt/os_crypt_mocker.cc
[add] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/os_crypt/os_crypt_mocker.h
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/os_crypt/os_crypt_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/password_manager/core/browser/BUILD.gn
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/password_manager/core/browser/login_database_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/signin/core/browser/BUILD.gn
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/signin/core/browser/webdata/token_service_table_unittest.cc
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/sync_driver/BUILD.gn
[modify] https://crrev.com/d281ff9e63f782362ef6ea0ccdde6c1238f34431/components/sync_driver/system_encryptor_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, May 31 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8

commit 3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8
Author: cfroussios <cfroussios@chromium.org>
Date: Tue May 31 11:02:18 2016

Implement OSCrypt for Linux.

OSCrypt for linux uses libsecret to store a randomised
encryption key.

Mentions of OSCrypt below refer to the linux version.

OSCrypt now supports a new version of encryption. If
libsecret is available, then a randomised password will be
generated, stored in libsecret and used for producing
encryption keys. The old ways of encrypting, i.e. using a
hardcoded password and using no encryption at all, are still
supported.

Data encrypted with the new version are marked with a version
prefix, "v11". That is an increment on the previous prefix,
"v10", which separated data that was encrypted with the
hardcoded password from data that were not encrypted.

To separate this new linux feature from the generic posix
implementation, a new implementation for OSCrypt was created
in os_crypt_linux.cc.

Since there is support on the way for more services in
addition to libsecret (gnome keyring, kwallet), interaction
with such services is abstracted with the KeyStorageLinux API.

The libsecret utilities that KeyStorageLinux uses
(i.e. LibsecretLoader) are also used by the PasswordManager.
Here we only actually need a subset of their features. Once
PasswordManager stops depending on them, they can be
simplified and/or hidden in the KeyStorageLibsecret
implementation.

BUG= 602624 

Review-Url: https://codereview.chromium.org/1973483002
Cr-Commit-Position: refs/heads/master@{#396814}

[modify] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/chrome/browser/password_manager/native_backend_libsecret.h
[modify] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/components_tests.gyp
[modify] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt.gypi
[modify] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/BUILD.gn
[add] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/key_storage_libsecret.cc
[add] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/key_storage_libsecret.h
[add] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/key_storage_linux.cc
[add] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/key_storage_linux.h
[rename] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/libsecret_util_linux.cc
[rename] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/libsecret_util_linux.h
[modify] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/os_crypt.h
[add] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/os_crypt_linux.cc
[add] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/os_crypt_linux_unittest.cc
[modify] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/os_crypt_mocker.cc
[add] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/os_crypt_mocker_linux.cc
[add] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/os_crypt_mocker_linux.h
[add] https://crrev.com/3b5a4e4f0f10e73b9ace1e61079b099a9e62b1d8/components/os_crypt/os_crypt_util_linux_unittest.cc

Comment 12 by vabr@chromium.org, Jun 6 2016

Labels: -refactoring Hotlist-Refactoring
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 22 2016

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/450339b9492844c74ba3b07aebca7d6c5770b984

commit 450339b9492844c74ba3b07aebca7d6c5770b984
Author: j.isorce <j.isorce@samsung.com>
Date: Thu Jun 23 14:30:44 2016

Fix build error with gcc 4.9.3

native_backend_kwallet_x.cc:391:1:
  error: control reaches end of non-void function [-Werror=return-type]

Regression introduced by https://codereview.chromium.org/2057123002

BUG= 602624 

R=vabr@chromium.org

Review-Url: https://codereview.chromium.org/2090343002
Cr-Commit-Position: refs/heads/master@{#401601}

[modify] https://crrev.com/450339b9492844c74ba3b07aebca7d6c5770b984/chrome/browser/password_manager/native_backend_kwallet_x.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Jun 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf

commit e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf
Author: cfroussios <cfroussios@chromium.org>
Date: Fri Jun 24 09:10:47 2016

Move KWalletDBus util from the Password Manager to OSCrypt.

This is part of deprecating the use of OS services for
storing passwords with Password Manager.

The moved util will be used to create a specialization of
KeyStorage for KWallet.

BUG= 602624 

TBR=davidben@chromium.org

Review-Url: https://codereview.chromium.org/2086123003
Cr-Commit-Position: refs/heads/master@{#401836}

[modify] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/chrome/browser/password_manager/native_backend_kwallet_x.h
[modify] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/chrome/chrome_browser.gypi
[modify] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/components/components_tests.gyp
[modify] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/components/os_crypt.gypi
[modify] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/components/os_crypt/BUILD.gn
[modify] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/components/os_crypt/DEPS
[rename] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/components/os_crypt/kwallet_dbus.cc
[rename] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/components/os_crypt/kwallet_dbus.h
[rename] https://crrev.com/e5d0dfbd158dad8e3f976fc493a8ea8fcdec29cf/components/os_crypt/kwallet_dbus_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Jun 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fb11a7da3cae85b93c68a905077f458beecd0127

commit fb11a7da3cae85b93c68a905077f458beecd0127
Author: cfroussios <cfroussios@chromium.org>
Date: Tue Jun 28 15:02:35 2016

Fix guard for UseMockKeyStorageForTesting

https://codereview.chromium.org/2086123003/
introduced the possibility to either use Libsecret or KWallet.

I'm updating a guard that I forgot to reflect the change.

BUG= 602624 

Review-Url: https://codereview.chromium.org/2104943002
Cr-Commit-Position: refs/heads/master@{#402469}

[modify] https://crrev.com/fb11a7da3cae85b93c68a905077f458beecd0127/components/os_crypt/os_crypt.h

Project Member

Comment 17 by bugdroid1@chromium.org, Jul 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/92699e345c15243c4023a8739101333ed2658e37

commit 92699e345c15243c4023a8739101333ed2658e37
Author: cfroussios <cfroussios@chromium.org>
Date: Wed Jul 13 10:38:05 2016

Forward password-store switch to OSCrypt component

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}

[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/chrome/browser/chrome_browser_main_linux.cc
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/components_tests.gyp
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt.gypi
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/BUILD.gn
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/key_storage_linux.cc
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/key_storage_linux.h
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/os_crypt.h
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/os_crypt_linux.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Jul 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e8f3ac9fc9e4a8588e900b7fce5492c26da1986

commit 7e8f3ac9fc9e4a8588e900b7fce5492c26da1986
Author: cfroussios <cfroussios@chromium.org>
Date: Wed Jul 13 16:58:15 2016

Revert of Forward password-store switch to OSCrypt component (patchset #15 id:310001 of https://codereview.chromium.org/2118443002/ )

Reason for revert:
mahmadi@ reported reaching the DCHECK in components/os_crypt/os_crypt_linux.cc

Original issue's description:
> Forward password-store switch to OSCrypt component
>
> 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 
>
> Committed: https://crrev.com/92699e345c15243c4023a8739101333ed2658e37
> Cr-Commit-Position: refs/heads/master@{#405114}

TBR=thestig@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 602624 

Review-Url: https://codereview.chromium.org/2152493002
Cr-Commit-Position: refs/heads/master@{#405197}

[modify] https://crrev.com/7e8f3ac9fc9e4a8588e900b7fce5492c26da1986/chrome/browser/chrome_browser_main_linux.cc
[modify] https://crrev.com/7e8f3ac9fc9e4a8588e900b7fce5492c26da1986/components/components_tests.gyp
[modify] https://crrev.com/7e8f3ac9fc9e4a8588e900b7fce5492c26da1986/components/os_crypt.gypi
[modify] https://crrev.com/7e8f3ac9fc9e4a8588e900b7fce5492c26da1986/components/os_crypt/BUILD.gn
[modify] https://crrev.com/7e8f3ac9fc9e4a8588e900b7fce5492c26da1986/components/os_crypt/key_storage_linux.cc
[modify] https://crrev.com/7e8f3ac9fc9e4a8588e900b7fce5492c26da1986/components/os_crypt/key_storage_linux.h
[modify] https://crrev.com/7e8f3ac9fc9e4a8588e900b7fce5492c26da1986/components/os_crypt/os_crypt.h
[modify] https://crrev.com/7e8f3ac9fc9e4a8588e900b7fce5492c26da1986/components/os_crypt/os_crypt_linux.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Jul 13 2016

Labels: merge-merged-2795
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/92699e345c15243c4023a8739101333ed2658e37

commit 92699e345c15243c4023a8739101333ed2658e37
Author: cfroussios <cfroussios@chromium.org>
Date: Wed Jul 13 10:38:05 2016

Forward password-store switch to OSCrypt component

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}

[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/chrome/browser/chrome_browser_main_linux.cc
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/components_tests.gyp
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt.gypi
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/BUILD.gn
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/key_storage_linux.cc
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/key_storage_linux.h
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/os_crypt.h
[modify] https://crrev.com/92699e345c15243c4023a8739101333ed2658e37/components/os_crypt/os_crypt_linux.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Jul 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ea4c69178fd68bcdc880bc2c83481e57e293403

commit 3ea4c69178fd68bcdc880bc2c83481e57e293403
Author: cfroussios <cfroussios@chromium.org>
Date: Mon Jul 18 19:15:14 2016

Forward --password-store switch to os_crypt

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.

This CL is a fixed version of https://codereview.chromium.org/2118443002/

BUG= 602624 

Review-Url: https://codereview.chromium.org/2159743002
Cr-Commit-Position: refs/heads/master@{#406056}

[modify] https://crrev.com/3ea4c69178fd68bcdc880bc2c83481e57e293403/chrome/browser/chrome_browser_main_linux.cc
[modify] https://crrev.com/3ea4c69178fd68bcdc880bc2c83481e57e293403/components/components_tests.gyp
[modify] https://crrev.com/3ea4c69178fd68bcdc880bc2c83481e57e293403/components/os_crypt.gypi
[modify] https://crrev.com/3ea4c69178fd68bcdc880bc2c83481e57e293403/components/os_crypt/BUILD.gn
[modify] https://crrev.com/3ea4c69178fd68bcdc880bc2c83481e57e293403/components/os_crypt/key_storage_linux.cc
[modify] https://crrev.com/3ea4c69178fd68bcdc880bc2c83481e57e293403/components/os_crypt/key_storage_linux.h
[modify] https://crrev.com/3ea4c69178fd68bcdc880bc2c83481e57e293403/components/os_crypt/os_crypt.h
[modify] https://crrev.com/3ea4c69178fd68bcdc880bc2c83481e57e293403/components/os_crypt/os_crypt_linux.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e6729a44f5a07c939209bce99f803dc90c5966f

commit 2e6729a44f5a07c939209bce99f803dc90c5966f
Author: cfroussios <cfroussios@chromium.org>
Date: Tue Jul 26 09:18:12 2016

OSCrypt supports encryption with KWallet

Implemented KeyStorageLinux for KWallet.

BUG= 602624 

Review-Url: https://codereview.chromium.org/2150543002
Cr-Commit-Position: refs/heads/master@{#407746}

[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/chrome/browser/chrome_browser_main_linux.cc
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/components_tests.gyp
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt.gypi
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/BUILD.gn
[add] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/key_storage_kwallet.cc
[add] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/key_storage_kwallet.h
[add] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/key_storage_kwallet_unittest.cc
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/key_storage_libsecret.cc
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/key_storage_libsecret.h
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/key_storage_linux.cc
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/key_storage_linux.h
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/kwallet_dbus.cc
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/kwallet_dbus.h
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/kwallet_dbus_unittest.cc
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/os_crypt.h
[modify] https://crrev.com/2e6729a44f5a07c939209bce99f803dc90c5966f/components/os_crypt/os_crypt_linux.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da4e6a834587d15f7696c5a396099fafa719ddf1

commit da4e6a834587d15f7696c5a396099fafa719ddf1
Author: cfroussios <cfroussios@chromium.org>
Date: Mon Aug 01 07:37:07 2016

Deprecate DLOPEN_GNOME_KEYRING flag

GnomeKeyringLoader will be moved into a separate utility.
This change makes GnomeKeyringLoader simpler to build on and easier
to move.

Currently, GnomeKeyringLoader compiles different code, depending on
whether keyring is statically linked. Now it will only do dynamic
loading. The only place that requires static linking is tests, and they
don't need GnomeKeyringLoader to do it for them.

BUG= 602624 

Review-Url: https://codereview.chromium.org/2191873002
Cr-Commit-Position: refs/heads/master@{#408924}

[modify] https://crrev.com/da4e6a834587d15f7696c5a396099fafa719ddf1/build/common.gypi
[modify] https://crrev.com/da4e6a834587d15f7696c5a396099fafa719ddf1/build/linux/system.gyp
[modify] https://crrev.com/da4e6a834587d15f7696c5a396099fafa719ddf1/chrome/browser/BUILD.gn
[modify] https://crrev.com/da4e6a834587d15f7696c5a396099fafa719ddf1/chrome/browser/password_manager/native_backend_gnome_x.cc
[modify] https://crrev.com/da4e6a834587d15f7696c5a396099fafa719ddf1/chrome/browser/password_manager/native_backend_gnome_x.h
[modify] https://crrev.com/da4e6a834587d15f7696c5a396099fafa719ddf1/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc

Comment 25 by mfor...@gmail.com, Aug 13 2016

 I've opened  Issue 637551  for the fact that GN always has gnome keyring enabled. Maybe someone here can take a look at that. Should be a straightforward fix.
Project Member

Comment 26 by bugdroid1@chromium.org, Aug 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4e3e52dff0f5b9dff7167a1564b0e1978d3ea2e

commit c4e3e52dff0f5b9dff7167a1564b0e1978d3ea2e
Author: raphael.kubo.da.costa <raphael.kubo.da.costa@intel.com>
Date: Tue Aug 16 15:26:48 2016

gn: Use a separate flag for enabling libgnome-keyring support.

Right now, |is_desktop_linux| means GN requires libgnome-keyring to
be present during the build and also at run-time.

Since libgnome-keyring has been deprecated by libsecret, is used by a
small number of users and is not always present on a Linux
distribution (especially embedded ones), it makes sense to introduce a
specific argument, |use_gnome_keyring|, to control whether the code
should be used or not.

At the moment, |use_gnome_keyring| defaults to on under pretty much all
circumstances where checking for |is_desktop_linux| would work before:
it just needs |use_glib| to be on as well, as required by
libgnome-keyring itself.

R=brettw@chromium.org,vabr@chromium.org,thestig@chromium.org,jochen@chromium.org
BUG=466975, 602624 

Review-Url: https://codereview.chromium.org/2244653002
Cr-Commit-Position: refs/heads/master@{#412246}

[modify] https://crrev.com/c4e3e52dff0f5b9dff7167a1564b0e1978d3ea2e/chrome/browser/BUILD.gn
[modify] https://crrev.com/c4e3e52dff0f5b9dff7167a1564b0e1978d3ea2e/chrome/test/BUILD.gn
[modify] https://crrev.com/c4e3e52dff0f5b9dff7167a1564b0e1978d3ea2e/components/os_crypt/BUILD.gn
[add] https://crrev.com/c4e3e52dff0f5b9dff7167a1564b0e1978d3ea2e/components/os_crypt/features.gni
[modify] https://crrev.com/c4e3e52dff0f5b9dff7167a1564b0e1978d3ea2e/tools/gn/docs/cookbook.md

Project Member

Comment 27 by bugdroid1@chromium.org, Aug 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f05d7ffa621967ac92a5dcae91173647fc735c4

commit 2f05d7ffa621967ac92a5dcae91173647fc735c4
Author: cfroussios <cfroussios@chromium.org>
Date: Wed Aug 17 15:58:50 2016

Add two more options for the --password-store switch

Currently the option "gnome" is ambiguous between Keyring and Libsecret.
The two new options, "gnome-keyring" and "gnome-libsecret" allow
targeting those implementations individually.

Since password manager and OSCrypt both use this switch to select their
backends, the selection logic has been turned into a common util, to
avoid code duplication.

BUG= 602624 

Review-Url: https://codereview.chromium.org/2256633002
Cr-Commit-Position: refs/heads/master@{#412540}

[modify] https://crrev.com/2f05d7ffa621967ac92a5dcae91173647fc735c4/chrome/browser/password_manager/password_store_factory.cc
[modify] https://crrev.com/2f05d7ffa621967ac92a5dcae91173647fc735c4/chrome/common/chrome_switches.cc
[modify] https://crrev.com/2f05d7ffa621967ac92a5dcae91173647fc735c4/components/os_crypt/BUILD.gn
[modify] https://crrev.com/2f05d7ffa621967ac92a5dcae91173647fc735c4/components/os_crypt/key_storage_linux.cc
[add] https://crrev.com/2f05d7ffa621967ac92a5dcae91173647fc735c4/components/os_crypt/key_storage_util_linux.cc
[add] https://crrev.com/2f05d7ffa621967ac92a5dcae91173647fc735c4/components/os_crypt/key_storage_util_linux.h

Project Member

Comment 28 by bugdroid1@chromium.org, Aug 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c51d7ae19a5e001eb9aea418b3efc83fcc9b4649

commit c51d7ae19a5e001eb9aea418b3efc83fcc9b4649
Author: cfroussios <cfroussios@chromium.org>
Date: Fri Aug 19 12:01:14 2016

Initialize OSCrypt with a TaskRunner on the main thread

Gnome-Keyring requires calls to originate from the main thread. In order
to support Gnome-Keyring on OSCrypt, we initialize it with a SingleThreadTaskRunner.

In addition, the various static fields for OSCrypt's initialization
have been grouped into a struct.

BUG= 602624 

Review-Url: https://codereview.chromium.org/2262693002
Cr-Commit-Position: refs/heads/master@{#413110}

[modify] https://crrev.com/c51d7ae19a5e001eb9aea418b3efc83fcc9b4649/chrome/browser/chrome_browser_main_linux.cc
[modify] https://crrev.com/c51d7ae19a5e001eb9aea418b3efc83fcc9b4649/components/os_crypt/key_storage_linux.cc
[modify] https://crrev.com/c51d7ae19a5e001eb9aea418b3efc83fcc9b4649/components/os_crypt/key_storage_linux.h
[modify] https://crrev.com/c51d7ae19a5e001eb9aea418b3efc83fcc9b4649/components/os_crypt/os_crypt.h
[modify] https://crrev.com/c51d7ae19a5e001eb9aea418b3efc83fcc9b4649/components/os_crypt/os_crypt_linux.cc

Blockedon: 639298
Project Member

Comment 30 by bugdroid1@chromium.org, Sep 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b013c15b73b3f71e2ec1636ce27b93b4dde04361

commit b013c15b73b3f71e2ec1636ce27b93b4dde04361
Author: cfroussios <cfroussios@chromium.org>
Date: Sat Sep 03 01:10:16 2016

Implement gnome-keyring for OSCrypt

OSCrypt can now save its encryption password in gnome-keyring, where
that library exists.

BUG= 602624 

Review-Url: https://codereview.chromium.org/2297573002
Cr-Commit-Position: refs/heads/master@{#416423}

[modify] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/BUILD.gn
[add] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/key_storage_keyring.cc
[add] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/key_storage_keyring.h
[add] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/key_storage_keyring_unittest.cc
[modify] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/key_storage_libsecret_unittest.cc
[modify] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/key_storage_linux.cc
[modify] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/key_storage_linux.h
[modify] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/keyring_util_linux.cc
[modify] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/keyring_util_linux.h
[modify] https://crrev.com/b013c15b73b3f71e2ec1636ce27b93b4dde04361/components/os_crypt/os_crypt_mocker.cc

Comment 31 by vabr@chromium.org, Sep 6 2016

Note: r406056 from here has been approved for merge and merged in  http://crbug.com/643189#c21 .
Status: Fixed (was: Assigned)
OSCrypt now supports the same three libraries as Password Manager.
Where can we read documentation about how to migrate from GNOME keyring or KWallet to OSCrypt?
It is currently not possible to have passwords encrypted with OSCrypt.

Passwords will be automatically taken out of Keyring/KWallet when issue 571003 is implemented.
Thanks for the info.

Sign in to add a comment