GNOME Bugzilla – Bug 679849
libsecret migration
Last modified: 2013-02-18 20:33:47 UTC
Created attachment 218710 [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. It solves many problems with libgnome-keyring. Relevant to NetworkManager: it solves threading issues, uses GDBus instead of dbus-glib, and makes cancellation much cleaner. Note that libsecret can read passwords stored via libgnome-keyring. 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 for network-manager-applet in order to make sure that the libsecret API covered all the use cases. I'll attach that patch here. I hope it's a help for the migration, but I don't plan to iterate on it at the current time. Some notes about the patch: * I chose arbitrary schema names. You probably want to choose better ones and probably want to sync these schema names with other libsecret based nm-agents like gnome-shell: org.freedesktop.NetworkManager.GSM org.freedesktop.NetworkManager.Connection * I haven't tested the patch. That's hard for me to do without the devices involved, and I'm not familiar with all the code paths and logic. * I didn't patch the stuff in the gconf-helpers directory.
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 further 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 network-manager-applet for any API changes that come up.
Per r-t: Targetting GNOME 3.6
(In reply to comment #1) > 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 further 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 network-manager-applet for any API changes > that come up. What bits do you consider "additional details"? All the attributes nm-applet stores are necessary for looking up the secret, as you may (for example) have a PIN stored in the 'GSM' setting but *also* have a PPP password for the 'ppp' setting. Both are part of the same connection (hence the usage of the UUID) but both are used at different times (hence storage of the setting name). For the OOB GSM PIN stuff, we also need both the devid and the simid, since not all SIM cards allow querying the SIM ID when a PIN is required (chicken-in-egg...) and thus we have to fall back to the device ID.
(In reply to comment #3) > What bits do you consider "additional details"? All the attributes nm-applet > stores are necessary for looking up the secret, as you may (for example) have a > PIN stored in the 'GSM' setting but *also* have a PPP password for the 'ppp' > setting. Both are part of the same connection (hence the usage of the UUID) > but both are used at different times (hence storage of the setting name). I see, so in this case instead of doing two lookups network-manager is doing one. Makes sense. > For the OOB GSM PIN stuff, we also need both the devid and the simid, since not > all SIM cards allow querying the SIM ID when a PIN is required > (chicken-in-egg...) and thus we have to fall back to the device ID. Makes sense again. That's why network-manager can't just just match on both the simid and deviceid. Doing things this way just requires a few more lines of code, and use of the unstable api. But no problem at all.
Created attachment 218790 [details] [review] WIP patch to migrate to libsecret Updated for a last minute API change.
BTW, just a heads up: Please look at the patch critically. I really did the patch as a way to try out the API. There may be memory leaks or other logic errors. Most libsecret getters return data that must be unreferenced or freed.
Created attachment 222158 [details] [review] applet: Migrate from libgnome-keyring to libsecret ==== Updated patch. Things are somewhat simplified by dcbw's recent changes to move the keyring stuff from utils/ into applet-agent.c directly. In terms of memory management, the only thing I noticed was that the original patch wasn't unreffing the hash table returned from secret_item_get_attributes() in a few places. This still leaves the migration tool using gnome-keyring instead of libsecret, but it depends on gconf too, so if you're building that, you're already pulling in legacy stuff. Very minimally tested, but seems to work. Dan: we'll still need gnome-keyring support for RHEL6, but I'm assuming it will be nicer to just patch that back in in the RPM rather than leaving the code in here and having a compile-time switch between the two codepaths, right?
Created attachment 222159 [details] [review] applet: Migrate from libgnome-keyring to libsecret The optional migration tool still uses libgnome-keyring. Based on a patch from Stef Walter.
Created attachment 222160 [details] [review] applet: Migrate from libgnome-keyring to libsecret oops, had unsaved changes in an emacs buffer...
Moving off the 3.6 blocker list
Would be nice to get this done for 3.8
would still be nice to get this merged for 3.8
Pushed to master. dcbw and I agreed that testingwise, it's too late for this to get into 0.9.8 (on Wednesday), but we can put out a 0.9.8.2 to coincide with GNOME 3.8. Attachment 222160 [details] pushed as 37cc714 - applet: Migrate from libgnome-keyring to libsecret