From 124df9f038152e117feab03a4e9f2a3d7067036a Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Thu, 13 Jan 2022 20:10:58 +0000 Subject: [PATCH] Fall back from Negotiate to Basic The Negotiate authentication scheme can support multiple different types of authentication. The two most popular are NTLM and Kerberos. When we supported NTLM, we'd first try Kerberos, and if it failed, fall back to NTLM. However, we no longer support NTLM, but some people still have server configuration that uses NTLM via Negotiate. For these people, authentication may be broken. Let's fall back to Basic in such a case by keeping track of which authentication mechanisms we've tried, keeping only the supported mechanisms if we got a response, and stripping out failing mechanisms, falling back to Basic. To help with servers that support both Negotiate and Basic, we specifically consider SPNEGO (Negotiate) errors as authentication errors. This is because if the server supports Kerberos but the client does not have a ticket, then we'll get an error trying to read the client's tickets, which will manifest in this way. --- creds/access.go | 9 +++++++++ lfsapi/auth.go | 40 ++++++++++++++++++++++++++++------------ lfsapi/auth_test.go | 14 +++++++++++++- lfsapi/lfsapi.go | 2 ++ lfshttp/client.go | 6 ++++++ 5 files changed, 58 insertions(+), 13 deletions(-) diff --git a/creds/access.go b/creds/access.go index 66c18fa9..8cfc96e4 100644 --- a/creds/access.go +++ b/creds/access.go @@ -31,3 +31,12 @@ func (a *Access) Mode() AccessMode { func (a *Access) URL() string { return a.url } + +// AllAccessModes returns all access modes in the order they should be tried. +func AllAccessModes() []AccessMode { + return []AccessMode{ + NoneAccess, + NegotiateAccess, + BasicAccess, + } +} diff --git a/lfsapi/auth.go b/lfsapi/auth.go index e0ff54fe..6a7210bc 100644 --- a/lfsapi/auth.go +++ b/lfsapi/auth.go @@ -68,9 +68,11 @@ func (c *Client) doWithAuth(remote string, access creds.Access, req *http.Reques res, err := c.doWithCreds(req, credWrapper, access, via) if err != nil { if errors.IsAuthError(err) { - newAccess := access.Upgrade(getAuthAccess(res)) + newMode, newModes := getAuthAccess(res, access.Mode(), c.access) + newAccess := access.Upgrade(newMode) if newAccess.Mode() != access.Mode() { c.Endpoints.SetAccess(newAccess) + c.access = newModes } if credWrapper.Creds != nil { @@ -312,20 +314,34 @@ var ( authenticateHeaders = []string{"Lfs-Authenticate", "Www-Authenticate"} ) -func getAuthAccess(res *http.Response) creds.AccessMode { - for _, headerName := range authenticateHeaders { - for _, auth := range res.Header[headerName] { - pieces := strings.SplitN(strings.ToLower(auth), " ", 2) - if len(pieces) == 0 { - continue - } +func getAuthAccess(res *http.Response, access creds.AccessMode, modes []creds.AccessMode) (creds.AccessMode, []creds.AccessMode) { + newModes := make([]creds.AccessMode, 0, len(modes)) + for _, mode := range modes { + if access != mode { + newModes = append(newModes, mode) + } + } + if res != nil { + supportedModes := make(map[creds.AccessMode]struct{}) + for _, headerName := range authenticateHeaders { + for _, auth := range res.Header[headerName] { + pieces := strings.SplitN(strings.ToLower(auth), " ", 2) + if len(pieces) == 0 { + continue + } - switch creds.AccessMode(pieces[0]) { - case creds.NegotiateAccess: - return creds.AccessMode(pieces[0]) + switch creds.AccessMode(pieces[0]) { + case creds.BasicAccess, creds.NegotiateAccess: + supportedModes[creds.AccessMode(pieces[0])] = struct{}{} + } + } + } + for _, mode := range newModes { + if _, ok := supportedModes[mode]; ok { + return mode, newModes } } } - return creds.BasicAccess + return creds.BasicAccess, newModes } diff --git a/lfsapi/auth_test.go b/lfsapi/auth_test.go index ad61bee9..af59ef97 100644 --- a/lfsapi/auth_test.go +++ b/lfsapi/auth_test.go @@ -39,11 +39,23 @@ func TestAuthenticateHeaderAccess(t *testing.T) { res := &http.Response{Header: make(http.Header)} res.Header.Set(key, value) t.Logf("%s: %s", key, value) - assert.Equal(t, expected, getAuthAccess(res)) + result, _ := getAuthAccess(res, creds.NoneAccess, creds.AllAccessModes()) + assert.Equal(t, expected, result) } } } +func TestDualAccessModes(t *testing.T) { + res := &http.Response{Header: make(http.Header)} + res.Header["Www-Authenticate"] = []string{"Negotiate 123", "Basic 456"} + access, next := getAuthAccess(res, creds.NoneAccess, creds.AllAccessModes()) + assert.Equal(t, creds.NegotiateAccess, access) + access, next = getAuthAccess(res, access, next) + assert.Equal(t, creds.BasicAccess, access) + access, _ = getAuthAccess(res, access, next) + assert.Equal(t, creds.BasicAccess, access) +} + func TestDoWithAuthApprove(t *testing.T) { var called uint32 diff --git a/lfsapi/lfsapi.go b/lfsapi/lfsapi.go index f762c3ac..f3dc7458 100644 --- a/lfsapi/lfsapi.go +++ b/lfsapi/lfsapi.go @@ -17,6 +17,7 @@ type Client struct { client *lfshttp.Client context lfshttp.Context + access []creds.AccessMode } func NewClient(ctx lfshttp.Context) (*Client, error) { @@ -37,6 +38,7 @@ func NewClient(ctx lfshttp.Context) (*Client, error) { client: httpClient, context: ctx, credContext: creds.NewCredentialHelperContext(gitEnv, osEnv), + access: creds.AllAccessModes(), } return c, nil diff --git a/lfshttp/client.go b/lfshttp/client.go index 778184b8..09b809b4 100644 --- a/lfshttp/client.go +++ b/lfshttp/client.go @@ -3,6 +3,7 @@ package lfshttp import ( "context" "crypto/tls" + goerrors "errors" "fmt" "io" "net" @@ -301,6 +302,11 @@ func (c *Client) DoWithRedirect(cli *http.Client, req *http.Request, remote stri if err != nil { c.traceResponse(req, tracedReq, nil) + // SPNEGO (Negotiate) errors are authentication errors. + var spnegoErr *spnego.Error + if goerrors.As(err, &spnegoErr) { + return nil, nil, errors.NewAuthError(err) + } return nil, nil, err }