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

Re: [PATCH]: Add --password-file and --password-envvar

From: Ben Reser <ben_at_reser.org>
Date: Tue, 08 Jul 2014 15:55:10 -0700

On 7/7/14 7:38 PM, Martin Furter wrote:
>> On Linux I see only the environment of my own processes. On OpenBSD I
>> see only HOME and PATH for other users. So envvar seems to not be less
>> secure than a password file.

Except that it shows in up for the root user for all commands regardless of
user. For example with BSD style arguments on Linux:
# ps xewww | grep postfix | grep -v grep
 1930 ? Ss 1:14 /usr/lib/postfix/master
manpage_directory=/usr/share/man queue_directory=/var/spool/postfix
command_directory=/usr/sbin data_directory=/var/lib/postfix mail_owner=postfix
OLDPWD=/etc/postfix sample_directory=/usr/share/doc/postfix/examples
sendmail_path=/usr/sbin/sendmail setgid_group=postdrop readme_directory=no
config_directory=/etc/postfix html_directory=no
PATH=/bin:/usr/bin:/sbin:/usr/sbin LANG=C MAIL_CONFIG=/etc/postfix
daemon_directory=/usr/lib/postfix mailq_path=/usr/bin/mailq
PWD=/var/spool/postfix MAIL_LOGTAG=postfix newaliases_path=/usr/bin/newaliases

I suspect we want to avoid the root user accidentally seeing passwords just
from running ps, even if they do have to run ps with someone unusual options.

Granted root can ignore file permissions with respect to reading the password
from a file, but I think that a root user opening a file is a much more active
level of snooping. I know as a root user I would avoid opening files unless I
had a specific reason to do so.

>> If you really want to improve security the only option is using stdin.
>> I had a patch for that ready. But then people started wishing other
>> things so I just implemented without thinking too much :)

I don't like this patch for a couple of reasons.

First it blocks any other interactive prompting. Not just prompting for
authentication. Most notably all our conflict resolution prompting won't work
with this functionality. For example (with your patch):
svnadmin create repo
svn co file://$PWD/repo wc
cd wc
echo foo > foo
svn add foo
svn ci -mm
echo foobar > foo
svn ci -mm
svn up -r1
svn rm foo
svn up --password - < /dev/null

This forces users who want to script the authentication but not the conflict
resolution to choose, insecure passwords or no interactive conflict resolution.

Second it adds a magic value to a long standing argument that didn't have one.
 If the argument was expecting a path name I wouldn't think much of it because
'-' is an expected magic value in that case. In this case the argument is
expected to be the literal password. Using '-' for a password would be
terrible but I don't see the benefit to doing this when we can add
--password-file, which takes the '-' (allowing this usage) and allows reading
from a file that can work with interactive conflict resolution.

I realize that putting passwords into a file is far from perfect. You have to
worry about permissions (which root can ignore) and possibly securely deleting
the file when done (which is becoming far harder with SSDs). But I think it's
a reasonable security tradeoff for still having stdin available for prompting.

So I'd prefer your original patch with just --password-file.
Received on 2014-07-09 00:55:39 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.