New issue
Advanced search Search tips

Issue 709096 link

Starred by 9 users

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 571003



Sign in to add a comment

Introduce a setting to ignore gnome-keyring/kwallet

Project Member Reported by cfroussios@chromium.org, Apr 6 2017

Issue description

There are several problems related to gnome-keyring/kwallet (a.k.a. the backend) prompts, such as issue 690435, issue 658348

Common themes are
 * To handle the case where a user inconsistently grants access to the backends, components depending (directly or indirectly) on the backend must have fallbacks and awareness that carries between runs. This increases code complexity. Components that don't expect this and can malfunction.
 * The user gets interrupted by a prompt, which is produced non-deterministically by the inner workings of Chrome.
 * Chrome malfunctions, because the user is not reacting to the prompt for unlocking the backend, leading one or more components to block on (de)encryption.
 * Occasionally people mention their unhappiness with facing prompts at all.
 * Encryption of local data is redundant when the environment is already trusted and protected, e.g. by encrypting the entire drive.

The above is a family of issues, a subset of which can be solved by reworking how and what we expect from the backends. Primarily, we want to offer a promise of stability to the components interacting with backends.

We can achieve that by introducing a user setting on whether Chrome should use the backends. When enabled, access will be non-optional. Users who aren't interested in the value of encryption, either at all or enough to grant access consistently, will now be able to opt out.
 
Description: Show this description
Summary: Introduce a setting to ignore gnome-keyring/kwallet (was: Eagerly get access to gnome-keyring/kwallet)
Status: Started (was: Assigned)
The following design doc aims to address this problem
https://docs.google.com/document/d/1oNkL2PBig5AAEBScPu1ajRiei_vz7PTP4op7vyXAa10/edit

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 6 2017

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

commit 6b340f81b25adc1e97c50247aa3bf235eabcbf7e
Author: cfroussios <cfroussios@chromium.org>
Date: Thu Jul 06 15:05:10 2017

Create setting that disables password stores

A new setting is implemented, which can be set to prevent
OSCrypt/Password Manager from using a backend (i.e. keyring/kwallet).
Since Password Manager is expected to deprecate its backends, OSCrypt
is given ownership of the setting.

OSCrypt needs to be ready for use during profile initialisation,
therefore profile preferences cannot be used for this setting. The
setting is stored in a file in the chrome's user data directory. It is
implemented as a single empty file, whose presence or absence in the
filesystem defines whether the setting is set to true or false.

Additionally, a new commandline switch is implemented:
--enable-encryption-selection. This controls whether the feature above
will be used. When the feature is disabled, current behaviour is for
Chrome to try and reach the appropriate backend, as determined by the
OS environment.
The --password-store flag overrules both of the above behaviours.

BUG=709096

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

[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/chrome/browser/chrome_browser_main_linux.cc
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/chrome/browser/password_manager/password_store_factory.cc
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/chrome/common/chrome_switches.cc
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/chrome/common/chrome_switches.h
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/components/os_crypt/BUILD.gn
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/components/os_crypt/key_storage_linux.cc
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/components/os_crypt/key_storage_linux.h
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/components/os_crypt/key_storage_util_linux.cc
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/components/os_crypt/key_storage_util_linux.h
[add] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/components/os_crypt/key_storage_util_linux_unittest.cc
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/components/os_crypt/os_crypt.h
[modify] https://crrev.com/6b340f81b25adc1e97c50247aa3bf235eabcbf7e/components/os_crypt/os_crypt_linux.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 14 2017

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

commit 494196d155b5be1fd1e3b07b530dddfde90b02dc
Author: Christos Froussios <cfroussios@chromium.org>
Date: Fri Jul 14 10:10:04 2017

Consolidate config parameters for OSCrypt

The individual setters for each parameter required for OSCrypt's
initialisation are replaced by a single setter for a Config struct.
The Config definition is moved to a separate file.

The result is less code, especially in the top level files of OSCrypt.

Bug:709096

Change-Id: If6f05129879fcae86b1b2bf032641b05418fe8ee
Reviewed-on: https://chromium-review.googlesource.com/565567
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486717}
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/chrome/browser/chrome_browser_main_linux.cc
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/os_crypt/BUILD.gn
[add] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/os_crypt/key_storage_config_linux.cc
[add] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/os_crypt/key_storage_config_linux.h
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/os_crypt/key_storage_keyring.h
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/os_crypt/key_storage_linux.cc
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/os_crypt/key_storage_linux.h
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/os_crypt/os_crypt.h
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/os_crypt/os_crypt_linux.cc
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/os_crypt/os_crypt_mocker_linux.cc
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/password_manager/core/browser/hash_password_manager_unittest.cc
[modify] https://crrev.com/494196d155b5be1fd1e3b07b530dddfde90b02dc/components/password_manager/core/browser/password_store_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 21 2017

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

commit 2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc
Author: Christos Froussios <cfroussios@chromium.org>
Date: Fri Jul 21 16:30:34 2017

Cleanup OSCrypt after initialisation

OSCrypt only needs to read from the backend during its initialisation.
Instances created for this initialisation can be cleaned up afterwards.

To support this change, the mocking of OSCrypt can no longer be based
on a singleton. Tests using the mocking mechanism are updated.

Bug: 709096
Change-Id: Iba5b37ad45ada11b9a217dc2c388313f2371f647
Reviewed-on: https://chromium-review.googlesource.com/571221
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488675}
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/chrome/browser/password_manager/password_store_mac_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/chrome/browser/signin/local_auth_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/chrome/browser/sync/test/integration/sync_test.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/chrome/test/base/in_process_browser_test.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/autofill/core/browser/autofill_test_utils.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/autofill/core/browser/webdata/autofill_table_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/os_crypt/os_crypt.h
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/os_crypt/os_crypt_linux.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/os_crypt/os_crypt_linux_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/os_crypt/os_crypt_mocker.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/os_crypt/os_crypt_mocker.h
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/os_crypt/os_crypt_mocker_linux.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/os_crypt/os_crypt_mocker_linux.h
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/os_crypt/os_crypt_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/password_manager/core/browser/hash_password_manager_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/password_manager/core/browser/login_database_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/password_manager/core/browser/password_store_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/signin/core/browser/webdata/token_service_table_unittest.cc
[modify] https://crrev.com/2d15a8eb787a31d5bbab0ff4a6a539a0df7c99dc/components/sync/base/system_encryptor_unittest.cc

Sign in to add a comment