[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] don't store plain-text passwords by default

From: Daniel Shahaf <d.s_at_daniel.shahaf.co.il>
Date: Sat, 19 Apr 2008 20:26:55 +0300

Stefan Sperling wrote on Sat, 19 Apr 2008 at 17:10 +0200:
> The branch is still rough around the edges and I welcome any comments:
>
> $ svn diff https://svn.collab.net/repos/svn/trunk@30656 \
> https://svn.collab.net/repos/svn/branches/dont-save-plaintext-passwords-by-default
> Index: subversion/include/svn_config.h
> ===================================================================
> --- subversion/include/svn_config.h (.../trunk) (revision 30656)
> +++ subversion/include/svn_config.h (.../branches/dont-save-plaintext-passwords-by-default) (revision 30710)
> @@ -213,7 +214,20 @@ void svn_config_set(svn_config_t *cfg,
> * Parses the option as a boolean value. The recognized representations
> * are 'TRUE'/'FALSE', 'yes'/'no', 'on'/'off', '1'/'0'; case does not
> * matter. Returns an error if the option doesn't contain a known string.
> + *
> + * @a *default_value_was_used is non-NULL, it is set to TRUE if the option
      ^
"If "

> + * was not found in the configuration file, and set to FALSE if it was.
> + *
> + * @since New in 1.6
> */
> +svn_error_t *svn_config_get_bool2(svn_config_t *cfg, svn_boolean_t *valuep,
> Index: subversion/libsvn_subr/config.c
> ===================================================================
> --- subversion/libsvn_subr/config.c (.../trunk) (revision 30656)
> +++ subversion/libsvn_subr/config.c (.../branches/dont-save-plaintext-passwords-by-default) (revision 30710)
> @@ -636,6 +644,15 @@ svn_error_t *
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_config_get_bool(svn_config_t *cfg, svn_boolean_t *valuep,
> + const char *section, const char *option,
> + svn_boolean_t default_value)
> +{
> + svn_boolean_t dummy;

Local variable not needed after r30710.

> + return svn_config_get_bool2(cfg, valuep, section, option,
> + default_value, &dummy);
> +}
> Index: subversion/libsvn_subr/prompt.c
> ===================================================================
> --- subversion/libsvn_subr/prompt.c (.../trunk) (revision 30656)
> +++ subversion/libsvn_subr/prompt.c (.../branches/dont-save-plaintext-passwords-by-default) (revision 30710)
> @@ -376,7 +376,51 @@ svn_cmdline_auth_ssl_client_cert_pw_prompt
> +svn_cmdline_auth_plaintext_prompt(svn_boolean_t *may_save_plaintext,
> + void *baton,
> + apr_pool_t *pool)
> +{
> + const char *answer = NULL;
> + svn_boolean_t answered = FALSE;
> + const char *prompt_string = _("Store password unencrypted (yes/no)? ");
>
> + SVN_ERR(svn_cmdline_printf(pool, "\n"));

You could call svn_cmdline_printf() just once (passing a multiline
string); don't know if it matters.

> + SVN_ERR(svn_cmdline_printf(pool, "-------------------------------------"
> + "----------------------------------\n"));
> + SVN_ERR(svn_cmdline_printf(pool, _("ATTENTION! Your password is going to "
> + "be stored to disk unencrypted!")));
> Index: subversion/svn/cl.h
> ===================================================================
> --- subversion/svn/cl.h (.../trunk) (revision 30656)
> +++ subversion/svn/cl.h (.../branches/dont-save-plaintext-passwords-by-default) (revision 30710)
> @@ -205,6 +205,7 @@ typedef struct svn_cl__opt_state_t
> svn_cl__show_revs_t show_revs; /* mergeinfo flavor */
> svn_depth_t set_depth; /* new sticky ambient depth value */
> svn_boolean_t reintegrate; /* use "reintegrate" merge-source heuristic */
> + svn_boolean_t store_plaintext_passwords; /* do the unthinkable */

New struct member unneeded since r30701.

> } svn_cl__opt_state_t;

Also, in three places whitespace is missing:

> Index: subversion/libsvn_subr/cmdline.c
> ===================================================================
> --- subversion/libsvn_subr/cmdline.c (.../trunk) (revision 30656)
> +++ subversion/libsvn_subr/cmdline.c (.../branches/dont-save-plaintext-passwords-by-default) (revision 30710)
> @@ -467,9 +472,26 @@ svn_cmdline_setup_auth_baton(svn_auth_baton_t **ab
> + if (! default_value_was_used)
> + {
> + if (store_plaintext_password_val)
> + svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_STORE_PLAINTEXT_PASSWORDS,
> + "");
> + else
> + svn_auth_set_parameter(*ab,
> + SVN_AUTH_PARAM_DONT_STORE_PLAINTEXT_PASSWORDS,
> + "");

Here,

> Index: subversion/libsvn_subr/simple_providers.c
> ===================================================================
> --- subversion/libsvn_subr/simple_providers.c (.../trunk) (revision 30656)
> +++ subversion/libsvn_subr/simple_providers.c (.../branches/dont-save-plaintext-passwords-by-default) (revision 30710)
> @@ -238,13 +246,21 @@ simple_save_creds_helper(svn_boolean_t *saved,
> + svn_boolean_t non_interactive = apr_hash_get(parameters,
> SVN_AUTH_PARAM_NON_INTERACTIVE,
> APR_HASH_KEY_STRING) != NULL;

here,

> - svn_boolean_t password_stored = TRUE;
> -
> + simple_provider_baton_t *b = (simple_provider_baton_t*)provider_baton;
                                                          ^
and here.

> *saved = FALSE;
>

I'll leave you alone now. :)

Daniel

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-19 19:27:45 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.