Skip to content

Verifying multiple audiences produces undeterministic results #442

Open
@sfinnman-cotn

Description

@sfinnman-cotn

Hello! I posted an MR fixing this issue already at #441

Here is the issue briefly:

The code which verifies multiple audiences, namely:

func (v *Validator) verifyAudience(claims Claims, cmp []string, expectAllAud bool, required bool) error {
	aud, err := claims.GetAudience()
	if err != nil {
		return err
	}

	if len(aud) == 0 {
		return errorIfRequired(required, "aud")
	}

	// use a var here to keep constant time compare when looping over a number of claims
	matching := make(map[string]bool, 0)

	// build a matching hashmap out of the expected aud
	for _, expected := range cmp {
		matching[expected] = false
	}

	// compare the expected aud with the actual aud in a constant time manner by looping over all actual values
	var stringClaims string
	for _, a := range aud {
		a := a
		_, ok := matching[a]
		if ok {
			matching[a] = true
		}

		stringClaims = stringClaims + a
	}

	// check if all expected auds are present
	result := true
	for _, match := range matching {
		if !expectAllAud && match {
			break
		} else if !match {
			result = false
		}
	}

	// case where "" is sent in one or many aud claims
	if stringClaims == "" {
		return errorIfRequired(required, "aud")
	}

	return errorIfFalse(result, ErrTokenInvalidAudience)
}

produces non-deterministic results. The results produced by this method is highly dependent on the order evaluation of the following loop:

	// check if all expected auds are present
	result := true
	for _, match := range matching {
		if !expectAllAud && match {
			break
		} else if !match {
			result = false
		}
	}

As we can read in the implementation, we never set the result to true before breaking in case we don't expect all audiences to match. We also set result on each iteration which doesn't match. This results in false negatives for matching multiple audiences where we dont expect to match all.

The PR i posted addresses this issue and adds tests for the verification of audiences.

Minimum reproducible code:

parser := jwt.NewParser(
	jwt.WithAudience("example.com", "example.net"),
)
_, err := parser.parse("eyJhbGciOiJOT05FIiwidHlwIjoiSldUIn0.eyJhdWQiOiJleGFtcGxlLmNvbSIsInN1YiI6IjEyMzQ1Njc4OTAiLCJuYW1lIjoiSm9obiBEb2UiLCJpYXQiOjE1MTYyMzkwMjJ9.", func (token any) (any, error) {
	return nil, nil
})
if err != nil {
	panic("I died")
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions