A strong WTF on compiling out plaintext password support by default?!

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

A strong WTF on compiling out plaintext password support by default?!

Robby Zinchak
Small rant here from a very long time subversion user, regarding subversion project's decision to compile out plaintext password storage by default (https://marc.info/?l=subversion-commits&m=154101482302608&w=2). 

There are a tremendous number of scenarios where users would desire to use subversion without a keyring -- for me, that's today running Ubuntu 20.04 and trying to set up an automated subversion client on a server VM.  I obviously can't be logging into a keyring every time the server reboots, that'd be idiotic.

I cannot fathom how the team thought this was a good decision.  It reeks of devs thinking "we know better, force the users to do it this way," without actually understanding the needs of your users.  

I'm left with four solutions as far as I can see it:

1) Crowbarring one of the supported keyrings into not needing a keyring password.  Assuming this is even possible, it seems like a lot of extra work with no benefit, and added fragility.  I am loathe to dive into a whole set of docs to try and figure this out (assuming it's even possible), when the old methods worked just fine.

2) Compiling my own subversion with the enable-plaintext-password-storage flag -- obviously insecure since there's no way I'll be able to keep up with software updates.  And I've heard it's quite difficult to compile subversion, so that'll waste precious time I could be spending on something else that's actually productive for my business.

3) Finding an ubuntu package overlay by a third party, questionably insecure since now I have to trust an unofficial/unvetted third party with providing svn builds.

4) Bite the bullet and just switch to another VCS

Every time version control comes up in dev conversations among my peers, I go to great lengths to defend SVN against the many criticisms my peers level at it and promote it to other devs looking for a quality VCS.  But honestly this decision is one of the most myopic ones I've seen in years on any software project and reeks of the project developers making an idealistic stand that inconveniences users with no practical real-world benefit.  The decision should be left to the user, rather than forcing them into a difficult situation.  The earlier change to make plaintext something users have to intentionally opt into makes total sense so that users are aware of what they're opting into - but removing it entirely is too far.  I guarantee you, right now there are people trying to puzzle through this stack overflow answer, giving up, and switching to git.  (https://stackoverflow.com/questions/2899209/how-to-save-password-when-using-subversion-from-the-console)  This is a downright awful decision for the overall long term health of what's left of the subversion userbase.

I would love to hear what the expected workaround is for users running automated subversion clients on server VMs, because all the options look rather terrible.

Thanks for listening to my rant, and be assured it comes only from a place of wanting to see subversion succeed.

Best,
Robby
Reply | Threaded
Open this post in threaded view
|

Re: A strong WTF on compiling out plaintext password support by default?!

Daniel Shahaf-2
Robby Zinchak wrote on Thu, 06 Aug 2020 18:56 -0700:
> I would love to hear what the expected workaround is for users running
> automated subversion clients on server VMs [...].
>

As I wrote on SVN-4861 recently:

> > Also, if I'm reading the code correctly, simply adding the password
> > to the md5sum(realm)-named file in ~/.subversion/auth/svn.simple/
> > should work: the compile-time knob prevents passwords from being
> > _written_, but doesn't prevent passwords already there from being
> > read.

Reply | Threaded
Open this post in threaded view
|

Re: A strong WTF on compiling out plaintext password support by default?!

Dr. Thomas Orgis
Am Fri, 7 Aug 2020 05:53:24 +0000
schrieb Daniel Shahaf <[hidden email]>:

> > > should work: the compile-time knob prevents passwords from being
> > > _written_, but doesn't prevent passwords already there from being
> > > read.  

Then it might be a nice idea to allow users to intentionally trigger
that write when they know what they are doing. Well, that was of course
what the old behaviour did, but a bit implicitly. Once could imagine a
new command to make it explicit. Something like

        svn store-password $user $repo
.
But I suspect subversion devs don't fancy adding extra cruft to support
the use-cases for passwordless operation. I'm not sure what games one
could play with client certificates or similar. Storing the password is
for sure a lot simpler and doesn't need setting up svnserve with SASL
(although I did that for encryption).

Building it myself is not that hard. So I have my script that installs
svn with plaintext password storage and I use that as part of
bootstrapping our systems.

I'm preparing my little add-on rant as reply to the initial post to
give some more colour. Short version, in case I don't make it:

Disabling plaintext passwords makes it harder to keep using Subversion
in cases where it is truly superior to DVCS. For code development it's
hard to justify Subversion nowadays, when there's not even a standard
property to differentiate between author and committer of a patch (I
guess one could just define one by convention?). I keep using it for
existing projects but feel increasingly stupid for doing so, despite my
opinion of the file tree semantics being superior to branching/tagging
elsewhere.


Alrighty then,

Thomas

--
Dr. Thomas Orgis
HPC @ Universität Hamburg
Reply | Threaded
Open this post in threaded view
|

Re: A strong WTF on compiling out plaintext password support by default?!

Stefan Sperling
In reply to this post by Robby Zinchak
On Thu, Aug 06, 2020 at 06:56:55PM -0700, Robby Zinchak wrote:

> 2) Compiling my own subversion with the enable-plaintext-password-storage
> flag -- obviously insecure since there's no way I'll be able to keep up
> with software updates.  And I've heard it's quite difficult to compile
> subversion, so that'll waste precious time I could be spending on something
> else that's actually productive for my business.
>
> 3) Finding an ubuntu package overlay by a third party, questionably
> insecure since now I have to trust an unofficial/unvetted third party with
> providing svn builds.
>
> 4) Bite the bullet and just switch to another VCS

