Re: [PATCH] Cache ssl client cert passphrase in gnome-keyring

2008-05-29 08:43:05 Senthil Kumaran S napisał(a):
> subversion/libsvn_subr/cmdline.c
> ===================================================================
> --- subversion/libsvn_subr/cmdline.c    (revision 31507)
> +++ subversion/libsvn_subr/cmdline.c    (working copy)
> @@ -355,10 +355,11 @@
>  }
>  
>  #if defined(SVN_HAVE_KWALLET) || defined(SVN_HAVE_GNOME_KEYRING)
> -/* Dynamically load authentication simple provider. */
> +/* Dynamically load authentication provider. */
>  static svn_boolean_t
> -get_auth_simple_provider(svn_auth_provider_object_t **provider,
> +get_auth_provider(svn_auth_provider_object_t **provider,
>                           const char *provider_name,
> +                         const char *provider_type,
>                           apr_pool_t *pool)
                     ^^^^^^^
Indentation should be updated here.

> @@ -501,7 +511,36 @@
>    APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
>    svn_auth_get_ssl_client_cert_file_provider(&provider, pool);
>    APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
> -  svn_auth_get_ssl_client_cert_pw_file_provider(&provider, pool);
> +#ifdef SVN_HAVE_GNOME_KEYRING
> +  for (i = 0; i < password_stores->nelts; i++)
> +    {
> +      const char *password_store = APR_ARRAY_IDX(password_stores, i,
> +                                                 const char *);
> +
> +      if (apr_strnatcmp(password_store, "gnome-keyring") == 0)
> +        {
> +          if (get_auth_provider(&provider, "gnome_keyring",
> +                                "ssl_client_cert_pw", pool))
> +            {
> +              APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) =
> provider; +            }
> +          continue;
> +        }
> +    }

You should use the already existing loop.

> Index: subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
> ===================================================================
> --- subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (revision 31507)
> +++ subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (working copy)
> @@ -189,3 +189,60 @@
>  
>    gnome_keyring_init();
>  }
> +
> +/* Get cached encrypted credentials from the ssl client cert password
> + * provider's cache. */
> +static svn_error_t *
> +gnome_keyring_ssl_client_cert_pw_first_creds(void **credentials,
> +                                             void **iter_baton,
> +                                             void *provider_baton,
> +                                             apr_hash_t *parameters,
> +                                             const char *realmstring,
> +                                             apr_pool_t *pool)
> +{
> +  return svn_auth__ssl_client_cert_pw_file_first_creds_helper
> +           (credentials,
> +            iter_baton, provider_baton,
> +            parameters, realmstring,
> +            gnome_keyring_password_get,
> +            SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE,
> +            pool);
> +}
> +
> +/* Save encrypted credentials to the ssl client cert password provider's
> + * cache. */
> +static svn_error_t *
> +gnome_keyring_ssl_client_cert_pw_save_creds(svn_boolean_t *saved,
> +                                            void *credentials,
> +                                            void *provider_baton,
> +                                            apr_hash_t *parameters,
> +                                            const char *realmstring,
> +                                            apr_pool_t *pool)
> +{
> +  return svn_auth__ssl_client_cert_pw_file_save_creds_helper
> +           (saved, credentials,
> +            provider_baton, parameters,
> +            realmstring,
> +            gnome_keyring_password_set,
> +            SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE,
> +            pool);
> +}

You use gnome_keyring_password_get() and gnome_keyring_password_set().
Won't it cause collision with passwords stored by GNOME Keyring simple provider?

> +/* Public API */
> +void
> +svn_auth_get_gnome_keyring_ssl_client_cert_pw_provider
> +    (svn_auth_provider_object_t **provider,
> +     apr_pool_t *pool)
> +{
> +  svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
> +
> +  po->vtable = &gnome_keyring_ssl_client_cert_pw_provider;
> +  *provider = po;
> +}

You can declare this function in subversion/include/svn_auth_dso.h.

-- 
Arfrever Frehtes Taifersar Arahesis