Re: Git ransom campaign incident report - May 2019

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 20, 2019, 14:43 UTC
Message-ID: <nycvar.QRO.7.76.6.1905201339540.46@tvgsbejvaqbjf.bet>
In-Reply-To: <20190519050724.GA26179@sigill.intra.peff.net>

Hi Peff,

On Sun, 19 May 2019, Jeff King wrote:

Show 31 quoted lines

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

Show 7 quoted lines

> 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

Show 15 quoted lines

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