Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

1.16.x: revert bound audiences behavior change #310

Merged
merged 3 commits into from
Jun 13, 2024

Conversation

fairclothjm
Copy link
Contributor

Description

This PR proposes to revert the change made in #295 to address CVE-2024-5798.

We add a new test TestLoginBoundAudiences to test the behavior expectations.

Background

#295 fixed a bug where the login JWT's aud claim is ignored if it is a single string. The net effect is that the role's bound_audiences parameter must match at least one of the JWT's aud claims if the token has an aud claim.

Since the behavior change is a breaking change, we are reverting the change in the Vault versions after 1.15.10 and 1.16.4. However, the behavior change will go into effect in Vault 1.17 and later.

Relates

@@ -270,69 +446,6 @@ func testLogin_JWT(t *testing.T, jwks bool) {
}
}

// Test bound audiences unset, claims "aud" set
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test case is captured in the new test

@fairclothjm fairclothjm marked this pull request as ready for review June 13, 2024 18:15
@fairclothjm fairclothjm requested a review from thyton June 13, 2024 18:23
@fairclothjm fairclothjm merged commit 3962fb9 into release/vault-1.16.x Jun 13, 2024
4 checks passed
@fairclothjm fairclothjm deleted the revert/release/1.16.x/bound-aud branch June 13, 2024 20:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants