New issue
Advanced search Search tips

Issue 660005 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Libsecret API is not healthy

Project Member Reported by cfroussios@chromium.org, Oct 27 2016

Issue description

The gnome libsecret API has some dangerous quirks (probably bugs) w.r.t. unlocking keyrings.

*Cold keyrings may be ignored by lookup operations*

In this case, by "cold keyring" I mean a keyring that was not unlocked since the beginning of the user session, or since the last time its password was changed. A keyring that was unlocked and then locked again is NOT a cold keyring.

The methods _secret_service_lookup_sync_ and _secret_service_search_sync_ will ignore items in a cold keyring. They won't even prompt to unlock the keyring.
If the keyring isn't cold, i.e. it has been unlocked by whatever means since the beginning of the user session, and regardless of whether it has been locked again, lookup operations will discover items in them and prompt to unlock if necessary.

Store operations into keyrings (secret_password_store_sync) oddly enough do not suffer from the same quirk. As far as my testing goes, a store operation will always trigger the relevant unlock prompt.


I have put together some demonstrating code that is independent of Chrome (see attachment, warning: dirty code), removing plausible causes beyond the API, such as race conditions and faulty initialisations in Chrome.


The effect of these quirks is that lookup operations from Chrome may fail until the first store operation is performed. That store operation is either Password Manager repopulating what appeared as an empty keyring ( issue 657828 ) or OSCrypt generating and storing a new encryption key ( issue 631171 ).


Apparently this issue has been reported to GNOME, with no visible reaction from them.
https://bugzilla.gnome.org/show_bug.cgi?id=748625
 
main.cpp
3.0 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 28 2016

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

commit f3a8dfe38fab661729155dac82c8ab7d8422c2e1
Author: cfroussios <cfroussios@chromium.org>
Date: Fri Oct 28 15:30:32 2016

Add a dummy entry with libsecret when initializing OSCrypt.

Under certain conditions, gnome libsecret methods that perform a lookup
in keyring may ignore a locked keyring and return empty results, instead
of unlocking said keyring. Store operations are unaffected by this quirk
(or bug, presumably).

With this change, we store a dummy entry during the initialisation of
OSCrypt. This ensures that the default keyring gets unlocked and is
available for future reads.

This solution is also applied to Password Manager, before performing
calls to secret_service_search_sync.

This solution should be temporary, until the underlying issue gets fixed
or there is an official workaround.

BUG=660005, 631171 

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

[modify] https://crrev.com/f3a8dfe38fab661729155dac82c8ab7d8422c2e1/chrome/browser/password_manager/native_backend_libsecret.cc
[modify] https://crrev.com/f3a8dfe38fab661729155dac82c8ab7d8422c2e1/chrome/browser/password_manager/native_backend_libsecret.h
[modify] https://crrev.com/f3a8dfe38fab661729155dac82c8ab7d8422c2e1/chrome/browser/password_manager/native_backend_libsecret_unittest.cc
[modify] https://crrev.com/f3a8dfe38fab661729155dac82c8ab7d8422c2e1/components/os_crypt/key_storage_libsecret.cc
[modify] https://crrev.com/f3a8dfe38fab661729155dac82c8ab7d8422c2e1/components/os_crypt/key_storage_libsecret_unittest.cc
[modify] https://crrev.com/f3a8dfe38fab661729155dac82c8ab7d8422c2e1/components/os_crypt/libsecret_util_linux.cc
[modify] https://crrev.com/f3a8dfe38fab661729155dac82c8ab7d8422c2e1/components/os_crypt/libsecret_util_linux.h

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 1 2016

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

commit dc803359715785beade70e1af0a9b63930a6144d
Author: thomasanderson <thomasanderson@google.com>
Date: Tue Nov 01 00:12:33 2016

Add a dummy entry with libsecret when initializing OSCrypt.

Under certain conditions, gnome libsecret methods that perform a lookup
in keyring may ignore a locked keyring and return empty results, instead
of unlocking said keyring. Store operations are unaffected by this quirk
(or bug, presumably).

With this change, we store a dummy entry during the initialisation of
OSCrypt. This ensures that the default keyring gets unlocked and is
available for future reads.

This solution is also applied to Password Manager, before performing
calls to secret_service_search_sync.

This solution should be temporary, until the underlying issue gets fixed
or there is an official workaround.

BUG=660005, 631171 

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

NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2465083002
Cr-Commit-Position: refs/branch-heads/2883@{#402}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/dc803359715785beade70e1af0a9b63930a6144d/chrome/browser/password_manager/native_backend_libsecret.cc
[modify] https://crrev.com/dc803359715785beade70e1af0a9b63930a6144d/chrome/browser/password_manager/native_backend_libsecret.h
[modify] https://crrev.com/dc803359715785beade70e1af0a9b63930a6144d/chrome/browser/password_manager/native_backend_libsecret_unittest.cc
[modify] https://crrev.com/dc803359715785beade70e1af0a9b63930a6144d/components/os_crypt/key_storage_libsecret.cc
[modify] https://crrev.com/dc803359715785beade70e1af0a9b63930a6144d/components/os_crypt/key_storage_libsecret_unittest.cc
[modify] https://crrev.com/dc803359715785beade70e1af0a9b63930a6144d/components/os_crypt/libsecret_util_linux.cc
[modify] https://crrev.com/dc803359715785beade70e1af0a9b63930a6144d/components/os_crypt/libsecret_util_linux.h

Description: Show this description
Components: UI>Browser>Passwords Internals>LocalDataEncryption
Cc: -vabr@chromium.org
vabr going hobby only -> reducing involvement.
Please contact me directly in urgent matters.
Just run into this after noticing those weird 'Chrome Safe Storage Control' entries in my keyring. After running the attached demo code, I can confirm the observed behaviour with 'libsecret 0.18.7'.

However, it seems that the provided 'secret-tool' utility, which I use in my Bash scripts, does not suffer from this, i.e. lookup and search operations do trigger the unlocking dialog when required. Further inspecting the utility's source code reveals that the 'schema' parameter in the 'secret_service_lookup_sync' and 'secret_service_search_sync' invocations is actually NULL!

So, I made the same modifications to the attached demo code and it now seems to be working! Now, the only time that the keyring is not unlocked is when a password with the supplied attributes does not already exist in the keyring, which makes sense I suppose. If such a password does exist, both functions do in fact trigger the unlocking dialog. I don't know if this behaviour is by design or if it's some kind of workaround that they use, however, it is consistent with the way they do it in 'secret-tool'.

Regarding the demo code, I also had to add a string attribute to the schema, when storing the password, in order for the lookup and search operations to filter the appropriate password entry. I also added an invocation to 'secret_item_load_secret_sync', before invoking 'secret_item_get_secret', otherwise the latter would return NULL, but this behaviour is specified in the API docs anyway.
main2.cpp
3.2 KB View Download

Sign in to add a comment