Web lists-archives.com

Re: Git ransom campaign incident report - May 2019

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 ;) ).

I did 3b here: automagically enabling credential-store. I'm still on the
fence on whether that's a good idea or not.

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. :)


  [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(-)