Skip to content

Cherry-pick JWT CVE fix into master#2716

Merged
istio-testing merged 1 commit intoistio:masterfrom
yangminzhu:cp-jwt-fix
Feb 26, 2020
Merged

Cherry-pick JWT CVE fix into master#2716
istio-testing merged 1 commit intoistio:masterfrom
yangminzhu:cp-jwt-fix

Conversation

@yangminzhu
Copy link
Contributor

Problem: The JWT filter when matching exact paths included query parameters
which meant the JWT requirement could be bypassed by adding a "?" after the
path. The API was intended to only work for URIs.

Solution: The fix updates the match logic to only include URIs i.e. path
stripped off the query section.

Added unit tests to validate these cases.

  • Fixed formatting

  • Strip fragment of Path

Added unit tests to validate combination of query & fragment

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

@yangminzhu yangminzhu added the cherrypick/release-1.5 Set this label on a PR to auto-merge it to the release-1.5 branch label Feb 25, 2020
@yangminzhu yangminzhu added this to the 1.5 milestone Feb 25, 2020
@yangminzhu yangminzhu requested a review from a team February 25, 2020 23:49
@googlebot
Copy link
Collaborator

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. label Feb 25, 2020
@yangminzhu yangminzhu changed the title Cherry-pick JWT CVE fix into Cherry-pick JWT CVE fix into master Feb 25, 2020
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 25, 2020
* Fixed JWT CVE related to exact PATH matches (istio#9)

* Fixed JWT CVE related to exact PATH matches

Problem: The JWT filter when matching exact paths included query parameters
which meant the JWT requirement could be bypassed by adding a "?" after the
path. The API was intended to only work for URIs.

Solution: The fix updates the match logic to only include URIs i.e. path
stripped off the query section.

Added unit tests to validate these cases.

* Fixed formatting

* Strip fragment of Path

Added unit tests to validate combination of query & fragment

* Fix lint

* Minor refactoring and more unit test cases (istio#11)

* Minor refactoring and more unit test cases

* Lint fixes

Signed-off-by: Yangmin Zhu <ymzhu@google.com>
@yangminzhu yangminzhu added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 25, 2020
@googlebot
Copy link
Collaborator

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@yangminzhu yangminzhu removed the cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. label Feb 25, 2020
@yangminzhu
Copy link
Contributor Author

Manually set cla: yes as @nrjpoddar has committed the same change to other branches.

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new pull request created: #2718

@yangminzhu yangminzhu deleted the cp-jwt-fix branch February 26, 2020 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherrypick/release-1.5 Set this label on a PR to auto-merge it to the release-1.5 branch cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants