On Thu, Mar 30, 2023 at 8:39 AM Johan Corveleyn <jcor...@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