5) Convince Ubuntu packagers to enable this feature.

Package maintainers can very easily re-enable this at compile-time.
It's a single extra flag to pass to during the 'configure' step.
I did exactly this on OpenBSD last year (commit shown below).

The real problem is that whatever Subversion's upstream default is, one
group of people is going to be unhappy. Everybody seems to expect their
own use case to work out of the box, failing to recognize that contradictory
requirements exist. There really isn't a one-size fits all solution to this.
We provide all the options, but packagers need to choose and take the necessary
steps to get the behaviour they want to provide to their users.

A recent example in a related project (TortoiseSVN) where people want the
exact opposite of what you want:
https://groups.google.com/forum/#!topic/tortoisesvn/V3rLLYZgeRA
In this case, the problem is that allowing people to easily sniff passwords
is considered a big no-no in a particular deployment, and means SVN might
get banned there. The behaviour was not changed; it is TortoiseSVN's choice
as the packager of these particular SVN binaries, and this needs to be
respected.

Do you know if Ubuntu made a decision to disallow plaintext passwords?
Perhaps they just missed the news that it has become a compile-time option?

Regards,
Stefan


https://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/devel/subversion/Makefile
CVSROOT: /cvs
Module name: ports
Changes by: [hidden email] 2019/12/19 12:43:14

Modified files:
        devel/subversion: Makefile

Log message:
Re-enable support for storing plaintext passwords in Subversion.

Subversion has disabled saving of plaintext passwords by default
and a compile-time option is now required to enable this feature.
OpenBSD has always disabled this feature at run-time in /etc/subversion
and left users the choice to enable it in their configuration files.

Unfortunately, the alternative password stores, gnome-keyring and
KDE wallet, do not work in non-X11 environments. And the gpg-agent
password store is not persistent. So there is no better solution
for unattended SVN password authentication in non-X11 environments
on OpenBSD, or pretty much any UNIX-like system for that matter.

ok sthen@

Reply | Threaded
Open this post in threaded view
|

Re: A strong WTF on compiling out plaintext password support by default?!

Daniel Shahaf-2
In reply to this post by Dr. Thomas Orgis
Dr. Thomas Orgis wrote on Fri, 07 Aug 2020 09:41 +0200:

