[vlc-devel] [PATCH 3/4] keystore: add libsecret keystore module
Thomas Guillem
thomas at gllm.fr
Thu Nov 26 13:49:39 CET 2015
On Thu, Nov 26, 2015, at 11:47, Thomas Guillem wrote:
>
>
> On Wed, Nov 25, 2015, at 20:22, Rémi Denis-Courmont wrote:
> > On Wednesday 25 November 2015 19:14:27 Thomas Guillem wrote:
> > > ---
> > > configure.ac | 5 +
> > > modules/keystore/Makefile.am | 11 ++
> > > modules/keystore/secret.c | 267
> > > +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 283
> > > insertions(+)
> > > create mode 100644 modules/keystore/secret.c
> > >
> > > diff --git a/configure.ac b/configure.ac
> > > index 2a45941..5e776de 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -4067,6 +4067,11 @@ AS_IF([test "${enable_taglib}" != "no"], [
> > > ])
> > >
> > > dnl
> > > +dnl libsecret
> > > +dnl
> > > +PKG_ENABLE_MODULES_VLC([SECRET], [], [libsecret-1], [libsecret], [auto])
> >
> > Sigh. Why?
> >
> > How is gcrypt usage serialized with the rest of VLC and other libraries?
>
> gcrypt is not used by libsecret, it may be used by the daemon (but we
> don't link with it).
>
> > Do we really want to depend on Gobject? (this is a rhetorical question)
>
> I don't see any problems, only this (non mandatory) module will depend
> on Gobject.
>
> >
> > > +
> > > +dnl
> > > dnl Developers helper modules (should be hidden from configure help)
> > > dnl
> > > AC_ARG_ENABLE(devtools, [], [], [enable_devtools="no"])
> > > diff --git a/modules/keystore/Makefile.am b/modules/keystore/Makefile.am
> > > index 34cbf00..dc8c526 100644
> > > --- a/modules/keystore/Makefile.am
> > > +++ b/modules/keystore/Makefile.am
> > > @@ -2,3 +2,14 @@ keystoredir = $(pluginsdir)/keystore
> > >
> > > libplaintext_keystore_plugin_la_SOURCES = keystore/plaintext.c
> > > keystore_LTLIBRARIES = libplaintext_keystore_plugin.la
> > > +
> > > +libsecret_plugin_la_SOURCES = keystore/secret.c
> > > +libsecret_plugin_la_CPPFLAGS = $(AM_CPPFLAGS) $(SECRET_CFLAGS)
> > > +libsecret_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(keystoredir)'
> > > +libsecret_plugin_la_LIBADD = $(SECRET_LIBS)
> > > +
> > > +keystore_LTLIBRARIES += \
> > > + $(LTLIBsecret)
> > > +
> > > +EXTRA_LTLIBRARIES += \
> > > + libsecret_plugin.la
> > > diff --git a/modules/keystore/secret.c b/modules/keystore/secret.c
> > > new file mode 100644
> > > index 0000000..4d1f08f
> > > --- /dev/null
> > > +++ b/modules/keystore/secret.c
> > > @@ -0,0 +1,267 @@
> > > +/**************************************************************************
> > > *** + * secret.c: libsecret keystore module
> > > +
> > > ***************************************************************************
> > > ** + * Copyright © 2015 VLC authors, VideoLAN and VideoLabs
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU Lesser General Public License as published
> > > by + * the Free Software Foundation; either version 2.1 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU Lesser General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU Lesser General Public License
> > > + * along with this program; if not, write to the Free Software Foundation,
> > > + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA. +
> > > ***************************************************************************
> > > **/ +
> > > +#ifdef HAVE_CONFIG_H
> > > +# include "config.h"
> > > +#endif
> > > +
> > > +#include <vlc_common.h>
> > > +#include <vlc_plugin.h>
> > > +#include <vlc_keystore.h>
> > > +
> > > +#include <libsecret/secret.h>
> > > +
> > > +#define VLC_KEYRING "VLC"
> > > +
> > > +static int Open(vlc_object_t *);
> > > +static void Close(vlc_object_t *);
> > > +
> > > +vlc_module_begin()
> > > + set_shortname(N_("libsecret keystore"))
> > > + set_description(N_("secrets are stored via libsecret"))
> > > + set_category(CAT_ADVANCED)
> > > + set_subcategory(SUBCAT_ADVANCED_MISC)
> > > + set_capability("keystore", 100)
> > > + set_callbacks(Open, Close)
> > > +vlc_module_end ()
> > > +
> > > +struct vlc_keystore_secret
> > > +{
> > > + SecretItem *p_item;
> > > + SecretValue *p_value;
> > > +};
> > > +
> > > +static void
> > > +g_hash_key_destroy_cb(gpointer data)
> > > +{
> > > + free(data);
> > > +}
> > > +
> > > +static GHashTable *
> > > +Dict2GHashTable(const vlc_dictionary_t *p_dict)
> > > +{
> > > + GHashTable *p_hash = g_hash_table_new_full(g_str_hash, g_str_equal,
> > > + g_hash_key_destroy_cb,
> > > NULL); + if (!p_hash)
> > > + return NULL;
> > > + char **ppsz_keys = vlc_dictionary_all_keys(p_dict);
> > > +
> > > + if (!ppsz_keys)
> > > + {
> > > + g_hash_table_unref(p_hash);
> > > + return NULL;
> > > + }
> > > + for (char **ppsz_it = ppsz_keys; *ppsz_it; ppsz_it++)
> > > + {
> > > + char *psz_key = *ppsz_it;
> > > + const char *psz_value = vlc_dictionary_value_for_key(p_dict,
> > > psz_key); + if (psz_value)
> > > + g_hash_table_insert(p_hash, (gpointer) psz_key, (gpointer)
> > > psz_value); + }
> > > + free(ppsz_keys);
> > > + return p_hash;
> > > +}
> > > +
> > > +static void
> > > +GHash2DictPair(gpointer key, gpointer value, gpointer user_data)
> > > +{
> > > + vlc_dictionary_t *p_dict = user_data;
> > > +
> > > + if (!p_dict->i_size)
> > > + return;
> > > + const char *psz_key = key;
> > > + char *psz_value = strdup((const char *)value);
> > > +
> > > + if (!psz_value)
> > > + vlc_keystore_clear_dict(p_dict);
> > > + else
> > > + vlc_dictionary_insert(p_dict, psz_key, psz_value);
> > > +}
> > > +
> > > +static int
> > > +GHash2Dict(GHashTable *g_hash, vlc_dictionary_t *p_dict)
> > > +{
> > > + vlc_dictionary_init(p_dict, g_hash_table_size(g_hash));
> > > + if (!p_dict->i_size)
> > > + return VLC_EGENERIC;
> > > +
> > > + g_hash_table_foreach(g_hash, GHash2DictPair, p_dict);
> > > +
> > > + return p_dict->i_size ? VLC_SUCCESS : VLC_EGENERIC;
> > > +}
> > > +
> > > +static int
> > > +Store(vlc_keystore *p_keystore, const vlc_dictionary_t *p_dict,
> > > + const char *psz_secret, const char *psz_label)
> > > +{
> > > + SecretService *p_ss = (SecretService *) p_keystore->p_sys;
> > > + GHashTable *p_hash = Dict2GHashTable(p_dict);
> > > + msg_Err(p_keystore, "Store: '%p' : '%s'", p_hash, psz_secret);
> > > + if (!p_hash)
> > > + return VLC_EGENERIC;
> > > +
> > > + SecretValue *p_sv = secret_value_new(psz_secret, -1, "text/plain");
> > > + if (!p_sv)
> > > + {
> > > + g_hash_table_unref(p_hash);
> > > + return VLC_EGENERIC;
> > > + }
> > > + gboolean b_ret = secret_service_store_sync(p_ss, NULL, p_hash,
> > > + SECRET_COLLECTION_DEFAULT,
> > > + psz_label, p_sv, NULL,
> > > NULL); + secret_value_unref(p_sv);
> > > + g_hash_table_unref(p_hash);
> > > + return b_ret ? VLC_SUCCESS : VLC_EGENERIC;
> > > +}
> > > +
> > > +static GList*
> > > +SearchItems(SecretService *p_ss, const vlc_dictionary_t *p_dict)
> > > +{
> > > + GHashTable *p_hash = Dict2GHashTable(p_dict);
> > > + if (!p_hash)
> > > + return 0;
> > > +
> > > + GList *p_list = secret_service_search_sync(p_ss, NULL, p_hash,
> > > + SECRET_SEARCH_ALL, NULL,
> > > NULL); + g_hash_table_unref(p_hash);
> > > + return p_list;
> > > +}
> > > +
> > > +static unsigned int
> > > +Find(vlc_keystore *p_keystore, const vlc_dictionary_t *p_dict,
> > > + vlc_keystore_entry **pp_entries)
> > > +{
> > > + SecretService *p_ss = (SecretService *) p_keystore->p_sys;
> > > +
> > > + GList *p_list = SearchItems(p_ss, p_dict);
> > > + if (!p_list)
> > > + return 0;
> > > +
> > > + unsigned int i_found_count = g_list_length(p_list);
> > > + unsigned int i_entry_count = 0;
> > > + vlc_keystore_entry *p_entries = malloc(i_found_count
> > > + * sizeof(vlc_keystore_entry));
> > > + if (!p_entries)
> > > + goto error;
> > > +
> > > + for (GList *l = p_list; l != NULL; l = l->next)
> > > + {
> > > + SecretItem *p_item = (SecretItem *) l->data;
> > > + GHashTable *p_attrs = secret_item_get_attributes(p_item);
> > > +
> > > + vlc_keystore_entry *p_entry = &p_entries[i_entry_count++];
> > > + if (GHash2Dict(p_attrs, &p_entry->dict))
> > > + goto error;
> > > + p_entry->p_secret = malloc(sizeof(vlc_keystore_secret));
> > > + if (!p_entry->p_secret)
> > > + goto error;
> > > + p_entry->p_secret->p_item = p_item;
> > > + g_object_ref(p_entry->p_secret->p_item);
> > > + p_entry->p_secret->p_value = NULL;
> > > + }
> > > + g_list_free_full(p_list, g_object_unref);
> > > + *pp_entries = p_entries;
> > > + return i_entry_count;
> > > +
> > > +error:
> > > + g_list_free_full(p_list, g_object_unref);
> > > + if (i_entry_count > 0)
> > > + vlc_keystore_release_entries(p_keystore, p_entries, i_entry_count);
> > > + return 0;
> > > +}
> > > +
> > > +
> > > +static unsigned int
> > > +Remove(vlc_keystore *p_keystore, const vlc_dictionary_t *p_dict)
> > > +{
> > > + SecretService *p_ss = (SecretService *) p_keystore->p_sys;
> > > +
> > > + GList *p_list = SearchItems(p_ss, p_dict);
> > > + if (!p_list)
> > > + return 0;
> > > +
> > > + unsigned int i_entry_count = 0;
> > > + for (GList *l = p_list; l != NULL; l = l->next)
> > > + {
> > > + SecretItem *p_item = (SecretItem *) l->data;
> > > + secret_item_delete(p_item, NULL, NULL, NULL);
> > > + i_entry_count++;
> > > + }
> > > + g_list_free_full(p_list, g_object_unref);
> > > + return i_entry_count;
> > > +}
> > > +
> > > +static const char *
> > > +Secret_load(vlc_keystore *p_keystore, vlc_keystore_secret *p_secret)
> > > +{
> > > + if (!p_secret->p_value)
> > > + {
> > > + SecretService *p_ss = (SecretService *) p_keystore->p_sys;
> > > + GList *p_list = NULL;
> > > + p_list = g_list_append(p_list, p_secret->p_item);
> > > + if (!p_list)
> > > + return NULL;
> > > + if (secret_service_unlock_sync(p_ss, p_list, NULL, NULL, NULL) ==
> > > 0) + return NULL;
> > > + if (secret_item_load_secret_sync(p_secret->p_item, NULL, NULL))
> > > + p_secret->p_value = secret_item_get_secret(p_secret->p_item);
> > > + g_list_free(p_list);
> > > + }
> > > +
> > > + return p_secret->p_value ? secret_value_get_text(p_secret->p_value) :
> > > NULL; +}
> > > +
> > > +static void
> > > +Secret_release(vlc_keystore *p_keystore, vlc_keystore_secret *p_secret)
> > > +{
> > > + (void) p_keystore;
> > > + g_object_unref(p_secret->p_item);
> > > + if (p_secret->p_value)
> > > + secret_value_unref(p_secret->p_value);
> > > + free(p_secret);
> > > +}
> > > +
> > > +static int
> > > +Open(vlc_object_t *p_this)
> > > +{
> > > + SecretService *p_ss = secret_service_get_sync(SECRET_SERVICE_NONE,
> > > + NULL, NULL);
> >
> > "This method may block indefinitely and should not be used in user
> > interface
> > threads." <-- does not look good
>
> This won't be used by an user interface thread.
This module is used from the input thread that as an interruptible
context, right ?
If I can use and expose vlc_threadvar_get(vlc_interrupt_var),
vlc_interrupt_prepare, and vlc_interrupt_finish in vlc_interrupt.h, I
could set up a callback that cancel all these sync functions (via a
Cancellable that can be passed to all sync functions).
>
> >
> > Also does this fails properly and quickly on non-GNOME systems.
>
> Of course, this module has to fail instantaneously is there is no
> gnome-keyring-daemon running. It's not really the case for now:
> secret_service_get_sync will spawn a new daemon if there isn't one. I
> don't like this behavior, I need to fix it.
>
> >
> > > + if (!p_ss)
> > > + return VLC_EGENERIC;
> > > +
> > > + vlc_keystore *p_keystore = (vlc_keystore *)p_this;
> > > +
> > > + p_keystore->p_sys = (vlc_keystore_sys *) p_ss;
> > > + p_keystore->pf_store = Store;
> > > + p_keystore->pf_find = Find;
> > > + p_keystore->pf_remove = Remove;
> > > + p_keystore->pf_secret_load = Secret_load;
> > > + p_keystore->pf_secret_release = Secret_release;
> > > +
> > > + return VLC_SUCCESS;
> > > +}
> > > +
> > > +static void
> > > +Close(vlc_object_t *p_this)
> > > +{
> > > + vlc_keystore *p_keystore = (vlc_keystore *)p_this;
> > > + SecretService *p_ss = (SecretService *) p_keystore->p_sys;
> > > + g_object_unref(p_ss);
> > > +}
> >
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list