[ summarising yesterday's IRC discussion ]
Karl Fogel wrote on Sun, 29 Jun 2008 at 21:18 -0400:
> subversion/libsvn_subr/simple_providers.c:simple_password_get(), which
> is the function affected by r31884, is an implementation of the
> 'svn_auth__password_get_t()' interface:
>
> /* A function that stores in *PASSWORD (potentially after decrypting it)
> the user's password. It might be obtained directly from CREDS, or
> from an external store, using REALMSTRING and USERNAME as keys.
> If NON_INTERACTIVE is set, the user must not be involved in the
> retrieval process. POOL is used for any necessary allocation. */
> typedef svn_boolean_t (*svn_auth__password_get_t)
> (const char **password,
> apr_hash_t *creds,
> const char *realmstring,
> const char *username,
> svn_boolean_t non_interactive,
> apr_pool_t *pool);
>
> That doc string is not clear on whether REALMSTRING or USERNAME may be
> null. But r31884 assumes that USERNAME will not be null -- it passes
> the username directly to strcmp().
>
> Is there ever a situation where a null USERNAME and/or REALMSTRING would
> be reasonable? If so, we should fix up the typedef's doc string, and
> then tweak simple_password_get() (and perhaps other implementations of
> that interface) accordingly.
>
I don't think realm can be NULL because of the way it is used in
svn_auth__simple_first_creds_helper(): it passes it to
svn_config_read_auth_data() which eventually passes it to strlen().
I managed to get password_get() called (by svn_auth__simple_first_creds_helper)
with username=NULL by manually removing the "username" key from the dumped
hash file in ~/.subversion/auth/ -- that way, cached creds for the realm
existed (so creds_hash was non-NULL), but
str = apr_hash_get(creds_hash,
SVN_AUTH__AUTHFILE_USERNAME_KEY,
APR_HASH_KEY_STRING);
returned NULL. I don't know if it's possible to have username=NULL
without manually editing the auth files.
Daniel
> Below is r31884, for reference.
>
> -Karl
>
> ------------------------------------------------------------------------
> r31884 | stylesen | 2008-06-26 05:00:37 -0400 (Thu, 26 Jun 2008) | 7 lines
> Changed paths:
> M /trunk/subversion/libsvn_subr/simple_providers.c
>
> Fix issue #2242: auth cache picking up password from wrong username entry.
>
> * subversion/libsvn_subr/simple_providers.c
> (simple_password_get): Validate the username for which we get the password.
>
> Approved by: danielsh
>
> Index: trunk/subversion/libsvn_subr/simple_providers.c
> ===================================================================
> --- trunk/subversion/libsvn_subr/simple_providers.c (revision 31883)
> +++ trunk/subversion/libsvn_subr/simple_providers.c (revision 31884)
> @@ -65,12 +65,17 @@
> apr_pool_t *pool)
> {
> svn_string_t *str;
> - str = apr_hash_get(creds, SVN_AUTH__AUTHFILE_PASSWORD_KEY,
> + str = apr_hash_get(creds, SVN_AUTH__AUTHFILE_USERNAME_KEY,
> APR_HASH_KEY_STRING);
> - if (str && str->data)
> + if (str && strcmp(str->data, username) == 0)
> {
> - *password = str->data;
> - return TRUE;
> + str = apr_hash_get(creds, SVN_AUTH__AUTHFILE_PASSWORD_KEY,
> + APR_HASH_KEY_STRING);
> + if (str && str->data)
> + {
> + *password = str->data;
> + return TRUE;
> + }
> }
> return FALSE;
> }
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-01 08:12:30 CEST