Re: [PATCH] urlmatch: do not allow passwords in URLs by default

[Jeff King <peff@peff.net>]

On Fri, Apr 30, 2021 at 06:37:24PM +0000, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <dstolee@microsoft.com>
> 
> Git allows URLs of the following pattern:
> 
>   https://username:password@domain/route
> 
> These URLs are then parsed to pull out the username and password for use
> when authenticating with the URL. Git is careful to anonymize the URL in
> status messages with transport_anonymize_url(), but it stores the URL as
> plaintext in the .git/config file. The password may leak in other ways.

I'm not really opposed to disallowing this entirely (with an escape
hatch, as you have here), because it really is an awful practice for a
lot of reasons. But another option we discussed previously was to allow
the initial clone, but not store the password, which would result in the
user being prompted for subsequent fetches:

  https://lore.kernel.org/git/20190519050724.GA26179@sigill.intra.peff.net/

I think that third patch there is just too gross. But with the first
two, if you do have a credential helper configured, then:

  git clone https://user:pass@example.com/repo.git

would do what you want: clone with that user/pass, and then store the
result in the credential helper.

> @@ -191,6 +204,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
>  			}
>  			colon_ptr = strchr(norm.buf + scheme_len + 3, ':');
>  			if (colon_ptr) {
> +				die_if_username_password_not_allowed();
>  				passwd_off = (colon_ptr + 1) - norm.buf;
>  				passwd_len = norm.len - passwd_off;
>  				user_len = (passwd_off - 1) - (scheme_len + 3);

It's probably a bit nicer to just ignore the password, which will prompt
the user. But then, it is nicer still to use it just the one time but
not store it in the .git/config file. :)

-Peff