Hi Peff, On Sun, 19 May 2019, Jeff King wrote: > On Fri, May 17, 2019 at 06:20:31PM -0400, Jeff King wrote: > > > What if we did this: > > > > 1. Do not ever write the password part of a URL into config. > > > > 2. When we extract the user/pass out of a URL, put them into the > > credential struct, so that when we successfully authenticate, we > > trigger storage. This _might_ actually happen already, but we > > should definitely confirm it. > > > > 3. If the user has no credential helper defined, then do one of: > > > > a. Warn them that the credential was not recorded, but that they > > can use "git clone -c credential.helper=store" as a fallback > > (but probably worded in a way to recommend using something > > stronger if possible). > > > > This is slightly annoying because following the advice requires > > re-cloning. Fixing it up from there is more like: > > > > git config credential.helper store > > git fetch > > [interactively input password] > > > > b. Just use credential-store. Optionally notify them of what > > happened (and that they might want to choose a better helper). > > So here are patches to do that. Step 2 is indeed how things work > already, so nothing was needed there (there are still 3 patches because > there was a bit of prep-work ;) ). Thank you for working on this!!! > I did 3b here: automagically enabling credential-store. I'm still on the > fence on whether that's a good idea or not. I think you're right, it is a good idea. It is built by default (because it has no 3rd-party dependencies), and it should be "safe enough". I'll just have to look into mapping the Unix-y `chmod()` to a Windows-appropriate ACL action, I guess. AFAICT we don't do that yet. > I don't have any data on how many victims of this ransom campaign might > have been helped by this. But it certainly seems like a decent best > practice. I'd hope that _most_ people have moved on to using a > credential helper and supplying the initial password interactively these > days; this is really just aimed at people who don't know better. So the > goal is making them a bit more secure, but also educating them about the > other options. Hopefully without breaking any workflows. :) Well, let's also not underestimate the side effect of educating people (even transitively) by the mere action of doing something about it and talking about that. Ciao, Dscho > > -Peff > > [1/3]: transport_anonymize_url(): support retaining username > [2/3]: clone: avoid storing URL passwords in config > [3/3]: clone: auto-enable git-credential-store when necessary > > builtin/clone.c | 37 +++++++++++++++++++++++++++++++++++-- > credential.c | 13 +++++++++++++ > credential.h | 6 ++++++ > t/t5550-http-fetch-dumb.sh | 12 ++++++++++++ > transport.c | 21 ++++++++++++++------- > transport.h | 11 ++++++++++- > 6 files changed, 90 insertions(+), 10 deletions(-) >
