-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: drop path normalization to MERGE_SLASHES to allow apps to handle encoded slashes #330
Conversation
UDSPackage
s
UDSPackage
sUDSPackage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Holding this one hostage until we discuss further, I really don't think we should do this and would rather remove the Istio Path Normalization than add all this complexity.
Yep got no problem with that, was fun learning more about Istio anyway 😅 |
Ok. We'll likely be unable to use path-based AuthorizationPolices with any confidence across all of UDS without careful review of each app individually. I would have liked to have had the ability to use AuthorizationPolices to apply CVE mitigations on UDS apps, see #288 (comment) for more context. In theory we can still do this, but we'll have to review each app's URL decoding implementation before doing so. For Keycloak specifically, At least |
UDSPackage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the fix here!
Dismissing this as the target change pivoted from the original envoyfilter approach.
🤖 I have created a release *beep* *boop* --- ## [0.19.0](v0.18.0...v0.19.0) (2024-04-12) ### Features * add nightly testing eks ([#250](#250)) ([543b09d](543b09d)) ### Bug Fixes * drop path normalization to MERGE_SLASHES to allow apps to handle encoded slashes ([#330](#330)) ([26e965f](26e965f)) * loki bucket configuration service_account and namespace ([#332](#332)) ([9518634](9518634)) ### Miscellaneous * **deps:** update grafana ([#257](#257)) ([c98e566](c98e566)) * **deps:** update metrics-server ([#298](#298)) ([691fd87](691fd87)) * **deps:** update pepr ([#324](#324)) ([2ef0f96](2ef0f96)) * **deps:** update pepr to v0.28.7 ([#321](#321)) ([e7206bb](e7206bb)) * **deps:** update promtail ([#74](#74)) ([6a112b5](6a112b5)) * **deps:** update zarf to v0.32.6 ([#282](#282)) ([443426d](443426d)) * **deps:** update zarf to v0.33.0 ([#325](#325)) ([f2a2a66](f2a2a66)) * update codeowners ([#338](#338)) ([c419574](c419574)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
… encoded slashes (#330) ## Description PR to address `DECODE_AND_MERGE_SLASHES` causing issues in certain applications. Gives the ability to selectively turn this off in a namespace with the `UDSPackage` CR. ## Related Issue Fixes #288 ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [x] Test, docs, adr added or updated as needed - [X] [Contributor Guide Steps](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)(https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md#submitting-a-pull-request) followed --------- Co-authored-by: Micah Nagel <micah.nagel@defenseunicorns.com>
🤖 I have created a release *beep* *boop* --- ## [0.19.0](v0.18.0...v0.19.0) (2024-04-12) ### Features * add nightly testing eks ([#250](#250)) ([543b09d](543b09d)) ### Bug Fixes * drop path normalization to MERGE_SLASHES to allow apps to handle encoded slashes ([#330](#330)) ([26e965f](26e965f)) * loki bucket configuration service_account and namespace ([#332](#332)) ([9518634](9518634)) ### Miscellaneous * **deps:** update grafana ([#257](#257)) ([c98e566](c98e566)) * **deps:** update metrics-server ([#298](#298)) ([691fd87](691fd87)) * **deps:** update pepr ([#324](#324)) ([2ef0f96](2ef0f96)) * **deps:** update pepr to v0.28.7 ([#321](#321)) ([e7206bb](e7206bb)) * **deps:** update promtail ([#74](#74)) ([6a112b5](6a112b5)) * **deps:** update zarf to v0.32.6 ([#282](#282)) ([443426d](443426d)) * **deps:** update zarf to v0.33.0 ([#325](#325)) ([f2a2a66](f2a2a66)) * update codeowners ([#338](#338)) ([c419574](c419574)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Description
PR to address
DECODE_AND_MERGE_SLASHES
causing issues in certain applications. Gives the ability to selectively turn this off in a namespace with theUDSPackage
CR.Related Issue
Fixes #288
Type of change
Checklist before merging