From 6956fff5df9bebf7a2e6e44e84624550f90779a9 Mon Sep 17 00:00:00 2001 From: Kevin Manley Date: Thu, 7 Jan 2016 16:36:48 -0500 Subject: [PATCH 1/3] fix for abbot/go-http-auth#23, support proxy auth --- README.md | 1 + auth.go | 32 +++++++++++++++ basic.go | 14 ++++--- basic_test.go | 55 +++++++++++++------------- digest.go | 74 +++++++++++++++++++++++++---------- digest_test.go | 103 ++++++++++++++++++++++++++----------------------- 6 files changed, 179 insertions(+), 100 deletions(-) diff --git a/README.md b/README.md index 894467c..93ffea6 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,7 @@ Features -------- * Supports HTTP Basic and HTTP Digest authentication. + * Supports proxy authentication * Supports htpasswd and htdigest formatted files. * Automatic reloading of password files. * Pluggable interface for user/password storage. diff --git a/auth.go b/auth.go index d769ff9..71eaa5f 100644 --- a/auth.go +++ b/auth.go @@ -102,3 +102,35 @@ func JustCheck(auth AuthenticatorInterface, wrapped http.HandlerFunc) http.Handl wrapped(w, &ar.Request) }) } + +func AuthenticateHeaderName(proxy bool) string { + if proxy { + return "Proxy-Authenticate" + } + return "WWW-Authenticate" +} + +func AuthorizationHeaderName(proxy bool) string { + if proxy { + return "Proxy-Authorization" + } + return "Authorization" +} + +func AuthenticationInfoHeaderName(proxy bool) string { + if proxy { + return "Proxy-Authentication-Info" + } + return "Authentication-Info" +} + +func UnauthorizedStatusCode(proxy bool) int { + if proxy { + return http.StatusProxyAuthRequired + } + return http.StatusUnauthorized +} + +func UnauthorizedStatusText(proxy bool) string { + return http.StatusText(UnauthorizedStatusCode(proxy)) +} diff --git a/basic.go b/basic.go index e165c28..52e4e6f 100644 --- a/basic.go +++ b/basic.go @@ -29,6 +29,7 @@ var ( ) type BasicAuth struct { + IsProxy bool Realm string Secrets SecretProvider } @@ -44,7 +45,7 @@ var _ = (AuthenticatorInterface)((*BasicAuth)(nil)) Supports MD5 and SHA1 password entries */ func (a *BasicAuth) CheckAuth(r *http.Request) string { - s := strings.SplitN(r.Header.Get("Authorization"), " ", 2) + s := strings.SplitN(r.Header.Get(AuthorizationHeaderName(a.IsProxy)), " ", 2) if len(s) != 2 || s[0] != "Basic" { return "" } @@ -102,9 +103,8 @@ func compareMD5HashAndPassword(hashedPassword, password []byte) error { (or requires reauthentication). */ func (a *BasicAuth) RequireAuth(w http.ResponseWriter, r *http.Request) { - w.Header().Set("WWW-Authenticate", `Basic realm="`+a.Realm+`"`) - w.WriteHeader(401) - w.Write([]byte("401 Unauthorized\n")) + w.Header().Set(AuthenticateHeaderName(a.IsProxy), `Basic realm="`+a.Realm+`"`) + http.Error(w, UnauthorizedStatusText(a.IsProxy), UnauthorizedStatusCode(a.IsProxy)) } /* @@ -130,7 +130,7 @@ func (a *BasicAuth) NewContext(ctx context.Context, r *http.Request) context.Con info := &Info{Username: a.CheckAuth(r), ResponseHeaders: make(http.Header)} info.Authenticated = (info.Username != "") if !info.Authenticated { - info.ResponseHeaders.Set("WWW-Authenticate", `Basic realm="`+a.Realm+`"`) + info.ResponseHeaders.Set(AuthenticateHeaderName(a.IsProxy), `Basic realm="`+a.Realm+`"`) } return context.WithValue(ctx, infoKey, info) } @@ -138,3 +138,7 @@ func (a *BasicAuth) NewContext(ctx context.Context, r *http.Request) context.Con func NewBasicAuthenticator(realm string, secrets SecretProvider) *BasicAuth { return &BasicAuth{Realm: realm, Secrets: secrets} } + +func NewBasicAuthenticatorForProxy(realm string, secrets SecretProvider) *BasicAuth { + return &BasicAuth{IsProxy: true, Realm: realm, Secrets: secrets} +} diff --git a/basic_test.go b/basic_test.go index 99f44fc..47de9c9 100644 --- a/basic_test.go +++ b/basic_test.go @@ -8,33 +8,36 @@ import ( func TestAuthBasic(t *testing.T) { secrets := HtpasswdFileProvider("test.htpasswd") - a := &BasicAuth{Realm: "example.com", Secrets: secrets} - r := &http.Request{} - r.Method = "GET" - if a.CheckAuth(r) != "" { - t.Fatal("CheckAuth passed on empty headers") - } - r.Header = http.Header(make(map[string][]string)) - r.Header.Set("Authorization", "Digest blabla ololo") - if a.CheckAuth(r) != "" { - t.Fatal("CheckAuth passed on bad headers") - } - r.Header.Set("Authorization", "Basic !@#") - if a.CheckAuth(r) != "" { - t.Fatal("CheckAuth passed on bad base64 data") - } - data := [][]string{ - {"test", "hello"}, - {"test2", "hello2"}, - {"test3", "hello3"}, - {"test16", "topsecret"}, - } - for _, tc := range data { - auth := base64.StdEncoding.EncodeToString([]byte(tc[0] + ":" + tc[1])) - r.Header.Set("Authorization", "Basic "+auth) - if a.CheckAuth(r) != tc[0] { - t.Fatalf("CheckAuth failed for user '%s'", tc[0]) + for _, isProxy := range []bool{false, true} { + a := &BasicAuth{IsProxy: isProxy, Realm: "example.com", Secrets: secrets} + r := &http.Request{} + r.Method = "GET" + if a.CheckAuth(r) != "" { + t.Fatal("CheckAuth passed on empty headers") + } + r.Header = http.Header(make(map[string][]string)) + r.Header.Set(AuthorizationHeaderName(a.IsProxy), "Digest blabla ololo") + if a.CheckAuth(r) != "" { + t.Fatal("CheckAuth passed on bad headers") + } + r.Header.Set(AuthorizationHeaderName(a.IsProxy), "Basic !@#") + if a.CheckAuth(r) != "" { + t.Fatal("CheckAuth passed on bad base64 data") + } + + data := [][]string{ + {"test", "hello"}, + {"test2", "hello2"}, + {"test3", "hello3"}, + {"test16", "topsecret"}, + } + for _, tc := range data { + auth := base64.StdEncoding.EncodeToString([]byte(tc[0] + ":" + tc[1])) + r.Header.Set(AuthorizationHeaderName(a.IsProxy), "Basic "+auth) + if a.CheckAuth(r) != tc[0] { + t.Fatalf("CheckAuth failed for user '%s'", tc[0]) + } } } } diff --git a/digest.go b/digest.go index 524c7b1..b426920 100644 --- a/digest.go +++ b/digest.go @@ -20,6 +20,7 @@ type digest_client struct { } type DigestAuth struct { + IsProxy bool Realm string Opaque string Secrets SecretProvider @@ -81,16 +82,18 @@ func (a *DigestAuth) Purge(count int) { (or requires reauthentication). */ func (a *DigestAuth) RequireAuth(w http.ResponseWriter, r *http.Request) { + a.mutex.Lock() + defer a.mutex.Unlock() + if len(a.clients) > a.ClientCacheSize+a.ClientCacheTolerance { a.Purge(a.ClientCacheTolerance * 2) } nonce := RandomKey() a.clients[nonce] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - w.Header().Set("WWW-Authenticate", + w.Header().Set(AuthenticateHeaderName(a.IsProxy), fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="MD5", qop="auth"`, a.Realm, nonce, a.Opaque)) - w.WriteHeader(401) - w.Write([]byte("401 Unauthorized\n")) + http.Error(w, UnauthorizedStatusText(a.IsProxy), UnauthorizedStatusCode(a.IsProxy)) } /* @@ -98,8 +101,8 @@ func (a *DigestAuth) RequireAuth(w http.ResponseWriter, r *http.Request) { auth parameters or nil if the header is not a valid parsable Digest auth header. */ -func DigestAuthParams(r *http.Request) map[string]string { - s := strings.SplitN(r.Header.Get("Authorization"), " ", 2) +func (a *DigestAuth) DigestAuthParams(r *http.Request) map[string]string { + s := strings.SplitN(r.Header.Get(AuthorizationHeaderName(a.IsProxy)), " ", 2) if len(s) != 2 || s[0] != "Digest" { return nil } @@ -118,21 +121,46 @@ func (da *DigestAuth) CheckAuth(r *http.Request) (username string, authinfo *str defer da.mutex.Unlock() username = "" authinfo = nil - auth := DigestAuthParams(r) + auth := da.DigestAuthParams(r) if auth == nil || da.Opaque != auth["opaque"] || auth["algorithm"] != "MD5" || auth["qop"] != "auth" { return } - // Check if the requested URI matches auth header - switch u, err := url.Parse(auth["uri"]); { - case err != nil: - return - case r.URL == nil: - return - case len(u.Path) > len(r.URL.Path): - return - case !strings.HasPrefix(r.URL.Path, u.Path): - return + /* Check whether the requested URI matches auth header + NOTE: when we're a proxy and method is CONNECT, the request and auth uri + specify a hostname not a path, e.g. + + CONNECT 1-edge-chat.facebook.com:443 HTTP/1.1 + User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0 + Proxy-Connection: keep-alive + Connection: keep-alive + Host: 1-edge-chat.facebook.com:443 + Proxy-Authorization: Digest username="test", realm="", + nonce="iQSz9RcA1Qsa6ono", + uri="1-edge-chat.facebook.com:443", + algorithm=MD5, + response="a077a4676d60ff8bf48577ad7c7360d6", + opaque="EN3BwDsuWB5F6IWR", qop=auth, nc=0000000c, + cnonce="548d04d1bbd63926" + */ + + if r.Method == "CONNECT" { + if r.RequestURI != auth["uri"] { + return + } + } else { + + // Check if the requested URI matches auth header + switch u, err := url.Parse(auth["uri"]); { + case err != nil: + return + case r.URL == nil: + return + case len(u.Path) > len(r.URL.Path): + return + case !strings.HasPrefix(r.URL.Path, u.Path): + return + } } HA1 := da.Secrets(auth["username"], da.Realm) @@ -193,7 +221,7 @@ func (a *DigestAuth) Wrap(wrapped AuthenticatedHandlerFunc) http.HandlerFunc { } else { ar := &AuthenticatedRequest{Request: *r, Username: username} if authinfo != nil { - w.Header().Set("Authentication-Info", *authinfo) + w.Header().Set(AuthenticationInfoHeaderName(a.IsProxy), *authinfo) } wrapped(w, ar) } @@ -218,15 +246,15 @@ func (a *DigestAuth) NewContext(ctx context.Context, r *http.Request) context.Co info := &Info{Username: username, ResponseHeaders: make(http.Header)} if username != "" { info.Authenticated = true - info.ResponseHeaders.Set("Authentication-Info", *authinfo) + info.ResponseHeaders.Set(AuthenticationInfoHeaderName(a.IsProxy), *authinfo) } else { - // return back digest WWW-Authenticate header + // return back digest XYZ-Authenticate header if len(a.clients) > a.ClientCacheSize+a.ClientCacheTolerance { a.Purge(a.ClientCacheTolerance * 2) } nonce := RandomKey() a.clients[nonce] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - info.ResponseHeaders.Set("WWW-Authenticate", + info.ResponseHeaders.Set(AuthenticateHeaderName(a.IsProxy), fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="MD5", qop="auth"`, a.Realm, nonce, a.Opaque)) } @@ -244,3 +272,9 @@ func NewDigestAuthenticator(realm string, secrets SecretProvider) *DigestAuth { clients: map[string]*digest_client{}} return da } + +func NewDigestAuthenticatorForProxy(realm string, secrets SecretProvider) *DigestAuth { + da := NewDigestAuthenticator(realm, secrets) + da.IsProxy = true + return da +} diff --git a/digest_test.go b/digest_test.go index 626dae7..06430d4 100644 --- a/digest_test.go +++ b/digest_test.go @@ -10,60 +10,64 @@ import ( ) func TestAuthDigest(t *testing.T) { - secrets := HtdigestFileProvider("test.htdigest") - da := &DigestAuth{Opaque: "U7H+ier3Ae8Skd/g", - Realm: "example.com", - Secrets: secrets, - clients: map[string]*digest_client{}} - r := &http.Request{} - r.Method = "GET" - if u, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for empty request header") - } - r.Header = http.Header(make(map[string][]string)) - r.Header.Set("Authorization", "Digest blabla") - if u, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for bad request header") - } - r.Header.Set("Authorization", `Digest username="test", realm="example.com", nonce="Vb9BP/h81n3GpTTB", uri="/t2", cnonce="NjE4MTM2", nc=00000001, qop="auth", response="ffc357c4eba74773c8687e0bc724c9a3", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) - if u, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for unknown client") - } - r.URL, _ = url.Parse("/t2") - da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - if u, _ := da.CheckAuth(r); u != "test" { - t.Fatal("empty auth for legitimate client") - } + for _, isProxy := range []bool{false, true} { + secrets := HtdigestFileProvider("test.htdigest") + da := &DigestAuth{IsProxy: isProxy, + Opaque: "U7H+ier3Ae8Skd/g", + Realm: "example.com", + Secrets: secrets, + clients: map[string]*digest_client{}} + r := &http.Request{} + r.Method = "GET" + if u, _ := da.CheckAuth(r); u != "" { + t.Fatal("non-empty auth for empty request header") + } + r.Header = http.Header(make(map[string][]string)) + r.Header.Set(AuthorizationHeaderName(da.IsProxy), "Digest blabla") + if u, _ := da.CheckAuth(r); u != "" { + t.Fatal("non-empty auth for bad request header") + } + r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="Vb9BP/h81n3GpTTB", uri="/t2", cnonce="NjE4MTM2", nc=00000001, qop="auth", response="ffc357c4eba74773c8687e0bc724c9a3", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) + if u, _ := da.CheckAuth(r); u != "" { + t.Fatal("non-empty auth for unknown client") + } - // our nc is now 0, client nc is 1 - if u, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for outdated nc") - } + r.URL, _ = url.Parse("/t2") + da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} + if u, _ := da.CheckAuth(r); u != "test" { + t.Fatal("empty auth for legitimate client") + } - // try again with nc checking off - da.DisableNonceCountCheck = true - if u, _ := da.CheckAuth(r); u != "test" { - t.Fatal("empty auth for outdated nc even though nc checking is off") - } - da.DisableNonceCountCheck = false + // our nc is now 0, client nc is 1 + if u, _ := da.CheckAuth(r); u != "" { + t.Fatal("non-empty auth for outdated nc") + } - r.URL, _ = url.Parse("/") - da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - if u, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for bad request path") - } + // try again with nc checking off + da.DisableNonceCountCheck = true + if u, _ := da.CheckAuth(r); u != "test" { + t.Fatal("empty auth for outdated nc even though nc checking is off") + } + da.DisableNonceCountCheck = false - r.URL, _ = url.Parse("/t3") - da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - if u, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for bad request path") - } + r.URL, _ = url.Parse("/") + da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} + if u, _ := da.CheckAuth(r); u != "" { + t.Fatal("non-empty auth for bad request path") + } + + r.URL, _ = url.Parse("/t3") + da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} + if u, _ := da.CheckAuth(r); u != "" { + t.Fatal("non-empty auth for bad request path") + } - da.clients["+RbVXSbIoa1SaJk1"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - r.Header.Set("Authorization", `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000001, qop="auth", response="c08918024d7faaabd5424654c4e3ad1c", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) - if u, _ := da.CheckAuth(r); u != "test" { - t.Fatal("empty auth for valid request in subpath") + da.clients["+RbVXSbIoa1SaJk1"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} + r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000001, qop="auth", response="c08918024d7faaabd5424654c4e3ad1c", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) + if u, _ := da.CheckAuth(r); u != "test" { + t.Fatal("empty auth for valid request in subpath") + } } } @@ -79,8 +83,9 @@ Authorization: Digest username="test", realm="", nonce="FRPnGdb8lvM1UHhi", uri=" Connection: keep-alive ` + da := &DigestAuth{} req, _ := http.ReadRequest(bufio.NewReader(strings.NewReader(body))) - params := DigestAuthParams(req) + params := da.DigestAuthParams(req) if params["uri"] != "/css?family=Source+Sans+Pro:400,700,400italic,700italic|Source+Code+Pro" { t.Fatal("failed to parse uri with embedded commas") } From e8373172969cc1de55082d9cfdb439b0e22e72ad Mon Sep 17 00:00:00 2001 From: Kevin Manley Date: Fri, 8 Jan 2016 17:08:21 -0500 Subject: [PATCH 2/3] a better fix for #21; track ncs in a bitset which allows for detecting replays even if ncs arrive out of order --- bitset.go | 72 ++++++++++++++++++++++++++++++++++++++++++++ bitset_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++++++++ digest.go | 82 +++++++++++++++++++++++++++++++++----------------- digest_test.go | 81 ++++++++++++++++++++++++++++++++----------------- 4 files changed, 260 insertions(+), 54 deletions(-) create mode 100644 bitset.go create mode 100644 bitset_test.go diff --git a/bitset.go b/bitset.go new file mode 100644 index 0000000..06a5d9d --- /dev/null +++ b/bitset.go @@ -0,0 +1,72 @@ +// Package bitset implments a memory efficient bit array of booleans +// Adapted from https://github.com/lazybeaver/bitset + +package auth + +import "fmt" + +type BitSet struct { + bits []uint8 + size uint64 +} + +const ( + bitMaskZero = uint8(0) + bitMaskOnes = uint8((1 << 8) - 1) +) + +var ( + bitMasks = [...]uint8{0x1, 0x2, 0x4, 0x8, 0x10, 0x20, 0x40, 0x80} +) + +func (b *BitSet) getPositionAndMask(index uint64) (uint64, uint8) { + if index < 0 || index >= b.size { + panic(fmt.Errorf("BitSet index (%d) out of bounds (size: %d)", index, b.size)) + } + position := index >> 3 + mask := bitMasks[index%8] + return position, mask +} + +func (b *BitSet) Init(size uint64) { + b.bits = make([]uint8, (size+7)/8) + b.size = size +} + +func (b *BitSet) Size() uint64 { + return b.size +} + +func (b *BitSet) Get(index uint64) bool { + position, mask := b.getPositionAndMask(index) + return (b.bits[position] & mask) != 0 +} + +func (b *BitSet) Set(index uint64) { + position, mask := b.getPositionAndMask(index) + b.bits[position] |= mask +} + +func (b *BitSet) Clear(index uint64) { + position, mask := b.getPositionAndMask(index) + b.bits[position] &^= mask +} + +func (b *BitSet) String() string { + value := make([]byte, b.size) + var i uint64 + for i = 0; i < b.size; i++ { + if b.Get(i) { + value[i] = '1' + } else { + value[i] = '0' + } + } + return string(value) +} + +func NewBitSet(size uint64) *BitSet { + b := &BitSet{} + b.Init(size) + return b +} diff --git a/bitset_test.go b/bitset_test.go new file mode 100644 index 0000000..6444903 --- /dev/null +++ b/bitset_test.go @@ -0,0 +1,79 @@ +package auth + +import ( + "testing" +) + +func TestNew(t *testing.T) { + var size uint64 = 101 + bs := NewBitSet(size) + if bs.Size() != size { + t.Errorf("Unexpected initialization failure") + } + var i uint64 + for i = 0; i < size; i++ { + if bs.Get(i) { + t.Errorf("Newly initialized bitset cannot have true values") + } + } +} + +func TestGet(t *testing.T) { + bs := NewBitSet(2) + bs.Set(0) + bs.Clear(1) + if bs.Get(0) != true { + t.Errorf("Actual: false | Expected: true") + } + if bs.Get(1) != false { + t.Errorf("Actual: true | Expected: false") + } +} + +func TestSet(t *testing.T) { + bs := NewBitSet(10) + bs.Set(2) + bs.Set(3) + bs.Set(5) + bs.Set(7) + actual := bs.String() + expected := "0011010100" + if actual != expected { + t.Errorf("Actual: %s | Expected: %s", actual, expected) + } +} + +func TestClear(t *testing.T) { + bs := NewBitSet(10) + var i uint64 + for i = 0; i < 10; i++ { + bs.Set(i) + } + bs.Clear(0) + bs.Clear(3) + bs.Clear(6) + bs.Clear(9) + actual := bs.String() + expected := "0110110110" + if actual != expected { + t.Errorf("Actual: %s | Expected: %s", actual, expected) + } +} + +func BenchmarkGet(b *testing.B) { + bn := uint64(b.N) + bs := NewBitSet(bn) + var i uint64 + for i = 0; i < bn; i++ { + _ = bs.Get(i) + } +} + +func BenchmarkSet(b *testing.B) { + bn := uint64(b.N) + bs := NewBitSet(bn) + var i uint64 + for i = 0; i < bn; i++ { + bs.Set(i) + } +} diff --git a/digest.go b/digest.go index 8f691e5..3681dc2 100644 --- a/digest.go +++ b/digest.go @@ -14,8 +14,17 @@ import ( "golang.org/x/net/context" ) +const DefaultNcCacheSize = 65536 + type digest_client struct { - nc uint64 + /* + ncs_seen is a bitset used to record the nc values we've seen for a given nonce. + This allows us to identify and deny replay attacks without relying on nc values + always increasing. That's important since in practice a client's use of multiple + server connections, a hierarchy of proxies, and AJAX can cause nc values to arrive + out of order (See https://github.com/abbot/go-http-auth/issues/21) + */ + ncs_seen *BitSet last_seen int64 } @@ -25,7 +34,7 @@ type DigestAuth struct { Opaque string Secrets SecretProvider PlainTextSecrets bool - IgnoreNonceCount bool + NcCacheSize uint64 // The max number of nc values we remember before issuing a new nonce /* Approximate size of Client's Cache. When actual number of @@ -81,7 +90,7 @@ func (a *DigestAuth) Purge(count int) { http.Handler for DigestAuth which initiates the authentication process (or requires reauthentication). */ -func (a *DigestAuth) RequireAuth(w http.ResponseWriter, r *http.Request) { +func (a *DigestAuth) RequireAuth(w http.ResponseWriter, r *http.Request, stale bool) { a.mutex.Lock() defer a.mutex.Unlock() @@ -89,10 +98,13 @@ func (a *DigestAuth) RequireAuth(w http.ResponseWriter, r *http.Request) { a.Purge(a.ClientCacheTolerance * 2) } nonce := RandomKey() - a.clients[nonce] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - w.Header().Set(AuthenticateHeaderName(a.IsProxy), - fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="MD5", qop="auth"`, - a.Realm, nonce, a.Opaque)) + a.clients[nonce] = &digest_client{ncs_seen: NewBitSet(a.NcCacheSize), + last_seen: time.Now().UnixNano()} + value := fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="MD5", qop="auth"`, a.Realm, nonce, a.Opaque) + if stale { + value += ", stale=true" + } + w.Header().Set(AuthenticateHeaderName(a.IsProxy), value) http.Error(w, UnauthorizedStatusText(a.IsProxy), UnauthorizedStatusCode(a.IsProxy)) } @@ -111,16 +123,18 @@ func (a *DigestAuth) DigestAuthParams(r *http.Request) map[string]string { } /* - Check if request contains valid authentication data. Returns a pair - of username, authinfo where username is the name of the authenticated - user or an empty string and authinfo is the contents for the optional - Authentication-Info response header. + Check if request contains valid authentication data. Returns a triplet + of username, authinfo, stale where username is the name of the authenticated + user or an empty string, authinfo is the contents for the optional Authentication-Info + response header, and stale indicates whether the server-returned Authenticate header + should specify stale=true (see https://www.ietf.org/rfc/rfc2617.txt Section 3.3) */ -func (da *DigestAuth) CheckAuth(r *http.Request) (username string, authinfo *string) { +func (da *DigestAuth) CheckAuth(r *http.Request) (username string, authinfo *string, stale bool) { da.mutex.Lock() defer da.mutex.Unlock() username = "" authinfo = nil + stale = false auth := da.DigestAuthParams(r) if auth == nil || da.Opaque != auth["opaque"] || auth["algorithm"] != "MD5" || auth["qop"] != "auth" { return @@ -182,21 +196,30 @@ func (da *DigestAuth) CheckAuth(r *http.Request) (username string, authinfo *str return } - if client, ok := da.clients[auth["nonce"]]; !ok { + client, ok := da.clients[auth["nonce"]] + if !ok { + stale = true + return + } + + // Check the nonce-count + if nc >= client.ncs_seen.Size() { + // nc exceeds the size of our bitset. We can just treat this the + // same as a stale nonce + stale = true + return + } else if client.ncs_seen.Get(nc) { + // We've already seen this nc! Possible replay attack! return - } else { - if client.nc != 0 && client.nc >= nc && !da.IgnoreNonceCount { - return - } - client.nc = nc - client.last_seen = time.Now().UnixNano() } + client.ncs_seen.Set(nc) + client.last_seen = time.Now().UnixNano() resp_HA2 := H(":" + auth["uri"]) rspauth := H(strings.Join([]string{HA1, auth["nonce"], auth["nc"], auth["cnonce"], auth["qop"], resp_HA2}, ":")) info := fmt.Sprintf(`qop="auth", rspauth="%s", cnonce="%s", nc="%s"`, rspauth, auth["cnonce"], auth["nc"]) - return auth["username"], &info + return auth["username"], &info, stale } /* @@ -216,8 +239,8 @@ const DefaultClientCacheTolerance = 100 */ func (a *DigestAuth) Wrap(wrapped AuthenticatedHandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - if username, authinfo := a.CheckAuth(r); username == "" { - a.RequireAuth(w, r) + if username, authinfo, stale := a.CheckAuth(r); username == "" { + a.RequireAuth(w, r, stale) } else { ar := &AuthenticatedRequest{Request: *r, Username: username} if authinfo != nil { @@ -242,7 +265,7 @@ func (a *DigestAuth) JustCheck(wrapped http.HandlerFunc) http.HandlerFunc { // NewContext returns a context carrying authentication information for the request. func (a *DigestAuth) NewContext(ctx context.Context, r *http.Request) context.Context { - username, authinfo := a.CheckAuth(r) + username, authinfo, stale := a.CheckAuth(r) info := &Info{Username: username, ResponseHeaders: make(http.Header)} if username != "" { info.Authenticated = true @@ -253,10 +276,14 @@ func (a *DigestAuth) NewContext(ctx context.Context, r *http.Request) context.Co a.Purge(a.ClientCacheTolerance * 2) } nonce := RandomKey() - a.clients[nonce] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - info.ResponseHeaders.Set(AuthenticateHeaderName(a.IsProxy), - fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="MD5", qop="auth"`, - a.Realm, nonce, a.Opaque)) + a.clients[nonce] = &digest_client{ncs_seen: NewBitSet(a.NcCacheSize), + last_seen: time.Now().UnixNano()} + value := fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="MD5", qop="auth"`, + a.Realm, nonce, a.Opaque) + if stale { + value += ", stale=true" + } + info.ResponseHeaders.Set(AuthenticateHeaderName(a.IsProxy), value) } return context.WithValue(ctx, infoKey, info) } @@ -267,6 +294,7 @@ func NewDigestAuthenticator(realm string, secrets SecretProvider) *DigestAuth { Realm: realm, Secrets: secrets, PlainTextSecrets: false, + NcCacheSize: DefaultNcCacheSize, ClientCacheSize: DefaultClientCacheSize, ClientCacheTolerance: DefaultClientCacheTolerance, clients: map[string]*digest_client{}} diff --git a/digest_test.go b/digest_test.go index 08374d4..ef9395e 100644 --- a/digest_test.go +++ b/digest_test.go @@ -14,60 +14,87 @@ func TestAuthDigest(t *testing.T) { for _, isProxy := range []bool{false, true} { secrets := HtdigestFileProvider("test.htdigest") da := &DigestAuth{IsProxy: isProxy, - Opaque: "U7H+ier3Ae8Skd/g", - Realm: "example.com", - Secrets: secrets, - clients: map[string]*digest_client{}} + Opaque: "U7H+ier3Ae8Skd/g", + Realm: "example.com", + Secrets: secrets, + NcCacheSize: 20, + clients: map[string]*digest_client{}} r := &http.Request{} r.Method = "GET" - if u, _ := da.CheckAuth(r); u != "" { + if u, _, _ := da.CheckAuth(r); u != "" { t.Fatal("non-empty auth for empty request header") } r.Header = http.Header(make(map[string][]string)) r.Header.Set(AuthorizationHeaderName(da.IsProxy), "Digest blabla") - if u, _ := da.CheckAuth(r); u != "" { + if u, _, _ := da.CheckAuth(r); u != "" { t.Fatal("non-empty auth for bad request header") } + + r.URL, _ = url.Parse("/t2") r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="Vb9BP/h81n3GpTTB", uri="/t2", cnonce="NjE4MTM2", nc=00000001, qop="auth", response="ffc357c4eba74773c8687e0bc724c9a3", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) - if u, _ := da.CheckAuth(r); u != "" { + u, _, stale := da.CheckAuth(r) + if u != "" { t.Fatal("non-empty auth for unknown client") } - - r.URL, _ = url.Parse("/t2") - da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - if u, _ := da.CheckAuth(r); u != "test" { - t.Fatal("empty auth for legitimate client") + if !stale { + t.Fatal("stale should be true") } - // our nc is now 0, client nc is 1 - if u, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for outdated nc") + da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{ncs_seen: NewBitSet(da.NcCacheSize), + last_seen: time.Now().UnixNano()} + u, _, stale = da.CheckAuth(r) + if u != "test" { + t.Fatal("empty auth for legitimate client") } - - // try again with nc checking off - da.IgnoreNonceCount = true - if u, _ := da.CheckAuth(r); u != "test" { - t.Fatal("empty auth for outdated nc even though nc checking is off") + if stale { + t.Fatal("stale should be false") } - da.IgnoreNonceCount = false r.URL, _ = url.Parse("/") - da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - if u, _ := da.CheckAuth(r); u != "" { + da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{ncs_seen: NewBitSet(da.NcCacheSize), last_seen: time.Now().UnixNano()} + if u, _, _ := da.CheckAuth(r); u != "" { t.Fatal("non-empty auth for bad request path") } r.URL, _ = url.Parse("/t3") - da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} - if u, _ := da.CheckAuth(r); u != "" { + da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{ncs_seen: NewBitSet(da.NcCacheSize), last_seen: time.Now().UnixNano()} + if u, _, _ := da.CheckAuth(r); u != "" { t.Fatal("non-empty auth for bad request path") } - da.clients["+RbVXSbIoa1SaJk1"] = &digest_client{nc: 0, last_seen: time.Now().UnixNano()} + da.clients["+RbVXSbIoa1SaJk1"] = &digest_client{ncs_seen: NewBitSet(da.NcCacheSize), last_seen: time.Now().UnixNano()} r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000001, qop="auth", response="c08918024d7faaabd5424654c4e3ad1c", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) - if u, _ := da.CheckAuth(r); u != "test" { + if u, _, _ := da.CheckAuth(r); u != "test" { t.Fatal("empty auth for valid request in subpath") } + + // nc checking, we've already seen 00000001 so this should fail + if u, _, _ := da.CheckAuth(r); u != "" { + t.Fatal("non-empty auth for already-seen nc") + } + + // an updated request with nc 00000005 should succeed + r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000005, qop="auth", response="c553c9a48ec99de9474e662934f73de2", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) + if u, _, _ := da.CheckAuth(r); u != "test" { + t.Fatal("empty auth for valid nc 00000005") + } + + // but repeating it should fail... + r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000005, qop="auth", response="c553c9a48ec99de9474e662934f73de2", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) + if u, _, _ := da.CheckAuth(r); u != "" { + t.Fatal("non-empty auth for repeated nc 00000005") + } + + // an updated request with nc 00000002 should succeed even though it's out of order, since it hasn't been seen yet + r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000002, qop="auth", response="1c2a64978d9e8a61f823240304b95afd", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) + if u, _, _ := da.CheckAuth(r); u != "test" { + t.Fatal("empty auth for valid nc 00000002") + } + + if da.clients["+RbVXSbIoa1SaJk1"].ncs_seen.String() != "01100100000000000000" { + t.Fatal("ncs_seen bitmap didn't match expected") + } + } } From 202d88774b2bfd96ecb58766d14eef98df6db715 Mon Sep 17 00:00:00 2001 From: Kevin Manley Date: Fri, 15 Jan 2016 15:58:00 -0500 Subject: [PATCH 3/3] improve error handling for digest auth --- .gitignore | 2 + digest.go | 230 +++++++++++++++++++++++++++++++++++-------------- digest_test.go | 65 +++++++------- 3 files changed, 198 insertions(+), 99 deletions(-) diff --git a/.gitignore b/.gitignore index 112ea39..9a2bff0 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ *.6 *.out _testmain.go +.vscode + diff --git a/digest.go b/digest.go index 3681dc2..88a8bd3 100644 --- a/digest.go +++ b/digest.go @@ -86,6 +86,9 @@ func (a *DigestAuth) Purge(count int) { } } +const DigestAlgorithm = "MD5" +const DigestQop = "auth" + /* http.Handler for DigestAuth which initiates the authentication process (or requires reauthentication). @@ -100,7 +103,8 @@ func (a *DigestAuth) RequireAuth(w http.ResponseWriter, r *http.Request, stale b nonce := RandomKey() a.clients[nonce] = &digest_client{ncs_seen: NewBitSet(a.NcCacheSize), last_seen: time.Now().UnixNano()} - value := fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="MD5", qop="auth"`, a.Realm, nonce, a.Opaque) + value := fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="%s", qop="%s"`, a.Realm, nonce, + a.Opaque, DigestAlgorithm, DigestQop) if stale { value += ", stale=true" } @@ -122,59 +126,149 @@ func (a *DigestAuth) DigestAuthParams(r *http.Request) map[string]string { return ParsePairs(s[1]) } +type DigestAuthResult struct { + Username string + Authinfo string +} + +var ErrDigestAuthMissing = fmt.Errorf("missing digest auth header") +var ErrDigestOpaqueMismatch = fmt.Errorf("client opaque does not match server opaque") +var ErrDigestAlgorithmMismatch = fmt.Errorf("algorithm mismatch; expected %s", DigestAlgorithm) +var ErrDigestQopMismatch = fmt.Errorf("qop mismatch; expected %s", DigestQop) +var ErrDigestResponseMismatch = fmt.Errorf("response mismatch") +var ErrDigestRepeatedNc = fmt.Errorf("repeated nc! (replay attack?)") +var ErrDigestStaleNonce = fmt.Errorf("stale nonce") + +type ErrDigestUriMismatch struct { + fromRequest string + fromAuth string +} + +func (e ErrDigestUriMismatch) Error() string { + return fmt.Sprintf("path mismatch: %s != %s", e.fromRequest, e.fromAuth) +} + +type ErrDigestHostMismatch struct { + fromRequest string + fromAuth string +} + +func (e ErrDigestHostMismatch) Error() string { + return fmt.Sprintf("host mismatch: %s != %s", e.fromRequest, e.fromAuth) +} + +type ErrDigestBadNc struct { + err error +} + +func (e ErrDigestBadNc) Error() string { + return fmt.Sprintf("failed to parse nc: %s", e.err) +} + +type ErrDigestBadUri struct { + uri string +} + +func (e ErrDigestBadUri) Error() string { + return fmt.Sprintf("failed to parse uri: %s", e.uri) +} + /* - Check if request contains valid authentication data. Returns a triplet - of username, authinfo, stale where username is the name of the authenticated - user or an empty string, authinfo is the contents for the optional Authentication-Info - response header, and stale indicates whether the server-returned Authenticate header - should specify stale=true (see https://www.ietf.org/rfc/rfc2617.txt Section 3.3) + CheckAuth checks whether the request contains valid authentication data. Returns + a tuple of DigestAuthResult, error. On success, err is nil and the result contains + the name of the authenticated user and authinfo for the contents of the optional + XYZ-Authentication-Info response header. If err==ErrDigestStaleNonce then the caller + should specify stale=true (see https://www.ietf.org/rfc/rfc2617.txt Section 3.3) when + sending the XYZ-Authenticate header. */ -func (da *DigestAuth) CheckAuth(r *http.Request) (username string, authinfo *string, stale bool) { +func (da *DigestAuth) CheckAuth(r *http.Request) (*DigestAuthResult, error) { da.mutex.Lock() defer da.mutex.Unlock() - username = "" - authinfo = nil - stale = false + result := &DigestAuthResult{} auth := da.DigestAuthParams(r) - if auth == nil || da.Opaque != auth["opaque"] || auth["algorithm"] != "MD5" || auth["qop"] != "auth" { - return + if auth == nil { + return nil, ErrDigestAuthMissing + } else if da.Opaque != auth["opaque"] { + return nil, ErrDigestOpaqueMismatch + } else if auth["algorithm"] != DigestAlgorithm { + return nil, ErrDigestAlgorithmMismatch + } else if auth["qop"] != DigestQop { + return nil, ErrDigestQopMismatch } - /* Check whether the requested URI matches auth header - NOTE: when we're a proxy and method is CONNECT, the request and auth uri - specify a hostname not a path, e.g. - - CONNECT 1-edge-chat.facebook.com:443 HTTP/1.1 - User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0 - Proxy-Connection: keep-alive - Connection: keep-alive - Host: 1-edge-chat.facebook.com:443 - Proxy-Authorization: Digest username="test", realm="", - nonce="iQSz9RcA1Qsa6ono", - uri="1-edge-chat.facebook.com:443", - algorithm=MD5, - response="a077a4676d60ff8bf48577ad7c7360d6", - opaque="EN3BwDsuWB5F6IWR", qop=auth, nc=0000000c, - cnonce="548d04d1bbd63926" - */ + // Checking the proxy auth uri is surprisingly difficult... + // + // Typical examples: note that querystring is included in Proxy-Authorization uri so we + // must strip it off before comparing to our request path + // + // GET http://start.ubuntu.com/14.04/Google/?sourceid=hp HTTP/1.1 + // User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0 + // Host: start.ubuntu.com + // Proxy-Authorization: Digest username="test", realm="Proxy", nonce="FhnRLUZSHgPUhw5S", uri="/14.04/Google/?sourceid=hp", + // algorithm=MD5, response="c9f50ab9dd1b1a67c8ca03d1e1c4668a", opaque="6d33bebd35010c78c846cec1ed34373d", qop=auth, nc=00000001, cnonce="87e41ec0b553d1e3" + // + // For connect, there is no path so we compare hostname and port + // + // CONNECT 2.rto.microsoft.com:443 HTTP/1.1 + // Host: 2.rto.microsoft.com:443 + // User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0 + // Proxy-Authorization: Digest username="test", realm="Proxy", nonce="69uac299Qm9CdrHd", uri="2.rto.microsoft.com:443", + // algorithm=MD5, response="30d5eb727a1aaea879599d813bcaef57", opaque="6d33bebd35010c78c846cec1ed34373d", qop=auth, nc=00000001, cnonce="3057a2b17430ba89" + // + // Except that sometimes the ports don't match, so we have to exclude them from the matching logic... + // + // CONNECT core25usw2.fabrik.nytimes.com.:80 HTTP/1.1 + // User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0 + // Host: core25usw2.fabrik.nytimes.com.:80 + // Proxy-Authorization: Digest username="test", realm="Proxy", nonce="r+10+JoybdZlWpaW", uri="core25usw2.fabrik.nytimes.com.:443", + // algorithm=MD5, response="a827b29b872613509052ff8b68e3b365", opaque="6d33bebd35010c78c846cec1ed34373d", qop=auth, nc=00000001, cnonce="70876e4bbcdb1e4a" + // + // Or clients send malformed data (like the extra slashes here). It's not clear whether the client is sending malformed data, or whether they + // are trying to use protocol-relative addressing and the golang url parser just can't handle it. + // The request line path is parsed as "//www/delivery/retarget.php"" but the proxy auth uri path is parsed as "/delivery/retarget.php" + // + // GET http://ap.lijit.com//www/delivery/retarget.php?a=a&r=rtb_criteo&pid=9&3pid=1bbc9197-bea9-4fad-892a-f9ac383cbccd&cb=e1791e3c-eb07-4396-91e1-7bcfce7d3e17 HTTP/1.1 + // Host: ap.lijit.com + // User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0^M + // Proxy-Authorization: Digest username="test", realm="Proxy", nonce="fRFG/XucPibCZ+wy", uri="//www/delivery/retarget.php?a=a&r=rtb_criteo&pid=9&3pid=1bbc9197-bea9-4fad-892a-f9ac383cbccd&cb=e1791e3c-eb07-4396-91e1-7bcfce7d3e17", + // algorithm=MD5, response="58c7c6b5b8419f9a87d5e81e87ef2027", opaque="6d33bebd35010c78c846cec1ed34373d", qop=auth, nc=00000001, cnonce="620ce89123d29c75"^M + // + // Another example. The request line path is parsed as "//dynamic_preroll_playlist.fmil" and the proxy auth uri path is parsed as "" (seen on independent.co.uk) + // + // POST http://plg2.yumenetworks.com//dynamic_preroll_playlist.fmil?domain=2158REpjOITz&yvbsv=6.2.9.3&&protocol_version=2.0&sdk_ver=3.1.9.20&width=300&height=250&embeddedIn=http%3A%2F%2Fwww.independent.co.uk%2Fnews%2Fworld%2Famericas& + // sdk_url=http%3A%2F%2Fplg1.yumenetworks.com%2Fyvp%2F20%2Fvpaid%2Fcr%2F&viewport=970,1219,0,0,300,250,970,1447&ytp=0,228,1301,673,970,1447,1301,6843&ypt=none& HTTP/1.1^M + // Host: plg2.yumenetworks.com^M + // Proxy-Authorization: Digest username="magnus", realm="Magthor Proxy", nonce="yV12jLlyzAb7Lo9R", uri="//dynamic_preroll_playlist.fmil?domain=2158REpjOITz&yvbsv=6.2.9.3&&protocol_version=2.0&sdk_ver=3.1.9.20&width=300&height=250& + // embeddedIn=http%3A%2F%2Fwww.independent.co.uk%2Fnews%2Fworld%2Famericas&sdk_url=http%3A%2F%2Fplg1.yumenetworks.com%2Fyvp%2F20%2Fvpaid%2Fcr%2F&viewport=970,1219,0,0,300,250,970,1447&ytp=0,228,1301,673,970,1447,1301,6843&ypt=none&", algorithm=MD5, + // response="0813fb545b1558bef224b4ecdedf0e2f", opaque="6d33bebd35010c78c846cec1ed34373d", qop=auth, nc=00000279, cnonce="e59ed7e19d323c8b"^M + // + + // We have to parse auth["uri"] instead of comparing directly since it could contain url-escaped chars + authPath, err := url.Parse(auth["uri"]) + if err != nil { + return nil, &ErrDigestBadUri{auth["uri"]} + } - if r.Method == "CONNECT" { - if r.RequestURI != auth["uri"] { - return + if r.URL.Path == "" { + // e.g. CONNECT + // compare without port numbers, if any + if strings.Split(r.RequestURI, ":")[0] != strings.Split(auth["uri"], ":")[0] { + return nil, &ErrDigestHostMismatch{r.RequestURI, auth["uri"]} } } else { - - // Check if the requested URI matches auth header - switch u, err := url.Parse(auth["uri"]); { - case err != nil: - return - case r.URL == nil: - return - case len(u.Path) > len(r.URL.Path): - return - case !strings.HasPrefix(r.URL.Path, u.Path): - return + // e.g. GET + if authPath.Path == "" { + // e.g. path like "//dynamic_preroll_playlist.fmil?..." which isn't parsed correctly + compare := strings.Split(auth["uri"], "?")[0] + if r.URL.Path != compare { + return nil, &ErrDigestUriMismatch{r.URL.Path, compare + " (?)"} + } + } else if r.URL.Path != authPath.Path { + return nil, &ErrDigestUriMismatch{r.URL.Path, authPath.Path} } + //if !strings.HasPrefix(authPath.Path, r.URL.Path) { + // return nil, &ErrDigestUriMismatch{r.URL.Path, authPath.Path} + //} } HA1 := da.Secrets(auth["username"], da.Realm) @@ -182,44 +276,48 @@ func (da *DigestAuth) CheckAuth(r *http.Request) (username string, authinfo *str HA1 = H(auth["username"] + ":" + da.Realm + ":" + HA1) } HA2 := H(r.Method + ":" + auth["uri"]) + + // NOTE: it could be that the client nonce doesn't match ours (we check that later), but this calc + // verifies they provided the correct user password KD := H(strings.Join([]string{HA1, auth["nonce"], auth["nc"], auth["cnonce"], auth["qop"], HA2}, ":")) if subtle.ConstantTimeCompare([]byte(KD), []byte(auth["response"])) != 1 { - return + return nil, ErrDigestResponseMismatch } // At this point crypto checks are completed and validated. // Now check if the session is valid. - nc, err := strconv.ParseUint(auth["nc"], 16, 64) if err != nil { - return + return nil, &ErrDigestBadNc{err} } client, ok := da.clients[auth["nonce"]] if !ok { - stale = true - return + return nil, ErrDigestStaleNonce } // Check the nonce-count if nc >= client.ncs_seen.Size() { // nc exceeds the size of our bitset. We can just treat this the // same as a stale nonce - stale = true - return + return nil, ErrDigestStaleNonce } else if client.ncs_seen.Get(nc) { // We've already seen this nc! Possible replay attack! - return + return nil, ErrDigestRepeatedNc } + + // NOTE: we don't register that we've seen this nc until this point; not sure if that is correct. It may + // be preferable to parse nonce and nc as one of the first operations so that we get nc stored in our bitmap + // regardless of whether or not the request successfully passes authorization client.ncs_seen.Set(nc) client.last_seen = time.Now().UnixNano() resp_HA2 := H(":" + auth["uri"]) rspauth := H(strings.Join([]string{HA1, auth["nonce"], auth["nc"], auth["cnonce"], auth["qop"], resp_HA2}, ":")) - - info := fmt.Sprintf(`qop="auth", rspauth="%s", cnonce="%s", nc="%s"`, rspauth, auth["cnonce"], auth["nc"]) - return auth["username"], &info, stale + result.Authinfo = fmt.Sprintf(`qop="%s", rspauth="%s", cnonce="%s", nc="%s"`, DigestQop, rspauth, auth["cnonce"], auth["nc"]) + result.Username = auth["username"] + return result, nil } /* @@ -239,13 +337,11 @@ const DefaultClientCacheTolerance = 100 */ func (a *DigestAuth) Wrap(wrapped AuthenticatedHandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - if username, authinfo, stale := a.CheckAuth(r); username == "" { - a.RequireAuth(w, r, stale) + if result, err := a.CheckAuth(r); err != nil { + a.RequireAuth(w, r, err == ErrDigestStaleNonce) } else { - ar := &AuthenticatedRequest{Request: *r, Username: username} - if authinfo != nil { - w.Header().Set(AuthenticationInfoHeaderName(a.IsProxy), *authinfo) - } + ar := &AuthenticatedRequest{Request: *r, Username: result.Username} + w.Header().Set(AuthenticationInfoHeaderName(a.IsProxy), result.Authinfo) wrapped(w, ar) } } @@ -265,11 +361,11 @@ func (a *DigestAuth) JustCheck(wrapped http.HandlerFunc) http.HandlerFunc { // NewContext returns a context carrying authentication information for the request. func (a *DigestAuth) NewContext(ctx context.Context, r *http.Request) context.Context { - username, authinfo, stale := a.CheckAuth(r) - info := &Info{Username: username, ResponseHeaders: make(http.Header)} - if username != "" { + result, err := a.CheckAuth(r) + info := &Info{Username: result.Username, ResponseHeaders: make(http.Header)} + if err == nil { info.Authenticated = true - info.ResponseHeaders.Set(AuthenticationInfoHeaderName(a.IsProxy), *authinfo) + info.ResponseHeaders.Set(AuthenticationInfoHeaderName(a.IsProxy), result.Authinfo) } else { // return back digest XYZ-Authenticate header if len(a.clients) > a.ClientCacheSize+a.ClientCacheTolerance { @@ -278,9 +374,9 @@ func (a *DigestAuth) NewContext(ctx context.Context, r *http.Request) context.Co nonce := RandomKey() a.clients[nonce] = &digest_client{ncs_seen: NewBitSet(a.NcCacheSize), last_seen: time.Now().UnixNano()} - value := fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="MD5", qop="auth"`, - a.Realm, nonce, a.Opaque) - if stale { + value := fmt.Sprintf(`Digest realm="%s", nonce="%s", opaque="%s", algorithm="%s", qop="%s"`, + a.Realm, nonce, a.Opaque, DigestAlgorithm, DigestQop) + if err == ErrDigestStaleNonce { value += ", stale=true" } info.ResponseHeaders.Set(AuthenticateHeaderName(a.IsProxy), value) diff --git a/digest_test.go b/digest_test.go index ef9395e..121077f 100644 --- a/digest_test.go +++ b/digest_test.go @@ -2,6 +2,7 @@ package auth import ( "bufio" + "fmt" "net/http" "net/url" "strings" @@ -21,78 +22,78 @@ func TestAuthDigest(t *testing.T) { clients: map[string]*digest_client{}} r := &http.Request{} r.Method = "GET" - if u, _, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for empty request header") + if _, err := da.CheckAuth(r); err == nil { + t.Fatal("successful auth for missing request header") } r.Header = http.Header(make(map[string][]string)) r.Header.Set(AuthorizationHeaderName(da.IsProxy), "Digest blabla") - if u, _, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for bad request header") + if _, err := da.CheckAuth(r); err == nil { + t.Fatal("successful auth for bad request header") } r.URL, _ = url.Parse("/t2") r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="Vb9BP/h81n3GpTTB", uri="/t2", cnonce="NjE4MTM2", nc=00000001, qop="auth", response="ffc357c4eba74773c8687e0bc724c9a3", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) - u, _, stale := da.CheckAuth(r) - if u != "" { - t.Fatal("non-empty auth for unknown client") + result, err := da.CheckAuth(r) + if err == nil { + t.Fatal("successful auth for unknown client") } - if !stale { + if err != ErrDigestStaleNonce { + fmt.Println("err: ", err) t.Fatal("stale should be true") } da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{ncs_seen: NewBitSet(da.NcCacheSize), last_seen: time.Now().UnixNano()} - u, _, stale = da.CheckAuth(r) - if u != "test" { - t.Fatal("empty auth for legitimate client") - } - if stale { - t.Fatal("stale should be false") + result, err = da.CheckAuth(r) + if result.Username != "test" { + t.Fatal("failed auth for legitimate client") } - r.URL, _ = url.Parse("/") + r.URL.Path = "/" da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{ncs_seen: NewBitSet(da.NcCacheSize), last_seen: time.Now().UnixNano()} - if u, _, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for bad request path") + if _, err := da.CheckAuth(r); err == nil { + t.Fatal("successful auth for bad request path") } - r.URL, _ = url.Parse("/t3") + r.URL.Path = "/t3" da.clients["Vb9BP/h81n3GpTTB"] = &digest_client{ncs_seen: NewBitSet(da.NcCacheSize), last_seen: time.Now().UnixNano()} - if u, _, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for bad request path") + if _, err := da.CheckAuth(r); err == nil { + t.Fatal("successful auth for bad request path") } + // we don't support subpaths anymore, it's not clear that was ever correct... da.clients["+RbVXSbIoa1SaJk1"] = &digest_client{ncs_seen: NewBitSet(da.NcCacheSize), last_seen: time.Now().UnixNano()} r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000001, qop="auth", response="c08918024d7faaabd5424654c4e3ad1c", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) - if u, _, _ := da.CheckAuth(r); u != "test" { - t.Fatal("empty auth for valid request in subpath") + if _, err := da.CheckAuth(r); err == nil { + t.Fatal("successful auth for pad request path") } // nc checking, we've already seen 00000001 so this should fail - if u, _, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for already-seen nc") + if _, err := da.CheckAuth(r); err == nil { + t.Fatal("successful auth for already-seen nc") } + r.URL.Path = "/" // an updated request with nc 00000005 should succeed r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000005, qop="auth", response="c553c9a48ec99de9474e662934f73de2", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) - if u, _, _ := da.CheckAuth(r); u != "test" { - t.Fatal("empty auth for valid nc 00000005") + if _, err := da.CheckAuth(r); err != nil { + t.Fatal("failed auth for valid nc 00000005: ", err) } // but repeating it should fail... r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000005, qop="auth", response="c553c9a48ec99de9474e662934f73de2", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) - if u, _, _ := da.CheckAuth(r); u != "" { - t.Fatal("non-empty auth for repeated nc 00000005") + if _, err := da.CheckAuth(r); err == nil { + t.Fatal("successful auth for repeated nc 00000005") } // an updated request with nc 00000002 should succeed even though it's out of order, since it hasn't been seen yet r.Header.Set(AuthorizationHeaderName(da.IsProxy), `Digest username="test", realm="example.com", nonce="+RbVXSbIoa1SaJk1", uri="/", cnonce="NjE4NDkw", nc=00000002, qop="auth", response="1c2a64978d9e8a61f823240304b95afd", opaque="U7H+ier3Ae8Skd/g", algorithm="MD5"`) - if u, _, _ := da.CheckAuth(r); u != "test" { - t.Fatal("empty auth for valid nc 00000002") + if _, err := da.CheckAuth(r); err != nil { + t.Fatal("failed auth for valid nc 00000002: ", err) } - if da.clients["+RbVXSbIoa1SaJk1"].ncs_seen.String() != "01100100000000000000" { - t.Fatal("ncs_seen bitmap didn't match expected") + if da.clients["+RbVXSbIoa1SaJk1"].ncs_seen.String() != "00100100000000000000" { + t.Fatal("ncs_seen bitmap didn't match expected: ", da.clients["+RbVXSbIoa1SaJk1"].ncs_seen.String()) } }