GNOME Bugzilla – Bug 679884
libsecret migration
Last modified: 2012-08-03 13:54:53 UTC
Created attachment 218747 [details] [review] WIP patch to migrate to libsecret libsecret is a new client for the Secret Service DBus API. The Secret Service allows storage of passwords in a common way on the desktop. Supported by gnome-keyring and ksecretservice. libsecret solves many problems with libgnome-keyring. Relevant to empathy: it solves threading issues, uses GDBus instead of dbus-glib, and makes cancellation cleaner. A future GNOME goal will be to migrate away from libgnome-keyring to libsecret: https://live.gnome.org/GnomeGoals/LibsecretMigration I've done a rough WIP patch in order to make sure that the libsecret API covered all the use cases. I'll attach that patch here. I hope the patch is a help for the migration, but I don't plan to iterate on it at the current time. Some notes about the patch: * gnome-keyring-daemon is autostarted, so I've g_vfs_keyring_is_available() is now hard coded. * I chose an arbitrary schema name for the stored items, you may want to change it to something better. It would be good if telepathy and other code that looks up these passwords uses the the same schema and name. org.freedesktop.Telepathy.MissionControl org.gnome.Empathy.Room * I haven't tested or built this patch due to broken dependencies (folks breaks on current eds). * secret_password_remove() removes all unlocked matching passwords as opposed to gnome_keyring_find_items() + gnome_keyring_item_delete() which removed all passwords regardless whether they were locked or not, and prompted the user to unlock. If you want, you can use secret_service_search() and secret_item_delete() to effect the old behavior. Note that the patch uses the unstable 'advanced' parts of the libsecret API. In particular, it's not "best practice" to use gnome-keyring as a place to store account or connection details. In an ideal world gnome-keyring would be used just to store secrets, and attributes are used to lookup those secrets. However libsecret supports this use case, but that functionality is not yet absolutely stable. I'm aiming to get most of this stable by GNOME 3.8, but if you do migrate to libsecret before then, I would patch empathy for any API changes that come up.
Per r-t: Targetting GNOME 3.6
Created attachment 218795 [details] [review] WIP patch to migrate to libsecret Updated patch for last minute API change. The above comment should say: * secret_password_clear() removes all unlocked matching passwords as opposed to gnome_keyring_find_items() + gnome_keyring_item_delete() which removed all passwords regardless whether they were locked or not, and prompted the user to unlock. If you want, you can use secret_service_search() and secret_item_delete() to effect the old behavior.
Just a heads up: Please look at the patch critically. I did the patch as a way to try out the API, and it's not ready to commit. There may be memory leaks or other logic errors. Most libsecret getters return data that must be unreferenced or freed.
Review of attachment 218795 [details] [review]: Thanks a lot for the patch. I did some minor changes in order to be able to build and test it. I tried connecting an account not having a stored password and hit this warning: ** (empathy-auth-client:28701): WARNING **: secret_password_lookupv: must specify at least one attribute to match Program received signal SIGTRAP, Trace/breakpoint trap. 0x00007ffff4e1f1e9 in g_logv (log_domain=0x0, log_level=G_LOG_LEVEL_WARNING, format=0x7ffff76d38f0 "%s: must specify at least one attribute to match", args1=0x7fffffffdbc8) at gmessages.c:758 758 G_BREAKPOINT (); (gdb) bt
+ Trace 230510
::: configure.ac @@ +196,3 @@ gio-2.0 >= $GLIB_REQUIRED gio-unix-2.0 >= $GLIB_REQUIRED + secret-1 >= $LIBSECRET_REQUIRED I installed libsecret using jhbuild and it provides "libsecret-1" not "secret-1". Is that the new name?
Created attachment 218899 [details] [review] WIP: Migrate from libgnome-keyring to libsecret * See: https://live.gnome.org/GnomeGoals/LibsecretMigration
(In reply to comment #0) > * I chose an arbitrary schema name for the stored items, you may want to > change it to something better. It would be good if telepathy and other > code that looks up these passwords uses the the same schema and name. > org.freedesktop.Telepathy.MissionControl > org.gnome.Empathy.Room I sent a mail on the Telepathy mailing list to discuss this: http://lists.freedesktop.org/archives/telepathy/2012-July/006191.html
(In reply to comment #4) > Review of attachment 218795 [details] [review]: > > Thanks a lot for the patch. I did some minor changes in order to be able to > build and test it. > > > I tried connecting an account not having a stored password and hit this > warning: > > ** (empathy-auth-client:28701): WARNING **: secret_password_lookupv: must > specify at least one attribute to match This is my screwup. There's an extra NULL in the call to secret_password_lookup() on line 107 of empathy-keyring.c. And so the function call things that no attributes have been specified. Varargs functions are a mixed bag :) > + secret-1 >= $LIBSECRET_REQUIRED > > I installed libsecret using jhbuild and it provides "libsecret-1" not > "secret-1". Is that the new name? It got renamed to libsecret-1 in the last few days. Like I said, I haven't built this empathy patch, due to dependency problems. > I sent a mail on the Telepathy mailing list to discuss this: > http://lists.freedesktop.org/archives/telepathy/2012-July/006191.html Cool.
Created attachment 219000 [details] [review] Migrate from libgnome-keyring to libsecret * See: https://live.gnome.org/GnomeGoals/LibsecretMigration
Here is my updated version of the patch which seems to work fine. The only 'blocker' atm is the name/schema of the secret keys. Once that's sorted out I'll merge the patch.
We really want this and the storage won't be shared with KDE anyway so I'm going to merge this. Furtermore, the plan seems to rely more on the SSO frameworks to store such things so maybe having the same schema isn't that much important. We can always change later, this patch doesn't introduce any migration path anyway.
Attachment 219000 [details] pushed as 3de9dd6 - Migrate from libgnome-keyring to libsecret