Re: [PATCH 1/2] http: only reject basic auth credentials once they have been tried
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 5, 2024, 05:47 UTC
- Message-ID
- <ZcB2hHBoyiBdOw81@tanuki>
- In-Reply-To
- <20240204185427.39664-2-ypsah@devyard.org>
On Sun, Feb 04, 2024 at 07:54:26PM +0100, Quentin Bouget wrote:
Show 15 quoted lines
> When CURLAUTH_GSSNEGOTIATE is enabled, it is currently assumed that > the provided username/password relate to a GSSAPI auth attempt. > In practice, forges such as gitlab can be deployed with HTTP basic auth > and GSSAPI auth both listening on the same port, meaning just because > the server supports GSSAPI and failed an authentication attempt using > the provided credentials, it does not mean the credentials are not valid > HTTP basic auth credentials. > > This is documented as a long running bug here [1] and breaks token-based > authentication when the token is provided in the remote's URL itself. > > This commit makes it so credentials are only dropped once they have been > tried both as GSSAPI credentials and HTTP basic auth credentials. > > [1] https://gitlab.com/gitlab-org/gitlab/-/blob/b0e0d25646d1992fefda863febdcba8d4c7a1bbf/doc/integration/kerberos.md#L250
Do you think it's feasible to add a test for this? We already have a bunch of tests for authentication with Apache's httpd in t5563, so if we could extend t/lib-httpd.sh to set up `mod_auth_gssapi` that would be great.
I didn't try though, and it could just as well be that this would require a full-fledged Kerberos setup, which would be a deal breaker I guess. I ain't got enough familiarity with `mod_auth_gssapi` to tell.
Patrick
Show 35 quoted lines
> Signed-off-by: Quentin Bouget <ypsah@devyard.org>
> ---
> http.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/http.c b/http.c
> index e73b136e58..ccea19ac47 100644
> --- a/http.c
> +++ b/http.c
> @@ -1758,10 +1758,7 @@ static int handle_curl_result(struct slot_results *results)
> } else if (missing_target(results))
> return HTTP_MISSING_TARGET;
> else if (results->http_code == 401) {
> - if (http_auth.username && http_auth.password) {
> - credential_reject(&http_auth);
> - return HTTP_NOAUTH;
> - } else {
> + if ((http_auth_methods & CURLAUTH_GSSNEGOTIATE) == CURLAUTH_GSSNEGOTIATE) {
> http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
> if (results->auth_avail) {
> http_auth_methods &= results->auth_avail;
> @@ -1769,6 +1766,9 @@ static int handle_curl_result(struct slot_results *results)
> }
> return HTTP_REAUTH;
> }
> + if (http_auth.username && http_auth.password)
> + credential_reject(&http_auth);
> + return HTTP_NOAUTH;
> } else {
> if (results->http_connectcode == 407)
> credential_reject(&proxy_auth);
> --
> 2.43.0
>
>