You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Martin Furter <mf...@apache.org> on 2014/07/06 14:16:05 UTC
[PATCH]: Add --password-file and --password-envvar
Attached is a log message and a patch which adds the new options
'--password-file' and '--password-envvar'. It also adds Julians warning
to the '--password' help text.
I haven't found out yet how the test suite checks the '--password'
option. So I tested it just by hand :)
Also I added a check that stdin is specified only once. This should
probably go into a seperate patch.
This is just the first iteration of the patch. Comments welcome.
- Martin
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Ben Reser <be...@reser.org>.
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.
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Martin Furter <mf...@bluewin.ch>.
Again reply to the list too :)
GUI's which change buttons etc. depending on whatever they like are bad...
On 07/08/14 08:02, Martin Furter wrote:
> On 07/08/14 03:33, Ben Reser wrote:
>> On 7/6/14 5:16 AM, Martin Furter wrote:
>>> Attached is a log message and a patch which adds the new options
>>> '--password-file' and '--password-envvar'. It also adds Julians
>>> warning to the
>>> '--password' help text.
>>
>> I veto (-1) --password-envar (and peters follow-up suggestion of a
>> hard-coded
>> environment variable). As several other people have shown the
>> environment of a
>> running program is often just as available as the command line
>> arguments. The
>> whole point of this exercise is to improve the security of the manner
>> in which
>> we allow passwords to be provided in order to guide users to make good
>> choices.
>> We're not achieving anything if we only provide them with new insecure
>> choices.
>
> 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.
>
> 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 :)
>
> - Martin
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Ben Reser <be...@reser.org>.
On 7/6/14 5:16 AM, Martin Furter wrote:
> Attached is a log message and a patch which adds the new options
> '--password-file' and '--password-envvar'. It also adds Julians warning to the
> '--password' help text.
I veto (-1) --password-envar (and peters follow-up suggestion of a hard-coded
environment variable). As several other people have shown the environment of a
running program is often just as available as the command line arguments. The
whole point of this exercise is to improve the security of the manner in which
we allow passwords to be provided in order to guide users to make good choices.
We're not achieving anything if we only provide them with new insecure choices.
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Jul 06, 2014 at 08:43:06PM +0530, Martin Furter wrote:
>
> Resending my reply to the list too...
>
> >I don't know a command which shows the environment of a process as nice
> >as 'ps' shows the process arguments.
ps -e on OpenBSD < 5.3 used to show the environment of every process
on the system.
I would expect several other variants of unix to show env vars too.
So, generally, it's not safe to rely on environment variables being
secret.
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Branko Čibej <br...@wandisco.com>.
On 07.07.2014 10:51, Julian Foad wrote:
> Branko Čibej wrote:
>> On 07.07.2014 10:27, Julian Foad wrote:
>>> Aha! But Subversion already has a way to read authn creds from a file:
>>>
>>> --config-dir=x
>>>
>>> All we're lacking is a convenient way to put the required creds into
>>> the file. A user interface could be:
>>>
>>> svn auth authenticate $REPO_URL
>>>
>>> or, if you insist on being able to cache a user name and password when you
>>> don't currently have access to the server:
>>>
>>> svn auth authenticate $REPO_URL --force --username=y --password=z
>>>
>>> Thoughts?
>> Won't work given how we currently store credentials. The credentials key
>> is not the URL, it's the realmstring, which you (in general) will not
>> know without actually contacting the server.
> OK, right. To be clear, you mean the *second* thing I wrote (the --force variant) won't work. The first will.
Yes, certainly.
There are other uses for your proposed "svn auth authenticate" command,
though. For example, it would be really nice if users could register a
trusted server cert before contacting the server, and register a client
cert for a realm before contacting the server, too. For example, we
currently do not even store client certs in the credentials store, only
their passphrases; which is rather unfortunate, to put it mildly.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Jul 8, 2014 at 7:14 AM, Branko Čibej <br...@wandisco.com> wrote:
>...
> I just realized that even the variant without --force and with access to
> the may not always work. Consider what "authenticate" means in ra_serf:
> it would rely on issuing a request to the server that does not modify
> the repository, but does trigger the part of the HTTPd configuration
> that causes the server to respond with an authentication request. There
> is no way to tell how the server is configured and therefore which HTTP
> methods to which URLs will require authentication.
>
Right. In general, you have to attempt a modification before a
(typically-configured, HTTP-based) server will query for your credentials.
That said, this is a command that is run specially. Maybe ever just once.
It would be okay to make several requests to the server. In increasing
difficulty, the client could issue 1-3 requests to try and trigger an
authentication event:
0) client certificate needed to open TLS session
1) the opening OPTIONS request
2) HEAD or PROPFIND on a resource
3) POST/MKACTIVITY to begin a commit (then DELETE if successful)
That would likely trigger auth in 99% of configurations. (I could envision
a server with anon-write to most areas, and auth-write to specific paths,
so a "real" commit with paths would be needed)
Seems a new RA entrypoint would be appropriate, to build something like
this.
Cheers,
-g
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Branko Čibej <br...@wandisco.com>.
On 07.07.2014 10:51, Julian Foad wrote:
> Branko Čibej wrote:
>> On 07.07.2014 10:27, Julian Foad wrote:
>>> Aha! But Subversion already has a way to read authn creds from a file:
>>>
>>> --config-dir=x
>>>
>>> All we're lacking is a convenient way to put the required creds into
>>> the file. A user interface could be:
>>>
>>> svn auth authenticate $REPO_URL
>>>
>>> or, if you insist on being able to cache a user name and password when you
>>> don't currently have access to the server:
>>>
>>> svn auth authenticate $REPO_URL --force --username=y --password=z
>>>
>>> Thoughts?
>> Won't work given how we currently store credentials. The credentials key
>> is not the URL, it's the realmstring, which you (in general) will not
>> know without actually contacting the server.
> OK, right. To be clear, you mean the *second* thing I wrote (the --force variant) won't work. The first will.
I just realized that even the variant without --force and with access to
the may not always work. Consider what "authenticate" means in ra_serf:
it would rely on issuing a request to the server that does not modify
the repository, but does trigger the part of the HTTPd configuration
that causes the server to respond with an authentication request. There
is no way to tell how the server is configured and therefore which HTTP
methods to which URLs will require authentication.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> On 07.07.2014 10:27, Julian Foad wrote:
>> Aha! But Subversion already has a way to read authn creds from a file:
>>
>> --config-dir=x
>>
>> All we're lacking is a convenient way to put the required creds into
>> the file. A user interface could be:
>>
>> svn auth authenticate $REPO_URL
>>
>> or, if you insist on being able to cache a user name and password when you
>> don't currently have access to the server:
>>
>> svn auth authenticate $REPO_URL --force --username=y --password=z
>>
>> Thoughts?
>
> Won't work given how we currently store credentials. The credentials key
> is not the URL, it's the realmstring, which you (in general) will not
> know without actually contacting the server.
OK, right. To be clear, you mean the *second* thing I wrote (the --force variant) won't work. The first will.
- Julian
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Branko Čibej <br...@wandisco.com>.
On 07.07.2014 10:27, Julian Foad wrote:
> Martin Furter wrote:
>
>>>> For the file solution it might be more useful to use both username and
>>>> password from that file.
>>> I guess the option should be named different then, maybe something like
>>> --auth-file or --creds-file or so.
> Aha! But Subversion already has a way to read authn creds from a file:
>
> --config-dir=x
>
> All we're lacking is a convenient way to put the required creds into the file. A user interface could be:
>
> svn auth authenticate $REPO_URL
>
> or, if you insist on being able to cache a user name and password when you don't currently have access to the server:
>
> svn auth authenticate $REPO_URL --force --username=y --password=z
>
> Thoughts?
Won't work given how we currently store credentials. The credentials key
is not the URL, it's the realmstring, which you (in general) will not
know without actually contacting the server.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Julian Foad <ju...@btopenworld.com>.
Martin Furter wrote:
>>> For the file solution it might be more useful to use both username and
>>> password from that file.
>>
>> I guess the option should be named different then, maybe something like
>> --auth-file or --creds-file or so.
Aha! But Subversion already has a way to read authn creds from a file:
--config-dir=x
All we're lacking is a convenient way to put the required creds into the file. A user interface could be:
svn auth authenticate $REPO_URL
or, if you insist on being able to cache a user name and password when you don't currently have access to the server:
svn auth authenticate $REPO_URL --force --username=y --password=z
Thoughts?
- Julian
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Martin Furter <mf...@bluewin.ch>.
Resending my reply to the list too...
On 07/06/14 20:39, Martin Furter wrote:
> On 07/06/14 20:09, Bert Huijben wrote:
>> I'm not sure if the envvar option is really any safer than the argument
>> option, if the command line is really read from the environment block on
>> these platforms. As such I don't think having a command option for that
>> really helps.
>
> I don't know a command which shows the environment of a process as nice
> as 'ps' shows the process arguments.
>
> But on linux the environment is readable by the owner of the process
> through /proc/$PID/environ as the following commandline shows:
>
> $ PASSWORD=secret123 sh -c "tr '\\0' '\\n' < /proc/\$\$/environ" |
> grep PASSWORD
>
>> For the file solution it might be more useful to use both username and
>> password from that file.
>
> I guess the option should be named different then, maybe something like
> --auth-file or --creds-file or so.
>
> - Martin
RE: [PATCH]: Add --password-file and --password-envvar
Posted by Bert Huijben <be...@qqmail.nl>.
I'm not sure if the envvar option is really any safer than the argument option, if the command line is really read from the environment block on these platforms. As such I don't think having a command option for that really helps.
For the file solution it might be more useful to use both username and password from that file.
Bert
-----Original Message-----
From: "Martin Furter" <mf...@apache.org>
Sent: 6-7-2014 14:16
To: "dev@subversion.apache.org" <de...@subversion.apache.org>
Subject: [PATCH]: Add --password-file and --password-envvar
Attached is a log message and a patch which adds the new options
'--password-file' and '--password-envvar'. It also adds Julians warning
to the '--password' help text.
I haven't found out yet how the test suite checks the '--password'
option. So I tested it just by hand :)
Also I added a check that stdin is specified only once. This should
probably go into a seperate patch.
This is just the first iteration of the patch. Comments welcome.
- Martin
Re: [PATCH]: Add --password-file and --password-envvar
Posted by Peter Samuelson <pe...@p12n.org>.
[Martin Furter]
> Attached is a log message and a patch which adds the new options
> '--password-file' and '--password-envvar'.
I don't agree with --password-envvar. If we're going to support
reading a password from the environment at all, just do what everyone
always does with the environment: the application chooses the variable
name and looks for it unconditionally; and if the user wants or doesn't
want this, they can set or unset the variable.
(As others have noted, we may or may not wish to support this at all,
given that on some platforms a process environment is readable by other
users.)
Peter