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

On Fri, Apr 18, 2008 at 12:28:02PM -0700, David Glasser wrote:
> The problem is, it really does seem to be a bit of misdirection.

Dave (and others),

we've talked a bit more about this in IRC yesterday.
I really want to find a solution that everyone is OK with,
if at all possible.

I've changed one thing on the branch this morning, based on
your feedback that you'd really like not to lose the convenience
aspect of storing passwords by default.

I tossed out the --store-plaintext-pw command line option,
and instead added a prompt that pops up only if no password
is already cached, and if the 'store-plaintext-passwords' option
in the configuration file is set to neither 'yes' nor 'no'.

It currently looks like this (The look is a bikeshed issue.
I'm not in love with what it currently looks like):

  $ svn ls svn://localhost
  Authentication realm: <svn://localhost:3690> My First Repository
  Password for 'stsp':
  Authentication realm: <svn://localhost:3690> My First Repository
  Username: harry
  Password for 'harry':
  
  -----------------------------------------------------------------------
  ATTENTION! Your password is going to be stored to disk unencrypted!
  -----------------------------------------------------------------------
  You can get rid of this warning by editing your configuration file
  and setting 'store-plaintext-passwords' to either 'yes' or 'no'.
  Store password unencrypted (yes/no)? asjdklsajdklasdjaskldj
  Please type 'yes' or 'no': asjdklsjd
  Please type 'yes' or 'no': nonono
  Please type 'yes' or 'no': nox
  Please type 'yes' or 'no': no
  $ svn ls svn://localhost
  Authentication realm: <svn://localhost:3690> My First Repository
  Password for 'harry':
  
  -----------------------------------------------------------------------
  ATTENTION! Your password is going to be stored to disk unencrypted!
  -----------------------------------------------------------------------
  You can get rid of this warning by editing your configuration file
  and setting 'store-plaintext-passwords' to either 'yes' or 'no'.
  Store password unencrypted (yes/no)? yes
  $ svn ls svn://localhost
  $

As you can see, if people answer 'yes' to this prompt, it never
bothers them again (for the same authentication realm on the same
server). This is how I'm trying to keep the convenience factor
in the upper ranges.

The prompt is printed from a callback that clients can
set to whatever they wish (i.e. I'm trying to take GUIs into account).
See the diff for more details, the callback is declared in
subversion/include/svn_auth.h. 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

(That branch name is too long, I know.
I'm already really sick of typing it.)

There are still TODO items on the branch, as follows:

  - properly handle ^C during the prompt
  
  - Add the 'store-plaintext-passwords' option to ~/.subversion/servers,
    in addition to the global one in ~/.subversion/config.
    The use case being that some people may want to save plaintext
    passwords for some servers, but not for others.
    (Is this also possible on a per-repository basis?)
  
  - During the prompt, tell users where exactly the configuration file is,
    if possible.
  
  - Handle the non-interactive case properly.

Once these are done, do you think this is in principle something
you'd be more inclined to agree with than my first approach?

-- 
Stefan Sperling <stsp_at_elego.de>                 Software Developer
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner