From 7b7660af6023683d843d7f106e24446c409a448b Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Thu, 16 Nov 2017 15:01:11 +0100 Subject: GNOME: replace gnome-keyring with libsecret (FDO #104219) The GNOME keyring library has been obsoleted for a long time now, long enough that the replacement libsecret is available on all supported distros. Therefore we can switch unconditionally. Signed-off-by: Patrick Ohly --- src/backends/gnome/GNOMEPlatform.cpp | 170 ++++++++++++++--------------------- src/backends/gnome/configure-sub.in | 9 +- 2 files changed, 73 insertions(+), 106 deletions(-) diff --git a/src/backends/gnome/GNOMEPlatform.cpp b/src/backends/gnome/GNOMEPlatform.cpp index f2f7a1d3..37f9811a 100644 --- a/src/backends/gnome/GNOMEPlatform.cpp +++ b/src/backends/gnome/GNOMEPlatform.cpp @@ -22,7 +22,7 @@ #ifdef USE_GNOME_KEYRING extern "C" { -#include +#include } #include "GNOMEPlatform.h" @@ -30,46 +30,11 @@ extern "C" { #include #include #include +#include #include SE_BEGIN_CXX -// Occasionally, libgnome-keyring fails with the following error messages: -// Gkr: received an invalid, unencryptable, or non-utf8 secret -// Gkr: call to daemon returned an invalid response: (null).(null) -// -// We work around that by retrying the operation a few times, for at -// most this period of time. Didn't really help, so disable it for now -// by using a zero duration. -static const double GNOMEKeyringRetryDuration = 2; // seconds -static const double GNOMEKeyringRetryInterval = 0.1; // seconds - -/** - * libgnome-keyring has an internal gkr_reset_session() - * method which gets called when the "org.freedesktop.secrets" - * disconnects from the D-Bus session bus. - * - * We cannot call that method directly, but we can get it called by - * faking the "disconnect" signal. That works because - * on_connection_filter() in gkr-operation.c doesn't check who the - * sender of the signal is. - * - * Once gkr_reset_session() got called, the next operation will - * re-establish the connection. After the failure above, the second - * attempt usually works. - * - * Any other client using libgnome-keyring will also be tricked into - * disconnecting temporarily. That should be fine, any running - * operation will continue to run and complete (?). - */ -static void FlushGNOMEKeyring() -{ - // Invoking dbus-send is easier than writing this in C++. - // Besides, it ensures that the signal comes from some other - // process. Not sure whether signals are sent back to the sender. - system("dbus-send --session --type=signal /org/freedesktop/DBus org.freedesktop.DBus.NameOwnerChanged string:'org.freedesktop.secrets' string:':9.99' string:''"); -} - /** * GNOME keyring distinguishes between empty and unset * password keys. This function returns NULL for an @@ -97,6 +62,39 @@ static bool UseGNOMEKeyring(const InitStateTri &keyring) return true; } +class LibSecretHash : public GHashTableCXX +{ + std::list m_buffer; + +public: + LibSecretHash(const ConfigPasswordKey &key) : + GHashTableCXX(g_hash_table_new(g_str_hash, g_str_equal), TRANSFER_REF) + { + // see https://developer.gnome.org/libsecret/0.16/libsecret-SecretSchema.html#SECRET-SCHEMA-COMPAT-NETWORK:CAPS + insert("user", key.user); + insert("domain", key.domain); + insert("server", key.server); + insert("object", key.object); + insert("protocol", key.protocol); + insert("authtype", key.authtype); + if (key.port) { + std::string value = StringPrintf("%d", key.port); + insert("port", value); + } + } + + /** Keys are expected to be constants and not copied. Values are copied. */ + void insert(const char *key, const std::string &value) + { + if (!value.empty()) { + m_buffer.push_back(value); + g_hash_table_insert(get(), + const_cast(key), + const_cast(m_buffer.back().c_str())); + } + } +}; + bool GNOMELoadPasswordSlot(const InitStateTri &keyring, const std::string &passwordName, const std::string &descr, @@ -108,46 +106,24 @@ bool GNOMELoadPasswordSlot(const InitStateTri &keyring, return false; } - GnomeKeyringResult result = GNOME_KEYRING_RESULT_OK; - GList* list; - Timespec start = Timespec::monotonic(); - double sleepSecs = 0; - do { - if (sleepSecs != 0) { - SE_LOG_DEBUG(NULL, "%s: previous attempt to load password '%s' from GNOME keyring failed, will try again: %s", - key.description.c_str(), - key.toString().c_str(), - gnome_keyring_result_to_message(result)); - FlushGNOMEKeyring(); - Sleep(sleepSecs); - } - result = gnome_keyring_find_network_password_sync(passwdStr(key.user), - passwdStr(key.domain), - passwdStr(key.server), - passwdStr(key.object), - passwdStr(key.protocol), - passwdStr(key.authtype), - key.port, - &list); - sleepSecs = GNOMEKeyringRetryInterval; - } while (result != GNOME_KEYRING_RESULT_OK && - (Timespec::monotonic() - start).duration() < GNOMEKeyringRetryDuration); + GErrorCXX gerror; + LibSecretHash hash(key); + PlainGStr result(secret_password_lookupv_sync(SECRET_SCHEMA_COMPAT_NETWORK, + hash, + NULL, + gerror)); // if find password stored in gnome keyring - if(result == GNOME_KEYRING_RESULT_OK && list && list->data ) { - GnomeKeyringNetworkPasswordData *key_data; - key_data = (GnomeKeyringNetworkPasswordData*)list->data; - password = std::string(key_data->password); - gnome_keyring_network_password_list_free(list); + if (gerror) { + gerror.throwError(SE_HERE, StringPrintf("looking up password '%s'", descr.c_str())); + } else if (result) { SE_LOG_DEBUG(NULL, "%s: loaded password from GNOME keyring using %s", key.description.c_str(), key.toString().c_str()); + password = result; } else { - SE_LOG_DEBUG(NULL, "password not in GNOME keyring using %s: %s", - key.toString().c_str(), - result == GNOME_KEYRING_RESULT_NO_MATCH ? "no match" : - result != GNOME_KEYRING_RESULT_OK ? gnome_keyring_result_to_message(result) : - "empty result list"); + SE_LOG_DEBUG(NULL, "password not in GNOME keyring using %s", + key.toString().c_str()); } return true; @@ -173,38 +149,26 @@ bool GNOMESavePasswordSlot(const InitStateTri &keyring, key.toString().c_str())); } - guint32 itemId; - GnomeKeyringResult result = GNOME_KEYRING_RESULT_OK; - // write password to keyring - Timespec start = Timespec::monotonic(); - double sleepSecs = 0; - do { - if (sleepSecs != 0) { - SE_LOG_DEBUG(NULL, "%s: previous attempt to save password '%s' in GNOME keyring failed, will try again: %s", - key.description.c_str(), - key.toString().c_str(), - gnome_keyring_result_to_message(result)); - FlushGNOMEKeyring(); - Sleep(sleepSecs); - } - result = gnome_keyring_set_network_password_sync(NULL, - passwdStr(key.user), - passwdStr(key.domain), - passwdStr(key.server), - passwdStr(key.object), - passwdStr(key.protocol), - passwdStr(key.authtype), - key.port, - password.c_str(), - &itemId); - sleepSecs = GNOMEKeyringRetryInterval; - } while (result != GNOME_KEYRING_RESULT_OK && - (Timespec::monotonic() - start).duration() < GNOMEKeyringRetryDuration); - if (result != GNOME_KEYRING_RESULT_OK) { - Exception::throwError(SE_HERE, StringPrintf("%s: saving password '%s' in GNOME keyring failed: %s", - key.description.c_str(), - key.toString().c_str(), - gnome_keyring_result_to_message(result))); + GErrorCXX gerror; + LibSecretHash hash(key); + std::string label; + if (!key.user.empty() && !key.server.empty()) { + // This emulates the behavior of libgnomekeyring. + label = key.user + "@" + key.server; + } else { + label = passwordName; + } + gboolean result = secret_password_storev_sync(SECRET_SCHEMA_COMPAT_NETWORK, + hash, + NULL, + label.c_str(), + password.c_str(), + NULL, + gerror); + if (!result) { + gerror.throwError(SE_HERE, StringPrintf("%s: saving password '%s' in GNOME keyring", + key.description.c_str(), + key.toString().c_str())); } SE_LOG_DEBUG(NULL, "saved password in GNOME keyring using %s", key.toString().c_str()); diff --git a/src/backends/gnome/configure-sub.in b/src/backends/gnome/configure-sub.in index 199989e8..d02d52a6 100644 --- a/src/backends/gnome/configure-sub.in +++ b/src/backends/gnome/configure-sub.in @@ -1,10 +1,13 @@ -PKG_CHECK_MODULES(KEYRING, [gnome-keyring-1 >= 2.20], HAVE_KEYRING=yes, HAVE_KEYRING=no) +# According to https://developer.gnome.org/libsecret/0.16/libsecret-Password-storage.html#secret-password-store-sync +# the simple API was still considered unstable. All supported distros +# now have 0.18 where the API is stable. +PKG_CHECK_MODULES(KEYRING, [libsecret-1 >= 0.18], HAVE_KEYRING=yes, HAVE_KEYRING=no) AC_ARG_ENABLE(gnome-keyring, AS_HELP_STRING([--enable-gnome-keyring], - [enables or disables support for the GNOME keyring; default is on if development files are available]), + [enables or disables support for the GNOME keyring via libsecret; default is on if development files are available]), [enable_gnome_keyring="$enableval" test "$enable_gnome_keyring" = "yes" || test "$enable_gnome_keyring" = "no" || AC_MSG_ERROR([invalid value for --enable-gnome-keyring: $enable_gnome_keyring]) - test "$enable_gnome_keyring" = "no" || test "$HAVE_KEYRING" = "yes" || AC_MSG_ERROR([gnome-keyring-1 pkg >= 2.20 not found, needed for --enable-gnome-keyring])], + test "$enable_gnome_keyring" = "no" || test "$HAVE_KEYRING" = "yes" || AC_MSG_ERROR([libsecret-1 >= 0.18 not found, needed for --enable-gnome-keyring])], enable_gnome_keyring="$HAVE_KEYRING") if test $enable_gnome_keyring = "yes"; then have_keyring=yes -- cgit v1.2.3