On Sun, May 19 2019, Jeff King wrote:
> If the user clones with a URL containing a password and has no
> credential helper configured, we're stuck. We don't want to write the
> password into .git/config because that risks accidentally disclosing it.
> But if we don't record it somewhere, subsequent fetches will fail unless
> the user is there to input the password.
>
> The best advice we can give the user is to set up a credential helper.
> But we can actually go a step further and enable the "store" helper for
> them. This still records the password in plaintext, but:
>
> 1. It's not inside the repo directory, which makes it slightly less
> likely to be disclosed.
>
> 2. The permissions on the storage file are tighter than what would be
> on .git/config.
>
> So this is generally a security win over the old behavior of writing it
> into .git/config. And it's a usability win over the more recent behavior
> of just forgetting the password entirely.
>
> The biggest downside is that it's a bit magical from the user's
> perspective, because now the password is off in some other file (usually
> ~/.git-credentials, but sometimes in $XDG_CONFIG_HOME). Which
> complicates things if they want to purge the repo and password, for
> example, because now they can't just delete the repository directory.
>
> The file location is documented, though, and we point people to the
> documentation. So perhaps it will be enough (and better still, may lead
> to them configuring a more secure helper).
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> If we do decide this is too magical, I think the best alternate path is
> to advise them on credential helpers, and how to seed the password into
> the helper (basically configure the helper and then "git fetch"
> should work).
>
> One other thing I wondered: if they do have a helper configured this
> patch will omit the advice entirely, but we'll still print the warning.
> Is that useful (to remind them that passwords in URLs are a bad thing),
> or is it just annoying?
A few things, in no particular order, sorry for the braindump. Please
don't take this as "this all sucks" just trying to help by poking holes
in it.
1. I'm a bit uneasy about us acting on an incident where the details of
what happened / frequency haven't been released. I.e. GitHub
supposedly has ~100 million repos, one source claims on the order of
~300 of these (public)[1], none of the sites said anything on that,
that 300 number is third-party speculation.
Let's say it's 100x that counting private repos. That's still a very
small percentage of 100 million.
But more importantly, it doesn't give us any insight on how to
perhaps better solve this problem, e.g. maybe 80% of it is people
using some build system (docker?) where we'd expose /home too. Not
saying it is, just that we basically have a partial bug report here.
I don't have a good sense for what the state is there. Is this all
repos cloned in /var/www, then this is sufficient, or something
else?
2. We're now making the trade-off of auto-storing the password in ~/,
is that a worse default now in the wild? We're defaulting to storing
for a longer time (e.g. on a shared *nix server) in workflows where
you'd clone && rm -rf.
So yeah, we might handle this *specific* incident, but are we just
making it worse for others?
3. There's seemingly no way to just say "I want it the old way, make it
so!" without creating an auth helper that does that.
Storing passwords in config isn't per-se insecure, neither is having
passwords in (https) URLs. Yeah in combination with *other* stuff
the might be insecure, but often not (but may actually become
insecure by auto-storing in ~/)
4. Re #3: We have existing users in the wild who do this, e.g. GitLab
CI, the user/password token (lives for about an hour IIRC) is stored
in .gitconfig, there's no writable /home there IIRC (or might not be
in similar CI environments), and I daresay the people who set that
up understand the security trade-offs.
5. Given #2 && #3 && #4 I'd be much more comfortable with something
where we just make this fail outright, and force the user to eitiher
say "Yeah I want passwords in .git/config here" or "oh thanks, I'll
use a credential helper" via some core.* or credentials.* config.
I.e. let's not make #3 impossible, and for users who are clueless
about security we're doing them a disservice by auto-using a crappy
fallback ~/ helper, whereas they might have an *actual* helper they
can use that doesn't suck (i.e. OS pwd store) if it was an error.
6. We'll still happily let this new behavior pass by on init && config
fetch. Should we care? I was going to say this categorically breaks
core.sharedRepository=group with a password in the repo, it does in
the 'git -c core.sharedRepository=group clone' case, but you can
manually set it up still, you just can't use "clone" anymore.
7. We still accept passwords on the command-line for cloning, so if
we're trying to help novice users we're still open to the shared
server attack where someone just runs "ps auxf" in a loop to scrape
passwords.
To me this is another good argument for some variant of #3. We could
provide ssh-like security by outright refusing passwords on the
command-line (could take them interactively, or via some FD/file),
then allow relaxing that to the level of this auto-helper, or all
the way down to the old behavior.
1. https://hub.packtpub.com/atlassian-bitbucket-github-and-gitlab-take-collective-steps-against-the-git-ransomware-attack/
