On Mon Feb 5, 2024 at 12:01 AM CET, wrote:
> On Sunday, February 4, 2024 1:54 PM, Quentin Bouget wrote:
> > During a re-authentication (second attempt at authenticating with a remote, e.g.
> > after a failed GSSAPI attempt), git allows the remote to provide credential overrides
> > in the redirect URL and unconditionnaly drops the current HTTP credentials in favors
> > of those, even when there aren't any.
> >
> > This commit makes it so HTTP credentials are only overridden when the redirect URL
> > actually contains credentials itself.
> >
> > Signed-off-by: Quentin Bouget <ypsah@xxxxxxxxxxx>
> > ---
> > http.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/http.c b/http.c
> > index ccea19ac47..caba9cac1e 100644
> > --- a/http.c
> > +++ b/http.c
> > @@ -2160,7 +2160,25 @@ static int http_request_reauth(const char *url,
> > if (options && options->effective_url && options->base_url) {
> > if (update_url_from_redirect(options->base_url,
> > url, options->effective_url)) {
> > + char *username = NULL, *password = NULL;
> > +
> > + if (http_auth.username)
> > + username = xstrdup(http_auth.username);
> > + if (http_auth.password)
> > + password = xstrdup(http_auth.password);
> > +
> > credential_from_url(&http_auth, options->base_url->buf);
> > +
> > + if (http_auth.username)
> > + free(username);
> > + else if (username)
> > + http_auth.username = username;
> > +
> > + if (http_auth.password)
> > + free(password);
> > + else if (password)
> > + http_auth.password = password;
> > +
> > url = options->effective_url->buf;
> > }
> > }
>
> I am wondering whether this is a good idea. Having credentials in a redirect
> seems like it might be a vector for going somewhere other than what you want
> to do, with credentials you do not necessarily want. Others might no better
> than I on this, but would potentially lead to a CVE? I would prefer to see
> credentials in a redirect rejected rather than used.
I guess this can be controlled by setting http.followRedirects to false.
I am not sure there is generally more danger in following a redirect URL
with or without the provided credentials. Note that this is also how git
currently behaves and while I am not aware of any concrete use case
myself, I am also not confident there isn't any.
That being said, Brian M. Carlson pointed out that reusing credentials
from the initial URL for the redirect URL is quite dangerous.
In my reply, I have suggested to switch to implementing the same
behaviour as libcurl when it comes to reusing credentials: if the
hostname of the redirect is the same as the original URL, reuse the
credentials, otherwise drop them. [1]
I would still retain the (seemingly strange) current behaviour of
favoring credentials in the redirect URL over anything else.
Thanks,
Quentin
[1] https://curl.se/libcurl/c/CURLOPT_UNRESTRICTED_AUTH.html
