You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2008/07/15 20:10:19 UTC
Re: svn commit: r32132 - in trunk/subversion: include include/private
libsvn_auth_gnome_keyring libsvn_ra libsvn_ra_neon libsvn_subr
Had just a brief look on this, but:
kfogel@tigris.org wrote on Tue, 15 Jul 2008 at 12:41 -0700:
> Author: kfogel
> Date: Tue Jul 15 12:41:22 2008
> New Revision: 32132
>
> Log:
> Cache SSL client certificate passphrases, when user indicates it's okay.
>
> Patch by: stylesen
> (Tweaked by kfogel.)
>
> Modified: trunk/subversion/libsvn_subr/ssl_client_cert_pw_providers.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/ssl_client_cert_pw_providers.c?pathrev=32132&r1=32131&r2=32132
> ==============================================================================
> --- trunk/subversion/libsvn_subr/ssl_client_cert_pw_providers.c Tue Jul 15 12:33:20 2008 (r32131)
> +++ trunk/subversion/libsvn_subr/ssl_client_cert_pw_providers.c Tue Jul 15 12:41:22 2008 (r32132)
> @@ -67,23 +163,268 @@ ssl_client_cert_pw_file_first_credential
> }
>
>
> +svn_error_t *
> +svn_auth__ssl_client_cert_pw_file_save_creds_helper
> + (svn_boolean_t *saved,
> + void *credentials,
> + void *provider_baton,
> + apr_hash_t *parameters,
> + const char *realmstring,
> + svn_auth__password_set_t passphrase_set,
> + const char *passtype,
> + apr_pool_t *pool)
> +{
> + svn_auth_cred_ssl_client_cert_pw_t *creds = credentials;
> + apr_hash_t *creds_hash = NULL;
> + const char *config_dir;
> + svn_error_t *err;
> + svn_boolean_t dont_store_passphrase =
> + apr_hash_get(parameters,
> + SVN_AUTH_PARAM_DONT_STORE_SSL_CLIENT_CERT_PP,
> + APR_HASH_KEY_STRING) != NULL;
> + const char *store_ssl_client_cert_pp_plaintext =
> + apr_hash_get(parameters,
> + SVN_AUTH_PARAM_STORE_SSL_CLIENT_CERT_PP_PLAINTEXT,
> + APR_HASH_KEY_STRING);
> + svn_boolean_t non_interactive = apr_hash_get(parameters,
> + SVN_AUTH_PARAM_NON_INTERACTIVE,
> + APR_HASH_KEY_STRING) != NULL;
> + ssl_client_cert_pw_file_provider_baton_t *b =
> + (ssl_client_cert_pw_file_provider_baton_t *)provider_baton;
> +
> + svn_boolean_t no_auth_cache =
> + (! creds->may_save) || (apr_hash_get(parameters,
> + SVN_AUTH_PARAM_NO_AUTH_CACHE,
> + APR_HASH_KEY_STRING) != NULL);
> +
> + *saved = FALSE;
> +
> + if (no_auth_cache)
> + return SVN_NO_ERROR;
> +
> + config_dir = apr_hash_get(parameters,
> + SVN_AUTH_PARAM_CONFIG_DIR,
> + APR_HASH_KEY_STRING);
> + creds_hash = apr_hash_make(pool);
> +
> + /* Don't store passphrase in any form if the user has told
> + us not to do so. */
> + if (! dont_store_passphrase)
> + {
> + svn_boolean_t may_save_passphrase = FALSE;
> +
> + /* If the passphrase is going to be stored encrypted, go right
> + ahead and store it to disk. Else determine whether saving
> + in plaintext is OK. */
> + if (strcmp(passtype, SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE) == 0)
> + {
Why do we special-case gnome here? In simple_providers.c we have four
providers listed here:
/* If the password is going to be stored encrypted, go right
* ahead and store it to disk. Else determine whether saving
* in plaintext is OK. */
if (strcmp(passtype, SVN_AUTH__WINCRYPT_PASSWORD_TYPE) == 0
|| strcmp(passtype, SVN_AUTH__KEYCHAIN_PASSWORD_TYPE) == 0
|| strcmp(passtype, SVN_AUTH__KWALLET_PASSWORD_TYPE) == 0
|| strcmp(passtype, SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE) == 0)
And, indeed, this function and the simple_providers.c functions (e.g.
svn_auth__simple_save_creds_helper) seem very similar, almost duplicates
of each other. (I said that on IRC, but repeating here for completion.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r32132 - in trunk/subversion: include include/private libsvn_auth_gnome_keyring libsvn_ra libsvn_ra_neon libsvn_subr
Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Jul 15, 2008 at 4:36 PM, Karl Fogel <kf...@red-bean.com> wrote:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>>> + /* Don't store passphrase in any form if the user has told
>>> + us not to do so. */
>>> + if (! dont_store_passphrase)
>>> + {
>>> + svn_boolean_t may_save_passphrase = FALSE;
>>> +
>>> + /* If the passphrase is going to be stored encrypted, go right
>>> + ahead and store it to disk. Else determine whether saving
>>> + in plaintext is OK. */
>>> + if (strcmp(passtype, SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE) == 0)
>>> + {
>>
>> Why do we special-case gnome here? In simple_providers.c we have four
>> providers listed here:
>>
>> /* If the password is going to be stored encrypted, go right
>> * ahead and store it to disk. Else determine whether saving
>> * in plaintext is OK. */
>> if (strcmp(passtype, SVN_AUTH__WINCRYPT_PASSWORD_TYPE) == 0
>> || strcmp(passtype, SVN_AUTH__KEYCHAIN_PASSWORD_TYPE) == 0
>> || strcmp(passtype, SVN_AUTH__KWALLET_PASSWORD_TYPE) == 0
>> || strcmp(passtype, SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE) == 0)
>>
>> And, indeed, this function and the simple_providers.c functions (e.g.
>> svn_auth__simple_save_creds_helper) seem very similar, almost duplicates
>> of each other. (I said that on IRC, but repeating here for completion.)
>
> Thanks. Yes, as I mentioned in my mail (sent just a few moments ago,
> http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=141076),
> there's a lot of opportunity for abstraction/unification here.
>
> I assumed there was some difference between a password and a client-cert
> passphrase -- like: Gnome Keyring has some way of string the latter,
> whereas perhaps other systems don't, or have to do it by overloading
> their regular password-storage mechanism, which results in some kind of
> UI weirdness?
>
> But I don't know for sure. I have this sneaking memory that Senthil
> actually antcipated this question and told me the reason, too, but I
> can't remember it now. Senthil?
I think it was just that Senthil did not want to add any providers
without personally testing them. So he wanted to get the base code
and one provider committed, so he can move on to adding the other
providers. Which means setting up Windows, and OSX build systems. Of
course, it should now also be possible for others to pitch in and add
the other options.
Likewise, I think will eventually need someone to add the necessary
support into JavaHL (SVNClient.cpp). Should be a copy and paste from
cmdline.c.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r32132 - in trunk/subversion: include include/private libsvn_auth_gnome_keyring libsvn_ra libsvn_ra_neon libsvn_subr
Posted by Karl Fogel <kf...@red-bean.com>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
>> + /* Don't store passphrase in any form if the user has told
>> + us not to do so. */
>> + if (! dont_store_passphrase)
>> + {
>> + svn_boolean_t may_save_passphrase = FALSE;
>> +
>> + /* If the passphrase is going to be stored encrypted, go right
>> + ahead and store it to disk. Else determine whether saving
>> + in plaintext is OK. */
>> + if (strcmp(passtype, SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE) == 0)
>> + {
>
> Why do we special-case gnome here? In simple_providers.c we have four
> providers listed here:
>
> /* If the password is going to be stored encrypted, go right
> * ahead and store it to disk. Else determine whether saving
> * in plaintext is OK. */
> if (strcmp(passtype, SVN_AUTH__WINCRYPT_PASSWORD_TYPE) == 0
> || strcmp(passtype, SVN_AUTH__KEYCHAIN_PASSWORD_TYPE) == 0
> || strcmp(passtype, SVN_AUTH__KWALLET_PASSWORD_TYPE) == 0
> || strcmp(passtype, SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE) == 0)
>
> And, indeed, this function and the simple_providers.c functions (e.g.
> svn_auth__simple_save_creds_helper) seem very similar, almost duplicates
> of each other. (I said that on IRC, but repeating here for completion.)
Thanks. Yes, as I mentioned in my mail (sent just a few moments ago,
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=141076),
there's a lot of opportunity for abstraction/unification here.
I assumed there was some difference between a password and a client-cert
passphrase -- like: Gnome Keyring has some way of string the latter,
whereas perhaps other systems don't, or have to do it by overloading
their regular password-storage mechanism, which results in some kind of
UI weirdness?
But I don't know for sure. I have this sneaking memory that Senthil
actually antcipated this question and told me the reason, too, but I
can't remember it now. Senthil?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org