diff --git a/crypto/dpop/dpop.go b/crypto/dpop/dpop.go index 3f8ad8b932..5e6e585814 100644 --- a/crypto/dpop/dpop.go +++ b/crypto/dpop/dpop.go @@ -30,7 +30,6 @@ import ( "net/http" "net/url" "slices" - "strings" "time" "github.com/lestrrat-go/jwx/v2/jwa" @@ -156,10 +155,20 @@ func Parse(s string) (*DPoP, error) { if token.IssuedAt().IsZero() { return nil, fmt.Errorf("%w: missing iat claim", ErrInvalidDPoP) } - if v, ok := token.Get(HTUKey); !ok || v == "" { + if v, ok := token.Get(HTUKey); !ok { return nil, fmt.Errorf("%w: missing htu claim", ErrInvalidDPoP) + } else if htu, ok := v.(string); !ok { + return nil, fmt.Errorf("%w: invalid htu claim", ErrInvalidDPoP) + } else if htu == "" { + return nil, fmt.Errorf("%w: missing htu claim", ErrInvalidDPoP) + } else if _, err := url.Parse(htu); err != nil { + return nil, fmt.Errorf("%w: invalid htu claim: %w", ErrInvalidDPoP, err) } - if v, ok := token.Get(HTMKey); !ok || v == "" { + if v, ok := token.Get(HTMKey); !ok { + return nil, fmt.Errorf("%w: missing htm claim", ErrInvalidDPoP) + } else if htm, ok := v.(string); !ok { + return nil, fmt.Errorf("%w: invalid htm claim", ErrInvalidDPoP) + } else if htm == "" { return nil, fmt.Errorf("%w: missing htm claim", ErrInvalidDPoP) } if token.JwtID() == "" { @@ -220,8 +229,14 @@ func (t DPoP) Match(jkt string, method string, url string) (bool, error) { if method != t.HTM() { return false, fmt.Errorf("method mismatch, token: %s, given: %s", t.HTM(), method) } - urlLeft := strip(t.HTU()) - urlRight := strip(url) + urlLeft, err := strip(t.HTU()) + if err != nil { + return false, fmt.Errorf("invalid htu claim: %w", err) + } + urlRight, err := strip(url) + if err != nil { + return false, fmt.Errorf("invalid url: %w", err) + } if urlLeft != urlRight { return false, fmt.Errorf("url mismatch, token: %s, given: %s", urlLeft, urlRight) } @@ -229,13 +244,18 @@ func (t DPoP) Match(jkt string, method string, url string) (bool, error) { return true, nil } -func strip(raw string) string { - url, _ := url.Parse(raw) - url.Scheme = "https" - url.Host = strings.Split(url.Host, ":")[0] - url.RawQuery = "" - url.Fragment = "" - return url.String() +func strip(raw string) (string, error) { + u, err := url.Parse(raw) + if err != nil { + return "", err + } + u.Scheme = "https" + if host := u.Hostname(); host != "" { + u.Host = host + } + u.RawQuery = "" + u.Fragment = "" + return u.String(), nil } func (t DPoP) MarshalJSON() ([]byte, error) { diff --git a/crypto/dpop/dpop_test.go b/crypto/dpop/dpop_test.go index 908fb8c7de..fb2030d1ed 100644 --- a/crypto/dpop/dpop_test.go +++ b/crypto/dpop/dpop_test.go @@ -229,6 +229,40 @@ func TestParseDPoP(t *testing.T) { require.Error(t, err) assert.EqualError(t, err, "invalid DPoP token: jti claim too long") }) + t.Run("invalid htu claim URL", func(t *testing.T) { + // Regression test: url.Parse returns (nil, error) for invalid percent-encoded + // sequences. Without the fix, strip() dereferenced the nil pointer, panicking + // the server when Match() was called on a token with htu="%zz". + dpopToken := New(*request) + _ = dpopToken.Token.Set(HTUKey, "%zz") + dpopString, _ := dpopToken.Sign("kid", keyPair, alg) + + _, err := Parse(dpopString) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidDPoP) + assert.Contains(t, err.Error(), "invalid htu claim") + }) + t.Run("non-string htu claim", func(t *testing.T) { + dpopToken := New(*request) + _ = dpopToken.Token.Set(HTUKey, 42) + dpopString, _ := dpopToken.Sign("kid", keyPair, alg) + + _, err := Parse(dpopString) + + require.Error(t, err) + assert.EqualError(t, err, "invalid DPoP token: invalid htu claim") + }) + t.Run("non-string htm claim", func(t *testing.T) { + dpopToken := New(*request) + _ = dpopToken.Token.Set(HTMKey, 42) + dpopString, _ := dpopToken.Sign("kid", keyPair, alg) + + _, err := Parse(dpopString) + + require.Error(t, err) + assert.EqualError(t, err, "invalid DPoP token: invalid htm claim") + }) } func TestDPoP_Match(t *testing.T) { diff --git a/go.sum b/go.sum index 2ebaaae442..80d16f4f43 100644 --- a/go.sum +++ b/go.sum @@ -398,8 +398,6 @@ github.com/nuts-foundation/crypto-ecies v0.0.0-20211207143025-5b84f9efce2b h1:80 github.com/nuts-foundation/crypto-ecies v0.0.0-20211207143025-5b84f9efce2b/go.mod h1:6YUioYirD6/8IahZkoS4Ypc8xbeJW76Xdk1QKcziNTM= github.com/nuts-foundation/go-did v0.18.0 h1:IB0X8PrzDulpR1zAgDpaHfwoSjJpIhx5u1Tg8I2nnb8= github.com/nuts-foundation/go-did v0.18.0/go.mod h1:4od1gAmCi9HjHTQGEvHC8pLeuXdXACxidAcdA52YScc= -github.com/nuts-foundation/go-leia/v4 v4.2.0 h1:o/bgYVCyTgsfgtaKmlrcUaJ2z4NwetERC98SUWwYajM= -github.com/nuts-foundation/go-leia/v4 v4.2.0/go.mod h1:Gw6bXqJLOAmHSiXJJYbVoj+Mowp/PoBRywO0ZPsVzA0= github.com/nuts-foundation/go-leia/v4 v4.3.0 h1:R0qGISIeg2q/PCQTC9cuoBtA6cFu4WBV2DbmSOWKZyM= github.com/nuts-foundation/go-leia/v4 v4.3.0/go.mod h1:Gw6bXqJLOAmHSiXJJYbVoj+Mowp/PoBRywO0ZPsVzA0= github.com/nuts-foundation/go-stoabs v1.11.0 h1:q18jVruPdFcVhodDrnKuhq/24i0pUC/YXgzJS0glKUU=