Skip to content

Commit

Permalink
fix: strip query and fragment from URL (#15)
Browse files Browse the repository at this point in the history
According to the RFC query and fragment should not be taken into consideration when comparing the request URI to the `htu` claim. The `url` package handles scheme and syntax based normalization.
  • Loading branch information
SalladinBalwer committed Dec 4, 2023
1 parent b2dc025 commit a3842fc
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 15 deletions.
29 changes: 15 additions & 14 deletions parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type ParseOptions struct {
}

// Parse translates a DPoP proof string into a JWT token and parses it with the jwt package (github.com/golang-jwt/jwt/v5).
// It will also validate the proof according to https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-4.3
// It will also validate the proof according to https://datatracker.ietf.org/doc/html/rfc9449#section-4.3
// but not check whether the proof matches a bound access token. It also assumes point 1 is checked by the calling application.
//
// Protected resources should use the 'Validate' function on the returned proof to ensure that the proof matches any bound access token.
Expand All @@ -67,40 +67,44 @@ func Parse(
// Parse the token string
// Ensure that it is a well-formed JWT, that a supported signature algorithm is used,
// that it contains a public key, and that the signature verifies with the public key.
// This satisfies point 2, 5, 6 and 7 in https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-4.3
// This satisfies point 2, 5, 6 and 7 in https://datatracker.ietf.org/doc/html/rfc9449#section-4.3
var claims ProofTokenClaims
dpopToken, err := jwt.ParseWithClaims(tokenString, &claims, keyFunc)
if err != nil {
return nil, errors.Join(ErrInvalidProof, err)
}

// Check that all claims have been populated
// This satisfies point 3 in https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-4.3
// This satisfies point 3 in https://datatracker.ietf.org/doc/html/rfc9449#section-4.3
if claims.Method == "" || claims.URL == "" || claims.ID == "" || claims.IssuedAt == nil {
return nil, errors.Join(ErrInvalidProof, ErrMissingClaims)
}

// Check `typ` JOSE header that it is correct
// This satisfies point 4 in https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-4.3
// This satisfies point 4 in https://datatracker.ietf.org/doc/html/rfc9449#section-4.3
typeHeader := dpopToken.Header["typ"]
if typeHeader == nil || typeHeader.(string) != "dpop+jwt" {
return nil, errors.Join(ErrInvalidProof, ErrUnsupportedJWTType)
}

// Strip the incoming URI of query and fragment according to point 9 in https://datatracker.ietf.org/doc/html/rfc9449#section-4.3
httpURL.RawQuery = ""
httpURL.Fragment = ""

// Check that `htm` and `htu` claims match the HTTP method and URL of the current request.
// This satisfies point 8 and 9 in https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-4.3
// This satisfies point 8 and 9 in https://datatracker.ietf.org/doc/html/rfc9449#section-4.3
if httpMethod != claims.Method || httpURL.String() != claims.URL {
return nil, errors.Join(ErrInvalidProof, ErrIncorrectHTTPTarget)
}

// Check that `nonce` is correct
// This satisfies point 10 in https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-4.3
// This satisfies point 10 in https://datatracker.ietf.org/doc/html/rfc9449#section-4.3
if opts.Nonce != "" && opts.Nonce != claims.Nonce {
return nil, ErrIncorrectNonce
}

// Check that `iat` is within the acceptable window unless `nonce` contains a server managed timestamp.
// This satisfies point 11 in https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-4.3
// This satisfies point 11 in https://datatracker.ietf.org/doc/html/rfc9449#section-4.3
if !opts.NonceHasTimestamp {
// Check that `iat` is not too far into the past.
past := DEFAULT_TIME_WINDOW
Expand Down Expand Up @@ -141,7 +145,7 @@ func Parse(
b64URLjwkHash := base64.RawURLEncoding.EncodeToString(h.Sum(nil))

// Check that `dpop_jkt` is correct if supplied to the authorization server on token request.
// This satisfies https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#name-authorization-code-binding-
// This satisfies https://datatracker.ietf.org/doc/html/rfc9449#name-authorization-code-binding-
if opts.JKT != "" {
if b64URLjwkHash != opts.JKT {
return nil, errors.Join(ErrInvalidProof, ErrIncorrectJKT)
Expand All @@ -155,7 +159,7 @@ func Parse(
}

func keyFunc(t *jwt.Token) (interface{}, error) {
// Return the required jwkHeader header. See https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-4.2
// Return the required jwkHeader header. See https://datatracker.ietf.org/doc/html/rfc9449#section-4.2
// Used to validate the signature of the DPoP proof.
jwkHeader := t.Header["jwk"]
if jwkHeader == nil {
Expand Down Expand Up @@ -275,24 +279,21 @@ func getKeyStringRepresentation(key interface{}) ([]byte, error) {
"x": base64.RawURLEncoding.EncodeToString(key.X.Bytes()),
"y": base64.RawURLEncoding.EncodeToString(key.Y.Bytes()),
}
break
case *rsa.PublicKey:
keyParts = map[string]interface{}{
"kty": "RSA",
"e": base64.RawURLEncoding.EncodeToString(big.NewInt(int64(key.E)).Bytes()),
"n": base64.RawURLEncoding.EncodeToString(key.N.Bytes()),
}
break
case ed25519.PublicKey:
keyParts = map[string]interface{}{
"kty": "OKP",
"crv": "Ed25519",
"x": base64.RawURLEncoding.EncodeToString(key),
}
break
default:
return nil, ErrUnsupportedKeyAlgorithm
}
marshalledKey, err := json.Marshal(keyParts)
return marshalledKey, err

return json.Marshal(keyParts)
}
36 changes: 35 additions & 1 deletion parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import (
"encoding/base64"
"encoding/json"
"errors"
"github.com/golang-jwt/jwt/v5"
"math/big"
"net/url"
"testing"
"time"

"github.com/AxisCommunications/go-dpop"
"github.com/golang-jwt/jwt/v5"
)

// All proofs used in tests have been generated through the use of <https://github.com/panva/dpop>
Expand Down Expand Up @@ -253,6 +253,31 @@ func TestParse_IncorrectHtu(t *testing.T) {
}
}

func TestParse_HtuWithQueryAndFragment(t *testing.T) {
// Arrange
httpUrl, err := url.Parse("https://server.example.com/token?query=true#x/y%2Fz")
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
duration := time.Duration(438000) * time.Hour
opts := dpop.ParseOptions{
Nonce: "",
TimeWindow: &duration,
JKT: "0ZcOCORZNYy-DWpqq30jZyJGHTN0d2HglBV3uiguA4I",
}

// Act
proof, err := dpop.Parse(validES256_proof, dpop.POST, httpUrl, opts)

// Assert
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if proof == nil || proof.Valid != true {
t.Errorf("Expected token to be valid")
}
}

// Test that expired proof is rejected
func TestParse_ExpiredProof(t *testing.T) {
// Arrange
Expand Down Expand Up @@ -592,6 +617,9 @@ func TestParse_ProofWithExtraKeyMembersEC(t *testing.T) {
Method: jwt.SigningMethodES256,
}
tokenString, err := token.SignedString(privateKey)
if err != nil {
t.Error(err)
}
minimalKeyJSON, err := json.Marshal(jwkWithoutOptionalParameters)
if err != nil {
t.Error(err)
Expand Down Expand Up @@ -664,6 +692,9 @@ func TestParse_ProofWithExtraKeyMembersRSA(t *testing.T) {
Method: jwt.SigningMethodRS512,
}
tokenString, err := token.SignedString(rsaKey)
if err != nil {
t.Error(err)
}
minimalKeyJSON, err := json.Marshal(jwkWithoutOptionalParameters)
if err != nil {
t.Error(err)
Expand Down Expand Up @@ -736,6 +767,9 @@ func TestParse_ProofWithExtraKeyMembersOKT(t *testing.T) {
Method: jwt.SigningMethodEdDSA,
}
tokenString, err := token.SignedString(private)
if err != nil {
t.Error(err)
}
minimalKeyJSON, err := json.Marshal(jwkWithoutOptionalParameters)
if err != nil {
t.Error(err)
Expand Down

0 comments on commit a3842fc

Please sign in to comment.