> Am Fri, 7 Aug 2020 05:53:24 +0000
> schrieb Daniel Shahaf <[hidden email]>:
>
> > > > should work: the compile-time knob prevents passwords from being
> > > > _written_, but doesn't prevent passwords already there from being
> > > > read.    
>
> Then it might be a nice idea to allow users to intentionally trigger
> that write when they know what they are doing. Well, that was of course
> what the old behaviour did, but a bit implicitly. Once could imagine a
> new command to make it explicit. Something like
>
> svn store-password $user $repo

I'm attaching a prototype standalone script implementing this
functionality.

It successfully adds a password to the storage, in the sense that
after running it, a subsequent `svn auth --show-passwords` shows the
password.  Still, a subsequent `svn info` doesn't use the password.
Why?  By source inspection, SVN_DISABLE_PLAINTEXT_PASSWORD_STORAGE
affects svn_auth__simple_creds_cache_set() but not svn_auth__simple_creds_cache_get(),
so why doesn't the latter use the password?

Cheers,

Daniel

[[[
#!/usr/local/bin/zsh -f
# Prompt for a realm and a password, then cache that password for that realm, in plaintext.
PS3="Enter the number of the selected option: "
creds=( "${(ps.\n\n.)"$(svn auth)"}" )
creds=( ${(M)creds:#-*} )
select m in $creds
do
        realm=${(M)${(f)m}:#Authentication realm: *}
        realm=${realm#*: }
        IFS= read -s -r pw"?Password: "
        md5=${"$(printf %s "$realm" | openssl md5)"##*= }
        print -rC1 \
                \$ i "K 8" password "V ${#pw}" "$pw" "." "w" "q" \
                | ed -s ~/.subversion/auth/svn.simple/$md5
        echo edited $_
        break
done
]]]
Reply | Threaded
Open this post in threaded view
|

Re: A strong WTF on compiling out plaintext password support by default?!

Daniel Sahlberg
Den fre 7 aug. 2020 kl 11:34 skrev Daniel Shahaf <[hidden email]>:
It successfully adds a password to the storage, in the sense that
after running it, a subsequent `svn auth --show-passwords` shows the
password.  Still, a subsequent `svn info` doesn't use the password.
Why?  By source inspection, SVN_DISABLE_PLAINTEXT_PASSWORD_STORAGE
affects svn_auth__simple_creds_cache_set() but not svn_auth__simple_creds_cache_get(),
so why doesn't the latter use the password?

It seems you also need to set passtype = simple for svn_auth__simple_creds_cache_get() to accept the password.

Updated script, I changed to use /usr/bin/env to find zsh and explicitly set LANG to make sure svn auth return the expected text (normally I'm running sv_SE.UTF-8).

[[[
#!/usr/bin/env -S zsh -f
# Prompt for a realm and a password, then cache that password for that realm, in plaintext.
LANG=en_US.UTF-8
PS3="Enter the number of the selected option: "
creds=( "${(ps.\n\n.)"$(svn auth)"}" )
creds=( ${(M)creds:#-*} )
select m in $creds
do
        realm=${(M)${(f)m}:#Authentication realm: *}
        realm=${realm#*: }
        IFS= read -s -r pw"?Password: "
        md5=${"$(printf %s "$realm" | openssl md5)"##*= }
        print -rC1 \
                \$ i "K 8" passtype "V 6" simple "K 8" password "V ${#pw}" "$pw" "." "w" "q" \
                | ed -s ~/.subversion/auth/svn.simple/$md5
        echo edited $_
        break
done
]]]

A proper svn store-password command would be nice to better support non-X11 automated environments in case of "stupid" compile time options. But that moots the point of SVN_DISABLE_PLAINTEXT_PASSWORD_STORAGE and I understand the need for it in certain (corporate) environments, I even think it could prevent reading an already stored password. Better to convince your favorite distribution to take the approach of OpenBSD (as detailed by Stefan Sperling elsewhere in the thread).

Kind regards,
Daniel

Reply | Threaded
Open this post in threaded view
|

Re: A strong WTF on compiling out plaintext password support by default?!

Daniel Shahaf-2
Daniel Sahlberg wrote on Fri, 14 Aug 2020 23:01 +0200:

> Den fre 7 aug. 2020 kl 11:34 skrev Daniel Shahaf <[hidden email]>:
>
> > It successfully adds a password to the storage, in the sense that
> > after running it, a subsequent `svn auth --show-passwords` shows the
> > password.  Still, a subsequent `svn info` doesn't use the password.
> > Why?  By source inspection, SVN_DISABLE_PLAINTEXT_PASSWORD_STORAGE
> > affects svn_auth__simple_creds_cache_set() but not
> > svn_auth__simple_creds_cache_get(),
> > so why doesn't the latter use the password?
> >  
>
> It seems you also need to set passtype = simple for
> svn_auth__simple_creds_cache_get() to accept the password.
>

Good catch.

> Updated script, I changed to use /usr/bin/env to find zsh
> and explicitly set LANG to make sure svn auth return the expected
> text (normally I'm running sv_SE.UTF-8).

Another good catch.  Further improvements: it should set LC_ALL rather
than LANG, and the setting can be pushed into the $(…) subshell.
Furthermore, since this doesn't even try to be a POSIX script, the
«autoload -Uz _comp_locale; … $(_comp_locale; …)» idiom is also available.

> I even think [SVN_DISABLE_PLAINTEXT_PASSWORD_STORAGE] could
> prevent reading an already stored password.

So what would be done with the already-stored password?

- Deleting it would be data loss.

- Keeping it but not using it would needlessly increase the attack
  surface: Mallory might find the plaintext password, but since svn
  itself won't use it, it won't show up in audits' syscall traces and
  so on.

- [There may very well be a third option, but I haven't the time to
  think one up right now.]

Thanks for the bugfixes!

Daniel
Reply | Threaded
Open this post in threaded view
|

Re: A strong WTF on compiling out plaintext password support by default?!

Daniel Sahlberg
Den fre 14 aug. 2020 23:44Daniel Shahaf <[hidden email]> skrev:
Daniel Sahlberg wrote on Fri, 14 Aug 2020 23:01 +0200:
> Den fre 7 aug. 2020 kl 11:34 skrev Daniel Shahaf <[hidden email]>:
>
> > It successfully adds a password to the storage, in the sense that
> > after running it, a subsequent `svn auth --show-passwords` shows the
> > password.  Still, a subsequent `svn info` doesn't use the password.
> > Why?  By source inspection, SVN_DISABLE_PLAINTEXT_PASSWORD_STORAGE
> > affects svn_auth__simple_creds_cache_set() but not
> > svn_auth__simple_creds_cache_get(),
> > so why doesn't the latter use the password?
> > 
>
> It seems you also need to set passtype = simple for
> svn_auth__simple_creds_cache_get() to accept the password.
>

Good catch.

> Updated script, I changed to use /usr/bin/env to find zsh
> and explicitly set LANG to make sure svn auth return the expected
> text (normally I'm running sv_SE.UTF-8).

Another good catch.  Further improvements: it should set LC_ALL rather
than LANG, and the setting can be pushed into the $(…) subshell.
Furthermore, since this doesn't even try to be a POSIX script, the
«autoload -Uz _comp_locale; … $(_comp_locale; …)» idiom is also available.

That was way above my shell script comfort zone.. 

> I even think [SVN_DISABLE_PLAINTEXT_PASSWORD_STORAGE] could
> prevent reading an already stored password.

So what would be done with the already-stored password?

- Deleting it would be data loss.

- Keeping it but not using it would needlessly increase the attack
  surface: Mallory might find the plaintext password, but since svn
  itself won't use it, it won't show up in audits' syscall traces and
  so on.

- [There may very well be a third option, but I haven't the time to
  think one up right now.]

Good point and I agree with your analysis. No perfect solution there.

Thanks for the bugfixes!

Daniel
Reply | Threaded
Open this post in threaded view
|

Re: A strong WTF on compiling out plaintext password support by default?!

Daniel Shahaf-2
Daniel Sahlberg wrote on Sat, 15 Aug 2020 11:28 +0200:

> Den fre 14 aug. 2020 23:44Daniel Shahaf <[hidden email]> skrev:
>
> > Daniel Sahlberg wrote on Fri, 14 Aug 2020 23:01 +0200:  
> > > Den fre 7 aug. 2020 kl 11:34 skrev Daniel Shahaf <[hidden email]
> > >:
> > >  
> > > > It successfully adds a password to the storage, in the sense that
> > > > after running it, a subsequent `svn auth --show-passwords` shows the
> > > > password.  Still, a subsequent `svn info` doesn't use the password.
> > > > Why?  By source inspection, SVN_DISABLE_PLAINTEXT_PASSWORD_STORAGE
> > > > affects svn_auth__simple_creds_cache_set() but not
> > > > svn_auth__simple_creds_cache_get(),
> > > > so why doesn't the latter use the password?
> > > >  
> > >
> > > It seems you also need to set passtype = simple for
> > > svn_auth__simple_creds_cache_get() to accept the password.
> > >  
> >
> > Good catch.
> >  
> > > Updated script, I changed to use /usr/bin/env to find zsh
> > > and explicitly set LANG to make sure svn auth return the expected
> > > text (normally I'm running sv_SE.UTF-8).  
> >
> > Another good catch.  Further improvements: it should set LC_ALL rather
> > than LANG, and the setting can be pushed into the $(…) subshell.
> > Furthermore, since this doesn't even try to be a POSIX script, the
> > «autoload -Uz _comp_locale; … $(_comp_locale; …)» idiom is also available.
> >  
>
> That was way above my shell script comfort zone..

For completeness, I'm attaching the script again with that (trivial)
change made.

FWIW, I wouldn't usually have used zsh for code to be posted to this
list, since that language is spoken by few people here and isn't always
self-explanatory.  That's one reason I described this script as
a "prototype".  It should be easy to port this script to any other
language; what it does is:

1. Run `svn auth` with "LC_ALL=C" in the environment.

2. Split the output on empty lines ("\n\n").  This produces an array.

3. Remove the last element of the array (by pattern matching, but that's
the effect).

4. Prompt the user to choose an element of the array.  This selects
a specific authn realm.

5. Prompt the user for the corresponding password.

6. Compute the md5 of the realm string, without a trailing newline.

7. Insert two key-value pairs to the serialized hash [see
svn_hash_write2()] in ~/.subversion/auth/ for the realm in question.
That uses ed(1) because the file format has a fixed trailer string.

This design means the script is only able to cache passwords for realms
for which a username is already cached.

Cheers,

Daniel


[[[
#!/usr/bin/env -S zsh -f
# Prompt for a realm and a password, then cache that password for that realm, in plaintext.
PS3="Enter the number of the selected option: "
creds=( "${(ps.\n\n.)"$(LC_ALL=C svn auth)"}" )
creds=( ${(M)creds:#-*} )
select m in $creds
do
        realm=${(M)${(f)m}:#Authentication realm: *}  
        realm=${realm#*: }
        IFS= read -s -r pw"?Password: "
        md5=${"$(printf %s "$realm" | openssl md5)"##*= }
        print -rC1 \
                \$ i \
                "K 8" passtype "V 6" simple \
                "K 8" password "V ${#pw}" "$pw" \
                "." \
                "w" \
                "q" \
                | ed -s ~/.subversion/auth/svn.simple/$md5
        echo edited $_
        break
done
]]]