After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 746873 - Gnucash asks sql passwords before wallet password
Gnucash asks sql passwords before wallet password
Status: RESOLVED FIXED
Product: GnuCash
Classification: Other
Component: General
2.6.5
Other Linux
: Normal normal
: ---
Assigned To: Geert Janssens
gnucash-general-maint
Depends on: 748625
Blocks:
 
 
Reported: 2015-03-27 10:08 UTC by Fabio Coatti
Modified: 2018-06-29 23:39 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Fabio Coatti 2015-03-27 10:08:39 UTC
I'm using gnucash with mysql backend and I'm facing an issue with password management. My environment is KDE, but with all gnome/gtk libs needed by gnucash. Every time I start gnucash and access mysql backed data, gnucash asks for database user/password, then for keyring password. I expected that next time gnucash will ask only for keyring password, but instead it repeats the previous steps: mysql user/pass, then keyring. It seems to me that gnucash tries to save user/password in keyring but at following access it does not checks for keyring, or fails the check.
It is not a showstopper, but it can be a bit confusing and time consuming for users: if the keyring is not available (understandable, as I'm running on KDE), maybe gnucash should avoid to try to save the pass in it (or use kwallet instead, that would be a big win :D )
Comment 1 Geert Janssens 2015-03-27 17:41:56 UTC
Thank you for your report. I can't seem to reproduce this however.

For your information my main DE is also KDE, running on Fedora 21.

What I did is this:

1. Start gnucash with the --nofile switch. This will open an empty gnucash window.
2. Select File->Open
3. In the Open window, switch to mysql and enter the coordinates of my gnucash test database in mysql. I did enter the database password in this case as well.
4. Hit ok

=> At this point the gnome keyring password window pops up and asks for the keyring password. I expect this to happen because GnuCash will attempt to store the entered password in the keyring (it can't know at this point whether it was already in the keyring or whether it has changed or not).

5. Quit gnucash
6. Restart the pc, so the gnome keyring is closed again
7. Start gnucash - which will automatically attempt to reload the mysql database.

=> I'm immediately asked for the gnome keyring password.

The same happens if (again after a reboot) first open another file and then select the mysql database from the recently used files list.

And lastly if I choose File->Open (again after a reboot), enter all mysql parameters except the password, I will only be asked for the gnome keyring password.

Note that on Fedora the gnome-keyring will be installed and available even if your main environment is KDE. It's a rpm dependency for gnucash.

So I'll need some more input to debug this further. Which are the exact detailed steps you follow that cause gnucash to ask for the database password before asking the gnome-keyring password ?
Comment 2 John Ralls 2015-03-28 00:28:09 UTC
The last case might be what the OP is referring to: The DB URI and Password request are in the same box, so he may think that that's "asking for the password". If he fills it in he will be asked for his Keyring password so that GnuCash can check whether it's changed.
Comment 3 Fabio Coatti 2015-03-30 15:15:05 UTC
Well, it goes more or less this way:
- start gnucash (after a boot), --nofile
- select from last used databases the mysql entry that I want to connect to
a box appears asking for username and password to connect to <<mysql://IP/db>>
(it is the same if I enter manually the database name using open file menu item)
- after entering the mysql user/pass a box appears requesting the pass to unlock gnome keyring.

after giving the keyring pass the connection is established.

To get some clues about what's going on, I did the following: after the operation above, I closed gnucash, restarted it and selected the same database as before. Gnucash connected straigh away, being gnome-keyring already opened.

Not really sure about what's going on, maybe when gnome-keyring is not yet opened gnucash fails to open it?
Comment 4 Fabio Coatti 2015-04-21 09:33:49 UTC
After digging a little more, one thing that comes to my mind is that secret_password_lookup_sync() does not implies to unlock the keyring, so if the keyring is locked the schema can't be retrieved and the query fails (looking at dbus, the array with the answer is empty). 

This is related only to libsecret, not gnome-keyring (if HAVE_LIBSECRET is defined)

If I change the schema flag 
e.g.
-    "org.gnucash.password", SECRET_SCHEMA_NONE,
+    "org.gnucash.password", SECRET_SCHEMA_DONT_MATCH_NAME,

the query works as no schema name is involved, the keyring is unlocked and all works fine.

To see if this is indeed the case, I modified the source inserting, before password lookup, a couple of calls to open the keyring: (ok, ok, it's only a test, i know it's ugly)
=====
    secret_service = secret_service_get_sync (SECRET_SERVICE_LOAD_COLLECTIONS, NULL, &error);
    collections = secret_service_get_collections(secret_service);
    secret_service_unlock_sync(secret_service, collections, NULL, NULL, &error);

  libsecret_password = secret_password_lookup_sync (SECRET_SCHEMA_GNUCASH, ...
=====

And all seems to work fine, of course with SECRET_SCHEMA_NONE flag. right now I'm patching my sources to see if got it right and fix this behaviour (and maybe use a gnucash collection in keyring
). 
I can have completely missed the point, of course.

If you think I'm right and you are interested in a pull request, just let me know.

Thanks,

Fabio.
Comment 5 Geert Janssens 2015-04-21 11:11:31 UTC
(In reply to Fabio Coatti from comment #4)
> After digging a little more, one thing that comes to my mind is that
> secret_password_lookup_sync() does not implies to unlock the keyring, so if
> the keyring is locked the schema can't be retrieved and the query fails
> (looking at dbus, the array with the answer is empty). 
> 
> This is related only to libsecret, not gnome-keyring (if HAVE_LIBSECRET is
> defined)
> 
> If I change the schema flag 
> e.g.
> -    "org.gnucash.password", SECRET_SCHEMA_NONE,
> +    "org.gnucash.password", SECRET_SCHEMA_DONT_MATCH_NAME,
> 
According to this migration page [1], you're right on the bat with this patch:

"If you wish to lookup items that were stored by libgnome-keyring, you should specify the SECRET_SCHEMA_DONT_MATCH_NAME flag in the schema so that the schema name is not matched, since it was not stored by libgnome-keyring."

So for libgnome-keyring compatibility we need to look up the password with SECRET_SCHEMA_DONT_MATCH_NAME. For pure libsecret based install, we'd want the SECRET_SCHEMA_NONE flag because libsecret encourages the use of schemas.

The end user of course doesn't care about all this. She just wants here password to be retrieve without fuss.

To actually achieve that I think we need to distinguish several user situations and react to them accordingly:

1. if neither libgnome-keyring nor libsecret is available, always ask for a password. This one is the easiest case.
2. If the OS X keychain library is available, use that one and fall back to asking for a password if it wasn't stored yet.
3. if libsecret is available, what to do depends on whether or not the password was originally stored with libgnome-keyring or with libsecret. This is hard to tell just like that though. The only hint is that with libgnome-keyring the schema name is not stored while it is with libsecret. So we have to query for the password via both mechanisms and use the first one that gives a successful result. Fall back to asking for a password if none was found.
4. if libsecret is not available, but libgnome-keyring is, use that library and fall back to asking for a password if none was found.

Considering libsecret is the future and libgnome-keyring will be deprecated at some point, I think in all cases we should favour libsecret. That means

- whenever we have to store a password and libsecret is available, store it with a schema (the libsecret way).
- whenever we have to lookup a password and libsecret is availabe, first query with a schema and only fall back to querying without schema if that fails
- moreover, if a password was found that has been stored using libgnome-keyring while we do have libsecret, we should convert that password to libsecret (so save it again with schema, and drop the one without schema).

What do you think of this ?

> the query works as no schema name is involved, the keyring is unlocked and
> all works fine.
> 
> To see if this is indeed the case, I modified the source inserting, before
> password lookup, a couple of calls to open the keyring: (ok, ok, it's only a
> test, i know it's ugly)
> =====
>     secret_service = secret_service_get_sync
> (SECRET_SERVICE_LOAD_COLLECTIONS, NULL, &error);
>     collections = secret_service_get_collections(secret_service);
>     secret_service_unlock_sync(secret_service, collections, NULL, NULL,
> &error);
> 
>   libsecret_password = secret_password_lookup_sync (SECRET_SCHEMA_GNUCASH,
> ...
> =====
> 
> And all seems to work fine, of course with SECRET_SCHEMA_NONE flag. right
> now I'm patching my sources to see if got it right and fix this behaviour
> (and maybe use a gnucash collection in keyring
> ). 
> I can have completely missed the point, of course.
> 
I don't think we should be using the complex API. For our simple use case the simple API should suffice.

The core of the issue IMO is that we are querying for passwords originally stored with libgnome-keyring and expect it to have a schema name stored with it. It doesn't.

So I don't think we have to explicitly open the keychain beforehand. That should be taken care of. We should just query for the password with schema first and if that fails query again without schema. And lastly convert the password if it wasn't stored with schema.

> If you think I'm right and you are interested in a pull request, just let me
> know.
> 
> Thanks,
> 
> Fabio.

Thank you for having done the major part of the investigation.

[1] https://developer.gnome.org/libsecret/unstable/migrating-schemas.html
Comment 6 Fabio Coatti 2015-04-21 12:31:02 UTC
(In reply to Geert Janssens from comment #5)

> 
> To actually achieve that I think we need to distinguish several user
> situations and react to them accordingly:
> 
> 1. if neither libgnome-keyring nor libsecret is available, always ask for a
> password. This one is the easiest case.
> 2. If the OS X keychain library is available, use that one and fall back to
> asking for a password if it wasn't stored yet.
> 3. if libsecret is available, what to do depends on whether or not the
> password was originally stored with libgnome-keyring or with libsecret. This
> is hard to tell just like that though. The only hint is that with
> libgnome-keyring the schema name is not stored while it is with libsecret.
> So we have to query for the password via both mechanisms and use the first
> one that gives a successful result. Fall back to asking for a password if
> none was found.
> 4. if libsecret is not available, but libgnome-keyring is, use that library
> and fall back to asking for a password if none was found.
> 
> Considering libsecret is the future and libgnome-keyring will be deprecated
> at some point, I think in all cases we should favour libsecret. That means
> 
> - whenever we have to store a password and libsecret is available, store it
> with a schema (the libsecret way).
> - whenever we have to lookup a password and libsecret is availabe, first
> query with a schema and only fall back to querying without schema if that
> fails
> - moreover, if a password was found that has been stored using
> libgnome-keyring while we do have libsecret, we should convert that password
> to libsecret (so save it again with schema, and drop the one without schema).
> 
> What do you think of this ?

That was exactly my line of thought, so I wrote a couple of lines of python code using libsecret wrapper to test it. Unfortunately it turned out that a simple password_lookup wasn't enough to cause the keyring to prompt for password. I'm still puzzled about this and I'm not sure that it's not my mistake (I would be happy to use only the password lookup straight away). 
Regarding the migration issue, I also wrote a test code to have gnucash convert gnome-keyring to libsecret format exactly as you described, only to discover that I haven't had old keys in my keyring to start with :) (I erased all keyring files to be sure of this).



> 
> > the query works as no schema name is involved, the keyring is unlocked and
> > all works fine.
> > 
> > To see if this is indeed the case, I modified the source inserting, before
> > password lookup, a couple of calls to open the keyring: (ok, ok, it's only a
> > test, i know it's ugly)
> > =====
> >     secret_service = secret_service_get_sync
> > (SECRET_SERVICE_LOAD_COLLECTIONS, NULL, &error);
> >     collections = secret_service_get_collections(secret_service);
> >     secret_service_unlock_sync(secret_service, collections, NULL, NULL,
> > &error);
> > 
> >   libsecret_password = secret_password_lookup_sync (SECRET_SCHEMA_GNUCASH,
> > ...
> > =====
> > 
> > And all seems to work fine, of course with SECRET_SCHEMA_NONE flag. right
> > now I'm patching my sources to see if got it right and fix this behaviour
> > (and maybe use a gnucash collection in keyring
> > ). 
> > I can have completely missed the point, of course.
> > 
> I don't think we should be using the complex API. For our simple use case
> the simple API should suffice.


That would be great, but I'm not able to make simple API to ask for unlock password. My hope is that I'm doing something wrong, so if you are able to make  this work using only simple API I will be more than happy :)


> 
> The core of the issue IMO is that we are querying for passwords originally
> stored with libgnome-keyring and expect it to have a schema name stored with
> it. It doesn't.
> 
> So I don't think we have to explicitly open the keychain beforehand. That
> should be taken care of. We should just query for the password with schema
> first and if that fails query again without schema. And lastly convert the
> password if it wasn't stored with schema.

Indeed this is the right approach IMHO, my test code was working exactly as you described, however it didn't fixed the issue. That's why I started to think that simple lookup was not enough to prompt for password if keyring was locked, even in case of new format keystore.
Comment 7 Fabio Coatti 2015-04-22 07:36:59 UTC
To help the discussion, I made some tries to have libsecret / keyring to prompt for unlock password and the only way I found is the following:
https://github.com/cova-fe/gnucash/commit/2a373e83f386472b324238a69ad8f9abfc3d1a83

beware, it's only a quick test, don't blame me for the code, I know it's ugly :)
Comment 8 Geert Janssens 2015-04-28 09:10:07 UTC
Fabio, I took some time this weekend to look closer at what is happening here. (I first had to upgrade my pc to a distro that has a recent enough version of libsecret to test with).

I have come up with a patch that seems to fix it properly on my system:
https://github.com/gjanssens/gnucash/commits/bug746873

Feel free to test it and report back.

These were my findings while studying this issue:

1. The simple libsecret API (secret_password_lookup_sync and friends) does unlock the password store *whenever needed and only when needed*. It took me a while to figure out what that meant: libsecret will only unlock the store if the query for a password returns a (locked) password for the attributes passed to the query. This is different from gnome-keyring which would unlock the password store whenever a gnome-keyring api call was made.

2. The original gnucash code was written that if libsecret is present it would first use that API to lookup a password. If that fails it would fall back to the gnome-keyring API. Of course that could only work if libgnome-keyring is installed on the system AND gnucash being compiled with libgnome-keyring support (in addition to libsecret support). I never asked you explicitly if that was the case. If gnome-keyring support is not available that fallback would fail and you would be asked to manually enter a password instead. From my experiments this seems to be the scenario you hit.

3. I have now rewritten this code to ONLY depend on libsecret if it's available. libsecret can also query for legacy gnome-keyring passwords if the proper password schema is used. As you rightfully suggested in comment 4, the legacy schema requires a different flag. However libsecret makes it even easier than that as it defines a complete schema (SECRET_SCHEMA_COMPAT_NETWORK) to query for legacy gnome-keyring passwords.

There were two tricky bits which took me a while to realize:
a. ghome-keyring stored passwords had an additional attribute which we so far had ignored in our tests: the "object" attribute. This is the name of the database. We don't use this with libsecret anymore because it's a redundant attribute to get the password for a user on any given database server. Still I had to add this to the libsecret queries for legacy gnome-keyring passwords because the old passwords are stored with it.

b. The gnome-keyring api would not store the port attribute if port is 0. libsecret will store whatever attribute/value pair you pass to it, so also a port attribute with value 0. In our tests above we were querying for legacy gnome-keyring passwords with the port attribute set (to 0 but still set). As gnome-keyring didn't set these, the libsecret query never matched the passwords as stored via gnome-keyring. So I also had to rewrite part of the libsecret logic to only add the port attribute if its different from 0.

4. Lastly I have added some code that would do the necessary password conversions. If a legacy password is found, a copy of it is stored using gnucash' libsecret schema. If a libsecret password was found with a port attribute of 0, a copy is written without the port attribute.

I was also tempted to remove the legacy password(s) once converted in point 4. For now I have not done this yet however. A user may still revert back to an older gnucash version for some reason and these older versions may still try to query with the legacy attributes and not find the new attributes.

Can you test the commit and report your findings together with any feedback on my explanations above ?

Thanks.
Comment 9 Fabio Coatti 2015-04-28 20:52:05 UTC
(In reply to Geert Janssens from comment #8)
> Fabio, I took some time this weekend to look closer at what is happening
> here. (I first had to upgrade my pc to a distro that has a recent enough
> version of libsecret to test with).
> 
> I have come up with a patch that seems to fix it properly on my system:
> https://github.com/gjanssens/gnucash/commits/bug746873
> 
> Feel free to test it and report back.

Many thanks for having a look at this issue!

Just compiled your branch, and the behaviour indeed changed, but still not completely ok.(At least, on my system).

This is how I tested it:
1 deleted keyring files (just to avoid leftovers)
2 started gnucash, entered username+pass, got the unlock popup and password saved (I placed some debug lines to be sure of the flow). exited gnucash.
3 killall gnome-keyring-daemon (this to reproduce the usual behaviour on my system, kde based, where gnome-keyring is not started at login)
4 started gnucash, got prompt for username+pwd, that means failure to access gnome-keyring.
5 entered user+pass, prompted for unlock password
6 entered unlock password, access granted and password saved
7 exited gnucash
8 restarted gnucash, got the same behaviour from step 4.

Now, if I launch seahorse and unlock the keyring before step 8 and then I start gnucash, I get no prompts and access is granted,  the correct behaviour.

so it seems that when the keyring is locked, gnucash is unable to cause the unlock prompt to appear...sort of: if now I lock the keyring with seahorse and restart gnucash (without killing keyring daemon), I get the unlock prompt from keyring, not the user+pass prompt. Maybe the user got cached by gnome-keyring so libsecret is able to get an handle to the object and so trigger unlock prompt?

the way to reproduce this behaviour is to kill -HUP gnome-keyring-daemon. The fact that keyring daemon is not running at first request should not cause issues, it is launched on request. (and with my test patch reported above, the behaviour is correct also when gnome-keyring-daemon is not running before starting gnucash).



> 
> These were my findings while studying this issue:
> 
> 1. The simple libsecret API (secret_password_lookup_sync and friends) does
> unlock the password store *whenever needed and only when needed*. It took me
> a while to figure out what that meant: libsecret will only unlock the store
> if the query for a password returns a (locked) password for the attributes
> passed to the query. This is different from gnome-keyring which would unlock
> the password store whenever a gnome-keyring api call was made.
> 
> 2. The original gnucash code was written that if libsecret is present it
> would first use that API to lookup a password. If that fails it would fall
> back to the gnome-keyring API. Of course that could only work if
> libgnome-keyring is installed on the system AND gnucash being compiled with
> libgnome-keyring support (in addition to libsecret support). I never asked
> you explicitly if that was the case. If gnome-keyring support is not
> available that fallback would fail and you would be asked to manually enter
> a password instead. From my experiments this seems to be the scenario you
> hit.
> 
> 3. I have now rewritten this code to ONLY depend on libsecret if it's
> available. libsecret can also query for legacy gnome-keyring passwords if
> the proper password schema is used. As you rightfully suggested in comment
> 4, the legacy schema requires a different flag. However libsecret makes it
> even easier than that as it defines a complete schema
> (SECRET_SCHEMA_COMPAT_NETWORK) to query for legacy gnome-keyring passwords.
> 
> There were two tricky bits which took me a while to realize:
> a. ghome-keyring stored passwords had an additional attribute which we so
> far had ignored in our tests: the "object" attribute. This is the name of
> the database. We don't use this with libsecret anymore because it's a
> redundant attribute to get the password for a user on any given database
> server. Still I had to add this to the libsecret queries for legacy
> gnome-keyring passwords because the old passwords are stored with it.
> 
> b. The gnome-keyring api would not store the port attribute if port is 0.
> libsecret will store whatever attribute/value pair you pass to it, so also a
> port attribute with value 0. In our tests above we were querying for legacy
> gnome-keyring passwords with the port attribute set (to 0 but still set). As
> gnome-keyring didn't set these, the libsecret query never matched the
> passwords as stored via gnome-keyring. So I also had to rewrite part of the
> libsecret logic to only add the port attribute if its different from 0.
> 
> 4. Lastly I have added some code that would do the necessary password
> conversions. If a legacy password is found, a copy of it is stored using
> gnucash' libsecret schema. If a libsecret password was found with a port
> attribute of 0, a copy is written without the port attribute.
> 
> I was also tempted to remove the legacy password(s) once converted in point
> 4. For now I have not done this yet however. A user may still revert back to
> an older gnucash version for some reason and these older versions may still
> try to query with the legacy attributes and not find the new attributes.
> 
> Can you test the commit and report your findings together with any feedback
> on my explanations above ?

The explanation seemed quite right to me, but the actual behaviour is quite different from the expected one. Maybe I'm doing something wrong...but working with complex API shows the right behaviour, so maybe it is possible to get it working also with simple ones.

Many thanks!
Comment 10 Geert Janssens 2015-04-29 08:42:42 UTC
Thanks for the elaborate testing. That last bit - killing gnome-keyring-daemon - was something I hadn't tried before. I can reproduce this here as well.

Note that I'm using gnucash in exactly the same way - from a kde session.

I'm beginning to suspect a bug in libsecret rather than in gnucash here.

Running this scenario through gdb, I see that the calls to secret_password_lookup_sync do launch the gnome-keyring-daemon but don't trigger the dialog to unlock the keyring. Only the call to secret_password_store_sync does the latter. This makes no sense to me. I'll ask around in gnome circles for some feedback on this.
Comment 11 Geert Janssens 2015-04-29 10:33:37 UTC
FYI, I have filed a bug against libsecret [1] with a simplified test case.

Let's see what the authors have to say about this.

[1] https://bugzilla.gnome.org/show_bug.cgi?id=748625
Comment 12 Geert Janssens 2015-05-08 19:48:42 UTC
Meanwhile I have pushed a simple workaround.

My tests showed that secret_password_store_sync does unlock the secret store when it's still locked.
So to keep the code relatively simple I have chosen to write a dummy password and remove it again whenever an attempt is made to look up a password in the secret store. This ensures that the secret store is unlocked the moment the lookup function is called.

Once libsecret fixes its bug, this workaround can be reverted again.
Comment 13 Geert Janssens 2017-09-22 10:59:27 UTC
At it looks like the maintainers of libsecret don't consider this priority, I'll just take my workaround as the final solution. Bug closed.
Comment 14 John Ralls 2018-06-29 23:39:50 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=746873. Please update any external references or bookmarks.