You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2022/01/21 23:38:58 UTC
Re: [PROPOSAL] Allow plaintext passwords again.
On 21 Jan 2022, Mark Phippard wrote:
>In terms of what needs to be done, maybe I am wrong, but I did
>not
>think we had any mechanism in place where someone could choose
>not to
>compile in support for this feature. So that is new code that
>would
>need to be added.
Well:
------------------------------------------------------------------------
r1845377 | brane | 2018-10-31 14:40:21 -0500 (Wed, 31 Oct 2018)
| 6 lines Changed paths:
M /subversion/trunk/configure.ac
Disable plaintext password storage by default. It can still be
enabled at configure time. * configure.ac: Invert the default
of the plaintext-password-storage
option and update its help text.
------------------------------------------------------------------------
:-)
>1) I think there should be an easy way to know if the support
>exists
>or not. I am thinking "svn --version" maybe prints out if
>plaintext is
>available? So an admin could run this command and would look to
>confirm they do NOT see that in the output? Maybe this already
>exists?
As Nathan Hartman pointed out in his reply, we already do this (I
wasn't aware of it either until Nathan's mail, by the way!).
>2) If we have to add a new compile option, then I suggest we go
>all
>the way and also close the backdoor that exists. IOW, if svn is
>compiled without plaintext support then it also should not be
>able to
>read an existing stored plaintext credential.
That was a deliberate compatibility move, and I'm not sure we
should change it. Can you describe the harm that would come from
keeping that behavior vs changing it as you describe above? I
guess I don't see how it's a "backdoor".
Best regards,
-Karl
Re: [VOTE] Reverting r1845377 (Was: [PROPOSAL] Allow plaintext passwords again.)
Posted by Mark Phippard <ma...@gmail.com>.
On Sat, Apr 22, 2023 at 4:30 AM Branko Čibej <br...@apache.org> wrote:
>
> On 22.04.2023 10:27, Branko Čibej wrote:
>
> On 21.04.2023 16:43, Johan Corveleyn wrote:
>
>
> My plan is to revert r1845377 during next weekend. For the first bulletpoint nothing has to be done, but if consensus changes during the week, I can do the work to to implement option 1. For the second bullet point I'd like to reach consensus (on the short- and long term plan) by the same time, then if we decide on 2/3 we will put it on the wish list for a future release.
>
> Great!
+1. great to see progress on this. Getting the change reverted should
be the priority. If someone volunteers to take up some of the
additional ideas even better but we do not need to block on them.
> There was one additional suggestion: obfuscate plaintext passwords, or
> encrypt them with a key embedded in the code (or even with a key
> supplied at compile-time).
>
>
> There's actually a branch for that, but the feature never made it to trunk.
>
>
> I'll just add that the intent of that branch is to properly encrypt passwords at least on Linux, not to obfuscate them or embed a key in the code. The latter is worse than storing in plain text, because the effect is the same but it adds a veneer of false sense of security.
Feedback we had from users did not agree with this. They would have
seen something as simple as Base64 encoding the string as an
improvement. I believe there is a way to implement this without
creating the impression that the string is cryptographically secure.
For starters, you could just name the property in the file something
like "obfuscatedPassword" or "base64Password" to make it clear.
Kubernetes has used the term "Opaque" for its Base64 encoded but
otherwise plaintext "secrets".
Even if we did use encryption and stored the key in the same file it
would still be better than having a plain text string.
I would agree that we should not suggest that the password is "secure"
if we do this but we have let this argument be the enemy of making
progress for nearly 2 decades when lots of users have said this would
be an improvement for them.
Mark
Re: [VOTE] Reverting r1845377 (Was: [PROPOSAL] Allow plaintext passwords again.)
Posted by Daniel Sahlberg <da...@gmail.com>.
Den lör 22 apr. 2023 kl 10:30 skrev Branko Čibej <br...@apache.org>:
> On 22.04.2023 10:27, Branko Čibej wrote:
>
> On 21.04.2023 16:43, Johan Corveleyn wrote:
>
>
> My plan is to revert r1845377 during next weekend. For the first bulletpoint nothing has to be done, but if consensus changes during the week, I can do the work to to implement option 1. For the second bullet point I'd like to reach consensus (on the short- and long term plan) by the same time, then if we decide on 2/3 we will put it on the wish list for a future release.
>
> Great!
>
> There was one additional suggestion: obfuscate plaintext passwords, or
> encrypt them with a key embedded in the code (or even with a key
> supplied at compile-time).
>
>
> There's actually a branch for that, but the feature never made it to trunk.
>
>
> I'll just add that the intent of that branch is to properly encrypt
> passwords at least on Linux, not to obfuscate them or embed a key in the
> code. The latter is worse than storing in plain text, because the effect is
> the same but it adds a veneer of false sense of security.
>
I assume you are referring to the master-password branch? That will not
help with the use case some users are having - ie unattended svn actions -
since the user will have to enter the master password instead of the
password. (The use case "not remembering a lot of different passwords while
not storing passwords in clear" is of course solved by a master password).
--- thread reply ---
As you have probably seen, I've committed the revert as r1909351.
I have not made up my mind regarding the other points, but as pointed out
by several others, let's start with reverting and then take it from there.
I will read through all e-mails once more and see if i can sumarize the
discussion somewhat. So far I couldn't see any clear direction.
Kind regards,
Daniel
Re: [VOTE] Reverting r1845377 (Was: [PROPOSAL] Allow plaintext passwords again.)
Posted by Branko Čibej <br...@apache.org>.
On 22.04.2023 10:27, Branko Čibej wrote:
> On 21.04.2023 16:43, Johan Corveleyn wrote:
>>
>>> My plan is to revert r1845377 during next weekend. For the first bulletpoint nothing has to be done, but if consensus changes during the week, I can do the work to to implement option 1. For the second bullet point I'd like to reach consensus (on the short- and long term plan) by the same time, then if we decide on 2/3 we will put it on the wish list for a future release.
>> Great!
>>
>> There was one additional suggestion: obfuscate plaintext passwords, or
>> encrypt them with a key embedded in the code (or even with a key
>> supplied at compile-time).
>
> There's actually a branch for that, but the feature never made it to
> trunk.
I'll just add that the intent of that branch is to properly encrypt
passwords at least on Linux, not to obfuscate them or embed a key in the
code. The latter is worse than storing in plain text, because the effect
is the same but it adds a veneer of false sense of security.
-- Brane
Re: [VOTE] Reverting r1845377 (Was: [PROPOSAL] Allow plaintext passwords again.)
Posted by Branko Čibej <br...@apache.org>.
On 21.04.2023 16:43, Johan Corveleyn wrote:
>
>> My plan is to revert r1845377 during next weekend. For the first bulletpoint nothing has to be done, but if consensus changes during the week, I can do the work to to implement option 1. For the second bullet point I'd like to reach consensus (on the short- and long term plan) by the same time, then if we decide on 2/3 we will put it on the wish list for a future release.
> Great!
>
> There was one additional suggestion: obfuscate plaintext passwords, or
> encrypt them with a key embedded in the code (or even with a key
> supplied at compile-time).
There's actually a branch for that, but the feature never made it to trunk.
Re: [VOTE] Reverting r1845377 (Was: [PROPOSAL] Allow plaintext passwords again.)
Posted by Johan Corveleyn <jc...@gmail.com>.
On Sun, Apr 16, 2023 at 11:19 PM Daniel Sahlberg
<da...@gmail.com> wrote:
>
> The dicussion died again, but this time I intend make sure we complete it once and for all. I've marked the subject as VOTE to hopefully get some attention, although I believe votes have already been cast.
Thanks for picking it up once again :-). Let's hope we can land this now ...
> In my mind, it seems we have consensus to revert r1845377 (+1 from Nathan Hartman, Evgeny Kotkov, Johan Corveleyn, myself, I'm also considering Karl Fogel to have voted for this by making the initial proposition and volunteering to do it. No objections).
Indeed, +1 from me.
> Then we have a two other suggestions:
>
> * Changing the default value of store-plaintext-passwords
> Option 1 - set the default value of store-plaintext-passwords to no. For a non-edited servers config file, this will put the behaviour of 1.15 in line with 1.12-1.14.
> Option 2 - keep store-plaintext-passwords = ask. For a non-edited servers config file, the behaviour of 1.15 will be in line with pre-1.12.
>
> I have previously expressed support (but no formal vote, and I will not stand in the way of consensus) for option 1, since I was under the impression that it was a longterm goal go disable the plaintext password store (I have previously commented on how quickly r1845377 was discussed and implemented). Nathan has argued that it might give a bad user experience if credentials are not stored even though plaintext storage is enabled in the build options. We can improve on this by updating the svn --version output to explicitly say if plaintext cache is build but runtime disabled.
>
> If I'm counting correctly, Nathan, Evgeny and Johan has expressed +1 for option 2.
>
> Considering this, I conclude we have consensus for option 2.
>
>
> * Changing handling of already stored plaintext passwords
> This was discussed in 2022 and there were some suggestions that Subversion should not even use plaintext passwords even if they were found in the plaintext password store (initially suggested by Mark Phippard as a way to soften the impact of reverting r1845377). I'm proposing we choose amoung one of the following three options:
>
> Option 1 - do nothing. Keep using the stored passwords.
> Option 2 - add a new runtime config option dont-use-plaintext-passwords [default no] global overrides local.
> Option 3 - new compile time option to disable reading/using plaintext passwords.
Why a new compile-time option for disabling reading? Why not keep it
simple, and disable the reading of plaintext passwords if the
compile-time option that will be re-introduced by reverting r1845377
is supplied (--disable-plaintext-password-storage)? I think that is
what Mark was suggesting, and I still like it because it's simple and
doesn't add more knobs than necessary.
So maybe:
Option 4 - disable reading/using plaintext passwords if compile-time
/storing/ of plaintext-passwords is configured
(--disable-plaintext-password-storage).
>
> Options 2 and 3 should probably imply disabling storing plaintext passwords as well. I think they should also warn if the code finds a stored password and suggest the user to delete it using svn auth --remove. (It was hinted previously that we would disable the code that searches for the files storing the password, however the same files also store the username and we read them as an apr_hash_t so we don't really have the option to "not read the password").
>
> With option 2 or 3 we let a security consious organisation configure their system to their liking. I would love to have them, but I can't avoid the feeling that it is security theater: As far as I can tell it is not possible to avoid that a user can upload their own version of the svn binary and then all bets are off anyway. (On Windows and MacOS it is possible to only allow execution of signed binaries, and they don't even store the passwords in plaintext anyway).
>
> If we go with either 2 or 3, then I'm perfering option 2 since it will allow the administrators to set this without compiling their own version (which seems to be a major obstacle, considering the reaction to r1845377). I believe Karl Fogel and Mark was also leaning towards doing something during the discussion in 2022.
>
> Johan seems to believe option 1 is fine ("these additional \"mitigations\" are not absolutely required").
>
> In my mind, it is perfectly acceptable to vote +1 on both option 1 AND one of option 2/3. I would interpret that as "do nothing in the short term, but do X in the long term".
Agreed. So option 1 follows automatically from reverting r1845377; and
option 2, 3 or 4 can be done later.
>
> My plan is to revert r1845377 during next weekend. For the first bulletpoint nothing has to be done, but if consensus changes during the week, I can do the work to to implement option 1. For the second bullet point I'd like to reach consensus (on the short- and long term plan) by the same time, then if we decide on 2/3 we will put it on the wish list for a future release.
Great!
There was one additional suggestion: obfuscate plaintext passwords, or
encrypt them with a key embedded in the code (or even with a key
supplied at compile-time). Though that'd be nice, it might not be as
trivial as it seems at first sight (keeping in mind that old and new
clients might be mixed on someone's system, so we can't just put an
obfuscated password in the "simple" passtype). I wouldn't want this to
block the simple revert of r1845377.
--
Johan
[VOTE] Reverting r1845377 (Was: [PROPOSAL] Allow plaintext passwords again.)
Posted by Daniel Sahlberg <da...@gmail.com>.
The dicussion died again, but this time I intend make sure we complete it
once and for all. I've marked the subject as VOTE to hopefully get some
attention, although I believe votes have already been cast.
In my mind, it seems we have consensus to revert r1845377 (+1 from Nathan
Hartman, Evgeny Kotkov, Johan Corveleyn, myself, I'm also considering Karl
Fogel to have voted for this by making the initial proposition and
volunteering to do it. No objections).
Then we have a two other suggestions:
* Changing the default value of store-plaintext-passwords
Option 1 - set the default value of store-plaintext-passwords to no. For a
non-edited servers config file, this will put the behaviour of 1.15 in line
with 1.12-1.14.
Option 2 - keep store-plaintext-passwords = ask. For a non-edited servers
config file, the behaviour of 1.15 will be in line with pre-1.12.
I have previously expressed support (but no formal vote, and I will not
stand in the way of consensus) for option 1, since I was under the
impression that it was a longterm goal go disable the plaintext password
store (I have previously commented on how quickly r1845377 was discussed
and implemented). Nathan has argued that it might give a bad user
experience if credentials are not stored even though plaintext storage is
enabled in the build options. We can improve on this by updating the svn
--version output to explicitly say if plaintext cache is build but runtime
disabled.
If I'm counting correctly, Nathan, Evgeny and Johan has expressed +1 for
option 2.
Considering this, I conclude we have consensus for option 2.
* Changing handling of already stored plaintext passwords
This was discussed in 2022 and there were some suggestions that Subversion
should not even use plaintext passwords even if they were found in the
plaintext password store (initially suggested by Mark Phippard as a way to
soften the impact of reverting r1845377). I'm proposing we choose amoung
one of the following three options:
Option 1 - do nothing. Keep using the stored passwords.
Option 2 - add a new runtime config option dont-use-plaintext-passwords
[default no] global overrides local.
Option 3 - new compile time option to disable reading/using plaintext
passwords.
Options 2 and 3 should probably imply disabling storing plaintext passwords
as well. I think they should also warn if the code finds a stored password
and suggest the user to delete it using svn auth --remove. (It was hinted
previously that we would disable the code that searches for the files
storing the password, however the same files also store the username and we
read them as an apr_hash_t so we don't really have the option to "not read
the password").
With option 2 or 3 we let a security consious organisation configure their
system to their liking. I would love to have them, but I can't avoid the
feeling that it is security theater: As far as I can tell it is not
possible to avoid that a user can upload their own version of the svn
binary and then all bets are off anyway. (On Windows and MacOS it is
possible to only allow execution of signed binaries, and they don't even
store the passwords in plaintext anyway).
If we go with either 2 or 3, then I'm perfering option 2 since it will
allow the administrators to set this without compiling their own version
(which seems to be a major obstacle, considering the reaction to r1845377).
I believe Karl Fogel and Mark was also leaning towards doing something
during the discussion in 2022.
Johan seems to believe option 1 is fine ("these additional \"mitigations\"
are not absolutely required").
In my mind, it is perfectly acceptable to vote +1 on both option 1 AND one
of option 2/3. I would interpret that as "do nothing in the short term, but
do X in the long term".
My plan is to revert r1845377 during next weekend. For the first
bulletpoint nothing has to be done, but if consensus changes during the
week, I can do the work to to implement option 1. For the second bullet
point I'd like to reach consensus (on the short- and long term plan) by the
same time, then if we decide on 2/3 we will put it on the wish list for a
future release.
Kind regards,
Daniel Sahlberg
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Nathan Hartman <ha...@gmail.com>.
On Thu, Mar 30, 2023 at 8:39 AM Johan Corveleyn <jc...@gmail.com> wrote:
> Basically this would correspond to kfogel's proposal earlier in this
> thread [1] (and the one most participants agreed with):
>
> "I think it's just a matter of reverting r1845377, right? (And
> updating CHANGES, etc.)"
>
> For completeness, I want to quickly summarize two additional
> suggestions made by Mark Phippard [2][3]:
>
> - If plaintext-pwd-caching support is compiled out (by explicitly
> giving the compile-time flag for this), also stop reading already
> cached ones. This would render Daniel's python script useless in these
> environments. It might satisfy some security sensitive people who
> would regard the script as a way to "circumvent" the
> plaintext-disablement. It would make the "plaintext-disabling" more of
> a complete feature. Additionally, it was suggested that svn could warn
> or erase such legacy plaintext-cached passwords (instead of just
> ignoring them), as yet another mitigation improvement.
If the plaintext cache is explicitly disabled, I think it is
appropriate to stop reading plaintext passwords already cached AND
warn about the presence of previously cached passwords.
Both of those things should go together; in other words, if we no
longer can read previously cached plaintext passwords, then we should
warn users if they exist to avoid unknowingly keeping them around.
I don't think we should automatically erase passwords; IMO that should
remain the user's decision.
To be clear, I'm describing the scenario where the plaintext cache was
explicitly disabled at compile time. Otherwise, the plaintext cache is
fully functional and we don't issue the warning.
> - Apply some obfuscation or encryption with a key hidden in the code
> (or even supplied by a compile time option) to the plaintext
> passwords. This helps against non-malicious exposure, and makes it a
> tad harder for simple scripts to extract the password (which might
> appease some part of our user base). This might break legitimate
> scripts that explicitly (mis-)use the cached password for other
> purposes, though we could regard such uses as hacks that we are
> allowed to break.
I like this idea because it provides a simple self-contained middle
ground between "no caching without strong encryption ever" and "but
that prevents automated tasks, usage through ssh, etc".
As you pointed out earlier, the first suggestion renders DSahlberg's
python script useless in environments where the plaintext cache is
disabled. Perhaps, then, it could be modified to convert password
caches from plaintext to the new obfuscated format. There could be a
command line option to delete the original plaintext ones if
conversion succeeded. (Also, it should retain the ability to store in
plaintext, since 1.12 through 1.14 clients will still need this.)
(It doesn't escape me that people could use the code in the script as
an example of how to read the obfuscated cache but then they could use
the code in Subversion itself for that. Also, if a custom key can be
provided at compile time, that avoids much of the trouble.)
> IMHO these additional "mitigations" are not absolutely required (I
> mainly would like to see r1845377 reverted), but if some people feel
> strongly about them ... sure, why not (though in that case we'd need
> someone to put in the work too).
Although that is a challenge, it does help to know exactly what we
want as a project before anyone puts in that work. :-)
Cheers,
Nathan
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Johan Corveleyn <jc...@gmail.com>.
On Thu, Mar 30, 2023 at 12:15 AM Nathan Hartman
<ha...@gmail.com> wrote:
>
> On Wed, Mar 29, 2023 at 6:02 PM Evgeny Kotkov
> <ev...@visualsvn.com> wrote:
> >
> > Nathan Hartman <ha...@gmail.com> writes:
> >
> > > 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.
> >
> > +1.
+1
> > > * 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?)
> >
> > So, apparently, the current default is "ask".
> >
> > I haven't checked all the details, but I think that defaulting to "ask"
> > already makes the user decision explicit and allows it to happen naturally,
> > without requiring any additional instructions or knowledge.
> >
> > If we change the default to "no", this part of the experience could be worse,
> > because for the end users it might look like the credentials aren't being
> > stored for unknown reasons / a bug in the software.
>
> Ah, this makes sense. In that case, I'm +1 to leave it as "ask" (no change).
+1
Basically this would correspond to kfogel's proposal earlier in this
thread [1] (and the one most participants agreed with):
"I think it's just a matter of reverting r1845377, right? (And
updating CHANGES, etc.)"
For completeness, I want to quickly summarize two additional
suggestions made by Mark Phippard [2][3]:
- If plaintext-pwd-caching support is compiled out (by explicitly
giving the compile-time flag for this), also stop reading already
cached ones. This would render Daniel's python script useless in these
environments. It might satisfy some security sensitive people who
would regard the script as a way to "circumvent" the
plaintext-disablement. It would make the "plaintext-disabling" more of
a complete feature. Additionally, it was suggested that svn could warn
or erase such legacy plaintext-cached passwords (instead of just
ignoring them), as yet another mitigation improvement.
- Apply some obfuscation or encryption with a key hidden in the code
(or even supplied by a compile time option) to the plaintext
passwords. This helps against non-malicious exposure, and makes it a
tad harder for simple scripts to extract the password (which might
appease some part of our user base). This might break legitimate
scripts that explicitly (mis-)use the cached password for other
purposes, though we could regard such uses as hacks that we are
allowed to break.
IMHO these additional "mitigations" are not absolutely required (I
mainly would like to see r1845377 reverted), but if some people feel
strongly about them ... sure, why not (though in that case we'd need
someone to put in the work too).
[1] https://lists.apache.org/thread/shzxh04l493qnj8pdt8vl0x4gkjrkvcy
[2] https://lists.apache.org/thread/1tfny40nokqf6p6nll30p06t8or2c8hm
[3] https://lists.apache.org/thread/p2vn6foj8qz3lfvdl70bs62vg5krcgr7
--
Johan
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Mar 29, 2023 at 6:02 PM Evgeny Kotkov
<ev...@visualsvn.com> wrote:
>
> Nathan Hartman <ha...@gmail.com> writes:
>
> > 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.
>
> +1.
>
> > * 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?)
>
> So, apparently, the current default is "ask".
>
> I haven't checked all the details, but I think that defaulting to "ask"
> already makes the user decision explicit and allows it to happen naturally,
> without requiring any additional instructions or knowledge.
>
> If we change the default to "no", this part of the experience could be worse,
> because for the end users it might look like the credentials aren't being
> stored for unknown reasons / a bug in the software.
Ah, this makes sense. In that case, I'm +1 to leave it as "ask" (no change).
Cheers,
Nathan
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Evgeny Kotkov via dev <de...@subversion.apache.org>.
Nathan Hartman <ha...@gmail.com> writes:
> 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.
+1.
> * 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?)
So, apparently, the current default is "ask".
I haven't checked all the details, but I think that defaulting to "ask"
already makes the user decision explicit and allows it to happen naturally,
without requiring any additional instructions or knowledge.
If we change the default to "no", this part of the experience could be worse,
because for the end users it might look like the credentials aren't being
stored for unknown reasons / a bug in the software.
Thanks,
Evgeny Kotkov
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Daniel Sahlberg <da...@gmail.com>.
Den tis 28 mars 2023 kl 18:56 skrev Nathan Hartman <hartman.nathan@gmail.com
>:
> On Tue, Mar 28, 2023 at 10:35 AM Daniel Sahlberg
> <da...@gmail.com> wrote:
> > [...] 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.
>
[...]
* 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?)
>
If I read the code correctly (subversion/libsvn_subr/auth.c) this is
controlled by store-plaintext-passwords in the servers file. The default is
SVN_CONFIG_DEFAULT_OPTION_STORE_PLAINTEXT_PASSWORDS, defined
in subversion/include/svn_config.h to SVN_CONFIG_ASK.
In the default servers file (see svn_config_ensure in
subversion/libsvn_subr/config.c), the default config file has never had a
setting for store-plaintext-passwords. If the user has not edited the
servers file, we can change the default to no (probably FALSE, in
svn_config.h) and have the same behaviour in 1.15 as in 1.12 to 1.14 even
if we enable storing plaintext passwords compile-time.
If someone has edited the servers file, I believe we have three cases (for
default compile-time options):
store-plaintext-passwords=yes: In pre-1.12: storing passwords without
asking, 1.12 to 1.14: not storing/not asking, 1.15-later: storing passwords
without asking.
store-plaintext-passwords=no: In all versions: not storing/not asking.
store-plaintext-passwords=ask: In pre-1.12: asking about storing passwords,
1.12 to 1.14: not storing/not asking, 1.15-later: asking about storing
passwords.
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.
>
I'm sure we can come up with some cleaver wording in the FAQ, but good
thing you brought this up so we don't forget.
I would argue that since Subversion is open source, it is very easy for a
user to compile their own version of Subversion and run it, so the
administrators have very little options to control this anyway (unless it
is possible to limit the executables that are possible to run on a machine
- in Windows it would be possible with a group policy but I don't know what
can be done in SELinux, much less in the different Unix flavours).
As argued by Danielsh in the other threads, it is easy to pass the password
on the command line or to write scripts to automatically answer the
password prompt. So any administrator efforts to limit this will have
little effect anyway.
Kind regards,
Daniel
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Nathan Hartman <ha...@gmail.com>.
On Tue, Mar 28, 2023 at 10:35 AM Daniel Sahlberg
<da...@gmail.com> wrote:
>
> Den tis 28 mars 2023 kl 14:41 skrev Johan Corveleyn <jc...@gmail.com>:
>>
>> On Mon, Jan 24, 2022 at 5:02 PM Mark Phippard <ma...@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
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Daniel Sahlberg <da...@gmail.com>.
Den tis 28 mars 2023 kl 14:41 skrev Johan Corveleyn <jc...@gmail.com>:
> On Mon, Jan 24, 2022 at 5:02 PM Mark Phippard <ma...@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
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).
Kind regards,
Daniel
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Johan Corveleyn <jc...@gmail.com>.
On Mon, Jan 24, 2022 at 5:02 PM Mark Phippard <ma...@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
--
Johan
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Mark Phippard <ma...@gmail.com>.
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.
Mark
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Mark Phippard wrote on Fri, Jan 21, 2022 at 20:29:21 -0500:
> On Fri, Jan 21, 2022 at 7:22 PM Karl Fogel <kf...@red-bean.com> wrote:
> >
> > On 21 Jan 2022, Mark Phippard wrote:
> > >One aspect of the previous thread that came up is that someone
> > >demonstrated a simple script to create a cached password (as a
> > >workaround for current users). That is what led to the idea of
> > >formalizing this using the svn auth command to create this file.
> > >
> > >I am the only one calling this a backdoor. I am saying that if I am
> > >an admin that does not want plaintext passwords being cached and
> > >you then create a simple way to do exactly that, then that is a
> > >backdoor around the policy I wanted.
There's any number of other ways to auto-answer password prompts:
- Build a custom client
[It's easy, especially considering that many of our users happen to be
programmers.]
- Use the --password option
[which would be visible in ps(1) output without needing local root,
unlike the content of ~/.subversion/auth/]
- Use wrapper tools that will run $SHELL in a pty and auto-answer
password prompts
[They exist]
- Map some key or mouse button to one's password
[and then forget to chmod one's ~/.xmodmaprc restrictively]
- Run 1.14
[can be prevented server-side by checking the User-Agent string]
- Set the password to adjacent home row keys, so it's easy to type
[can be prevented by the admin]
- Use kwallet/gnome-keyring with an empty or easy-to-guess master password
And conversely, there might be encryption at or below the OS filesystem
level, in which case plaintext password caching is pretty much comparable
to the alternative caching options.
> > >Maybe it is not the right term to use here.
I'd say "circumvent". "Backdoor" specifically refers to bypassing
authz.
> > >I am just saying if we are going to make someone compile their own
> > >binaries we might as well at least give them what they want.
There'll always be ways to feed passwords to svn(1) automatically. The
question is just how difficult it would be to do so.
More below.
> > >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?
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).
>
> That said, I do not have a strong opinion on this one.
Neither do I :)
Cheers,
Daniel
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Jan 21, 2022 at 7:22 PM Karl Fogel <kf...@red-bean.com> wrote:
>
> On 21 Jan 2022, Mark Phippard wrote:
> >One aspect of the previous thread that came up is that someone
> >demonstrated a simple script to create a cached password (as a
> >workaround for current users). That is what led to the idea of
> >formalizing this using the svn auth command to create this file.
> >
> >I am the only one calling this a backdoor. I am saying that if I
> >am an
> >admin that does not want plaintext passwords being cached and you
> >then
> >create a simple way to do exactly that, then that is a backdoor
> >around
> >the policy I wanted. Maybe it is not the right term to use
> >here. I am
> >just saying if we are going to make someone compile their own
> >binaries
> >we might as well at least give them what they want.
> >
> >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.
That said, I do not have a strong opinion on this one.
Mark
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Karl Fogel <kf...@red-bean.com>.
On 21 Jan 2022, Mark Phippard wrote:
>One aspect of the previous thread that came up is that someone
>demonstrated a simple script to create a cached password (as a
>workaround for current users). That is what led to the idea of
>formalizing this using the svn auth command to create this file.
>
>I am the only one calling this a backdoor. I am saying that if I
>am an
>admin that does not want plaintext passwords being cached and you
>then
>create a simple way to do exactly that, then that is a backdoor
>around
>the policy I wanted. Maybe it is not the right term to use
>here. I am
>just saying if we are going to make someone compile their own
>binaries
>we might as well at least give them what they want.
>
>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?
Best regards,
-Karl
Re: [PROPOSAL] Allow plaintext passwords again.
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Jan 21, 2022 at 6:39 PM Karl Fogel <kf...@red-bean.com> wrote:
> >2) If we have to add a new compile option, then I suggest we go
> >all
> >the way and also close the backdoor that exists. IOW, if svn is
> >compiled without plaintext support then it also should not be
> >able to
> >read an existing stored plaintext credential.
>
> That was a deliberate compatibility move, and I'm not sure we
> should change it. Can you describe the harm that would come from
> keeping that behavior vs changing it as you describe above? I
> guess I don't see how it's a "backdoor".
One aspect of the previous thread that came up is that someone
demonstrated a simple script to create a cached password (as a
workaround for current users). That is what led to the idea of
formalizing this using the svn auth command to create this file.
I am the only one calling this a backdoor. I am saying that if I am an
admin that does not want plaintext passwords being cached and you then
create a simple way to do exactly that, then that is a backdoor around
the policy I wanted. Maybe it is not the right term to use here. I am
just saying if we are going to make someone compile their own binaries
we might as well at least give them what they want.
I return to my "two camps" argument. The people that do not want
plaintext passwords to be cached ... do not want them being cached.
Mark