[vlc-devel] [PATCH 3/4] keystore: add libsecret keystore module

Thomas Guillem thomas at gllm.fr
Thu Nov 26 11:47:43 CET 2015



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.

> 
> 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


More information about the vlc-devel mailing list