Skip to content

Commit

Permalink
Fix digest auth (#789)
Browse files Browse the repository at this point in the history
* fix: handle commas in digest challenge values

Reimplement parsing of digest auth challenge to handle cases where
the values of key/value pairs contain commas, such as in
qop="auth, auth-int"

* fix: digest auth handle qop values separated without spaces

The digest auth qop validation did only handle values separated
like "auth, auth-int", but not "auth,auth-int".
  • Loading branch information
phw authored Apr 16, 2024
1 parent 733395c commit 877d7e3
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 33 deletions.
2 changes: 2 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func TestClientDigestAuth(t *testing.T) {
func TestClientDigestSession(t *testing.T) {
conf := defaultDigestServerConf()
conf.algo = "MD5-sess"
conf.qop = "auth, auth-int"
ts := createDigestServer(t, conf)
defer ts.Close()

Expand Down Expand Up @@ -134,6 +135,7 @@ func TestClientDigestErrors(t *testing.T) {
{mutateConf: func(c *digestServerConfig) { c.uri = "/bad" }, expect: ErrDigestBadChallenge},
{mutateConf: func(c *digestServerConfig) { c.uri = "/unknown_param" }, expect: ErrDigestBadChallenge},
{mutateConf: func(c *digestServerConfig) { c.uri = "/missing_value" }, expect: ErrDigestBadChallenge},
{mutateConf: func(c *digestServerConfig) { c.uri = "/unclosed_quote" }, expect: ErrDigestBadChallenge},
{mutateConf: func(c *digestServerConfig) { c.uri = "/no_challenge" }, expect: ErrDigestBadChallenge},
{mutateConf: func(c *digestServerConfig) { c.uri = "/status_500" }, expect: nil},
}
Expand Down
98 changes: 65 additions & 33 deletions digest.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) 2015-2023 Jeevanandam M (jeeva@myjeeva.com)
// 2023 Segev Dagan (https://github.com/segevda)
// 2024 Philipp Wolfer (https://github.com/phw)
// All rights reserved.
// resty source code and usage is governed by a MIT style
// license that can be found in the LICENSE file.
Expand Down Expand Up @@ -125,48 +126,78 @@ type challenge struct {
userhash string
}

func (c *challenge) setValue(k, v string) error {
switch k {
case "realm":
c.realm = v
case "domain":
c.domain = v
case "nonce":
c.nonce = v
case "opaque":
c.opaque = v
case "stale":
c.stale = v
case "algorithm":
c.algorithm = v
case "qop":
c.qop = v
case "charset":
if strings.ToUpper(v) != "UTF-8" {
return ErrDigestCharset
}
case "userhash":
c.userhash = v
default:
return ErrDigestBadChallenge
}
return nil
}

func parseChallenge(input string) (*challenge, error) {
const ws = " \n\r\t"
const qs = `"`
s := strings.Trim(input, ws)
if !strings.HasPrefix(s, "Digest ") {
return nil, ErrDigestBadChallenge
}
s = strings.Trim(s[7:], ws)
sl := strings.Split(s, ",")
c := &challenge{}
var r []string
for i := range sl {
sl[i] = strings.TrimSpace(sl[i])
r = strings.SplitN(sl[i], "=", 2)
if len(r) != 2 {
return nil, ErrDigestBadChallenge
}
r[0] = strings.TrimSpace(r[0])
r[1] = strings.TrimSpace(r[1])
switch r[0] {
case "realm":
c.realm = strings.Trim(r[1], qs)
case "domain":
c.domain = strings.Trim(r[1], qs)
case "nonce":
c.nonce = strings.Trim(r[1], qs)
case "opaque":
c.opaque = strings.Trim(r[1], qs)
case "stale":
c.stale = strings.Trim(r[1], qs)
case "algorithm":
c.algorithm = strings.Trim(r[1], qs)
case "qop":
c.qop = strings.Trim(r[1], qs)
case "charset":
if strings.ToUpper(strings.Trim(r[1], qs)) != "UTF-8" {
return nil, ErrDigestCharset
b := strings.Builder{}
key := ""
quoted := false
for _, r := range s {
switch r {
case '"':
quoted = !quoted
case ',':
if quoted {
b.WriteRune(r)
} else {
val := strings.Trim(b.String(), ws)
b.Reset()
if err := c.setValue(key, val); err != nil {
return nil, err
}
key = ""
}
case '=':
if quoted {
b.WriteRune(r)
} else {
key = strings.Trim(b.String(), ws)
b.Reset()
}
case "userhash":
c.userhash = strings.Trim(r[1], qs)
default:
return nil, ErrDigestBadChallenge
b.WriteRune(r)
}
}
if quoted || (key == "" && b.Len() > 0) {
return nil, ErrDigestBadChallenge
}
if key != "" {
val := strings.Trim(b.String(), ws)
if err := c.setValue(key, val); err != nil {
return nil, err
}
}
return c, nil
Expand Down Expand Up @@ -233,9 +264,10 @@ func (c *credentials) validateQop() error {
if c.messageQop == "" {
return ErrDigestNoQop
}
possibleQops := strings.Split(c.messageQop, ", ")
possibleQops := strings.Split(c.messageQop, ",")
var authSupport bool
for _, qop := range possibleQops {
qop = strings.TrimSpace(qop)
if qop == "auth" {
authSupport = true
break
Expand Down
3 changes: 3 additions & 0 deletions resty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,9 @@ func createDigestServer(t *testing.T, conf *digestServerConfig) *httptest.Server
case "/missing_value":
setWWWAuthHeader(w, `Digest realm="hello", domain`)
return
case "/unclosed_quote":
setWWWAuthHeader(w, `Digest realm="hello, qop=auth`)
return
case "/no_challenge":
setWWWAuthHeader(w, "")
return
Expand Down

0 comments on commit 877d7e3

Please sign in to comment.