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

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 2621 bytes --]

On 2021-04-30 at 18:37:24, Derrick Stolee via GitGitGadget wrote:
> Create a new config option, core.allowUsernamePasswordUrls, which is
> disabled by default. If Git attempts to parse a password from a URL in
> this form, it will die() if this config is not enabled. This affects a
> few test scripts, but enabling the config in those places is relatively
> simple.

Let's call this http.allowUsernamePasswordURLs (or
http.allowCredentialsInURL) because this is ultimately about HTTP and
HTTPS (and maybe FTP if we still support that, which I certainly hope we
do not).  SSH doesn't have URLs and won't read a password from either
the URL or a credential helper, since OpenSSH won't read a password from
anything but a terminal (which is secure, but occasionally irritating).

> This will cause a significant change in behavior for users who rely upon
> this username:password pattern. The error message describes the config
> that they must enable to continue working with these URLs. This has a
> significant chance of breaking some automated workflows that use URLs in
> this fashion, but even those workflows would be better off using a
> different mechanism for credentials.

I will admit to using this pattern in a test I was writing just this
week.  I ultimately switched to an environment-based credential helper
(à la FAQ) in my test, but I think automated tests and other situations
where the credentials don't matter are really the only cases where this
is okay.  I do think we will break some systems as a result, especially
in situations where users cannot otherwise specify credentials (e.g., a
SaaS offering which clones your repository to provide some
functionality).

So I am a bit torn about this.  On one hand, we should really encourage
much more secure options whenever possible and I'm glad this does this,
but on the other hand, there are some useful cases where this is
unobjectionable or at least the least terrible option and the config
option may be a problem.  Don't let my doubts hold up this series if
everyone else is for it, though.  It's definitely an improvement in
security.

It is my intention (in my copious free time) to adjust credential
helpers to support arbitrary auth schemes (e.g., Bearer).  At that
point, I plan to deprecate support for Authorization extra headers.  In
that case, because the user is guaranteed to have the opportunity to
edit the config, they are guaranteed to have credential helper support
and therefore there are no use cases we're excluding.
-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]