On Tue, Mar 28, 2023 at 10:35 AM Daniel Sahlberg
<daniel.l.sahlb...@gmail.com> wrote:
>
> Den tis 28 mars 2023 kl 14:41 skrev Johan Corveleyn <jcor...@gmail.com>:
>>
>> On Mon, Jan 24, 2022 at 5:02 PM Mark Phippard <markp...@gmail.com> wrote:
>> >
>> > On Mon, Jan 24, 2022 at 10:44 AM Daniel Shahaf <d...@daniel.shahaf.name> 
>> > wrote:
>> >
>> > > > > >I return to my "two camps" argument. The people that do not want
>> > > > > >plaintext passwords to be cached ... do not want them being
>> > > > > >cached.
>> > > > >
>> > > > > I see what you mean.
>> > > > >
>> > > > > If svn is compiled to not cache passwords, but a legacy cached
>> > > > > password exists on disk for a given repository, should svn not
>> > > > > only not read it but actually warn the user that the cached
>> > > > > password exists?
>> > > >
>> > > > IMO, it is not necessary and if a compiler option disables the code
>> > > > then I would envision we do not even have any code running that is
>> > > > looking for those files to give the warning.
>> > >
>> > > Plaintext passwords are saved in the "password" element of the
>> > > serialized hash in ~/.subversion/auth/svn.simple/.
>> > >
>> > > Those are the same files that cache the username when the username is
>> > > cached without its password.
>> > >
>> > > We can't know whether a file contains a password or not until we have
>> > > opened, read, and parsed it.
>> > >
>> > > So, "not even have any code running that is looking for those files"
>> > > isn't an option (unless we're willing to throw away cached usernames
>> > > even if they were cached without a password)
>> > >
>> > > So, what should we do if we have parsed one of those files and the
>> > > resulting apr_hash_t contains a "password" key?
>> > >
>> > > Ignore it?  Erase it (memset() it to zero)?  Warn about it?  Use it?
>> >
>> > Good points and excellent questions. If we would already be
>> > discovering this file then I suppose we could do something. I would be
>> > fine with just ignoring the cached password but some kind of other
>> > option would also be good.
>> >
>> >
>> > > And for that matter, should there be a configure option that disables
>> > > the --password command-line option?  It, too, can be used insecurely
>> > > (see above about filesystem-level encryption).
>> >
>> > Also a good question. A configure option to disable this might be
>> > appreciated by some users.
>>
>> Is this issue on someone's radar? It seems the discussion died out
>> here, and I can't find anything further. Maybe worth taking another
>> look now that we're getting closer to 1.15?
>>
>> We seemed to get stuck "finding consensus on desired behaviour".
>> Various proposals were made, but none got over the "bar of
>> objections", and we ran out of steam. Which leaves us with the status
>> quo, however imperfect it is :-(.
>>
>> (This recently came up in my company, when we were looking at
>> upgrading the svn client on a unix build machine -- oops can't upgrade
>> past SVN 1.12 or so, because of the compiled-out plain-text pwd
>> caching support)
>>
>> For some background (warning long read):
>>
>> https://lists.apache.org/thread/b6g2hx2m3s117wcmno08opl874ons3q8
>> https://lists.apache.org/thread/shzxh04l493qnj8pdt8vl0x4gkjrkvcy
>
>
> One outcome was that I wrote a script (based on suggestions by danielsh) to 
> store plaintext passwords. It is linked from the website:
> https://subversion.apache.org/faq.html#plaintext-passwords

I was going to point that out as well: the Python script might provide
a reasonable workaround. Especially if we re-enable the plaintext
cache in the default build for 1.15+, the workaround would be needed
only temporarily.

More below:

> And the script can be found here:
> https://svn.apache.org/repos/asf/subversion/trunk/tools/client-side/store-plaintext-password.py
>
> Hopefully the script will help you at least some part of the way.
>
> Stefan Sperling has previously described how OpenBSD explicitly compile SVN 
> /with/ support for caching plaintext passwords but disable it in the global 
> runtime config. That sounds like a good option to me - reverting the previous 
> change and changing the default config. I don't know (didn't check, no time) 
> what the default config is right now and if it can be interpreted as "don't 
> cache passwords" for old clients being updated. One downside is that it might 
> be difficult for administrators to enforce a policy (ie, "don't allow users 
> to store a password") but since SVN uses existing passwords, such policy is 
> easily circumvented (create the auth cache file on another machine and copy 
> it manually).

Or they could use the above-mentioned script.

We have gotten many complaints about this change causing problems for
users, especially with automation or when accessing a remote SVN
client over SSH (the auth GUI dialog box appears on the remote
machine, inaccessible to the user).

I appreciate that if we reverse the change, we may get complaints from
the other camp.

I think a good middle ground is:

* Build with --enable-plaintext-password-storage by default; users who
  want to harden their system can do so, but will need to build their
  own client.

* Set the default run-time config to store-plaintext-passwords = no
  (if it isn't already; haven't checked) and instruct users on how to
  change it. This makes the decision to store in plaintext an explicit
  one that the user can opt into. (I appreciate that this could be
  changed without the user's knowledge; perhaps the systemwide config
  should always take precedence over the user-controlled one for this
  setting?)

* (For extra credit) Encourage packagers to create a second package
  without plaintext support, so users can choose which to install and
  avoid a custom build. For some consistency across package managers,
  we could suggest a name for the package if we can come up with one.

Disadvantages to making the above reversal: The FAQ entry will become
more complicated; possible complaints from those who prefer that the
client not support plaintext cache writing at all.

Just for completeness, I'll note that this not an issue on Windows or
macOS; both have OS facilities for caching passwords. It affects only
Unix; in case of desktop Unix machines that have GNOME Keyring and/or
KWallet (with support in the SVN client), this change should have
minimal impact for users, since those will receive priority over the
plaintext cache.

Nathan

Reply via email to