[release-1.4] authz: fix the validation for request.headers #21284

yangminzhu · 2020-02-19T23:15:08Z

This fixes the validation for the "request.headers" to correctly generate the filter config. Note the bug has already been fixed in master and release-1.5 in a separate PR (https://github.com/istio/istio/pull/20442/files#diff-4fa38d71f05c2090c15c94f43f3e248cR77) before.

yangminzhu · 2020-02-19T23:33:51Z

/retest

yangminzhu · 2020-02-19T23:59:47Z

It seems the lint is failing for unrelated issues?

> Checking links in ./README.md
> Will allow duplicate links
> Will allow redirects
> Will allow SSL errors
> Will allow network timeouts
> White list links matching: localhost:8080, storage.googleapis.com/istio-artifacts/pilot/, http://ratings.default.svc.cluster.local:9080/ratings 
> Will not save results
Links to check: 20, 0 white listed
  01. https://goreportcard.com/badge/github.com/istio/istio 
  02. https://goreportcard.com/report/github.com/istio/istio 
  03. https://godoc.org/istio.io/istio?status.svg 
  04. https://codecov.io/github/istio/istio/coverage.svg?branch=release-1.4 
  05. https://golangci.com/badges/github.com/istio/istio.svg 
  06. https://golangci.com/r/github.com/istio/istio 
  07. https://istio.io 
  08. https://discuss.istio.io 
  09. https://istio.io/about/community 
  10. https://github.com/istio/community 
  11. https://github.com/istio/istio/wiki/Preparing-for-Development 
  12. https://github.com/istio/istio/wiki/Development-Conventions 
  13. https://github.com/istio/istio/wiki/Writing-Fast-and-Lean-Code 
  14. https://github.com/istio/istio/wiki 
  15. https://istio.io/docs/concepts/traffic-management/#pilot 
  16. https://istio.io/docs/reference/config/networking/ 
  17. https://istio.io/docs/reference/commands/istioctl.html 
  18. https://github.com/istio/api 
  19. https://github.com/istio/proxy 
  20. https://github.com/envoyproxy/envoy 
Checking URLs: ✓✓✓✓→✓✓→✓✓✓✓✓✓→✓✓✓✓?
Issues :-(
> Links 
  1. [L004]  https://golangci.com/badges/github.com/istio/istio.svg Failed to open TCP connection to golangci.com:443 (Cannot assign requested address - connect(2) for "golangci.com" port 443)

@istio/release-managers-1-4 Is this an existing issue in 1.4 or do you have any ideas of this? Thank you.

yangminzhu · 2020-02-20T00:57:57Z

/retest

yangminzhu · 2020-02-20T02:13:01Z

/retest

liminw

/lgtm

yangminzhu · 2020-02-20T20:00:51Z

@istio/release-managers-1-4 Could someone take a look at this? Thank you.

howardjohn

Please link to the original master PRs/issue

howardjohn · 2020-02-20T20:43:02Z

tests/integration/security/rbac_v1beta1_test.go

@@ -285,3 +285,51 @@ func TestV1beta1_WorkloadSelector(t *testing.T) {
 			rbacUtil.RunRBACTest(t, cases)
 		})
 }
+
+// TestV1beta1_RequestHeaders tests v1beta1 authorization with "request.headers".
+func TestV1beta1_RequestHeaders(t *testing.T) {


Why do we have a special case for this? This seems like just a normal case, should this be added as a case in an existing test rather than a whole new test?

I didn't find a good existing test case to add this one, I will try do refactor this a little bit with more tests in master.

* authz: fix the validation for request.headers (istio#21284) * manual backport of 21513 (istio#21514) Signed-off-by: Rama Chavali <rama.rao@salesforce.com> * Fix PSP sample file to allow NET_RAW. (istio#21533) * [release-1.4] remove use_downstream_protocol for gateway (istio#21727) * remove use_downstream_protocol for gateway * comment * writing Co-authored-by: Yan Xue <3491507+yxue@users.noreply.github.com> * Update dependencies (istio#21765) * Make iptables script and output consistent with golang (istio#21871) Co-authored-by: knrc <knrc@users.noreply.github.com> * [release-1.4] resign certificate (istio#22131) * resign certificate * test Co-authored-by: xuzhonghu <xuzhonghu@huawei.com> * Stop linting links of 1.4 branch (istio#22132) This is going to continue to break as istio.io changes. We already disabled this on 1.5/master. * [release-1.4] Build: Honor GOBUILDFLAGS variable also in test mode (istio#22171) We already honor it in `go build`, we should do the same in `go test`. Manual backport of istio#22163 * Update base image for release 1.4 (istio#22165) * Fix GOBUILDFLAGS usage in codecov scripts (istio#22182) This variable needs to be set before used (set -u) and must not be quoted, otherwise it will become an empty arg for go test if it's empty. Co-authored-by: Jonh Wendell <jonh.wendell@redhat.com> * Update proxy to pickup fixes for ISTIO-SECURITY-2020-003. (istio#22257) * Fix extra .Value in deployment file for stackdriver tracing vars (istio#22506) Co-authored-by: gargnupur <gargnupur@google.com> * Update operator SHA (istio#22523) * clone LbEndpoint to prevent data race (istio#22023) (istio#22528) (cherry picked from commit fdc6dd4) Co-authored-by: Zhonghu Xu <xuzhonghu@huawei.com> * cni: update SHA (istio#22569) Signed-off-by: Yuchen Dai <silentdai@gmail.com> * Make sure to use CNI 1.4 image when running tests under CNI (istio#23035) * Make sure to use CNI 1.4 image when running tests under CNI * Fix lint * [release-1.4] Update dependencies with update_deps.sh (istio#23010) * Update deps with update_deps.sh * Dep updates again * [release-1.4] Run update_deps.sh (istio#23051) * Run update_deps.sh Had to do go get istio.io/operator@5f8ecc70a0f4059bfd4d0f7867d4dc76407f2f08 since update_deps.sh was not updating it. * Run go mod tidy * Update with another cni change (istio#23061) * [release-1.4] Fix Gateway helm chart for helm 3 (istio#23077) * Fix Gateway helm chart for helm 3 Helm 3 threw an error that `$spec.sds` is not a function. Removing the parentheses led to the next problem: the and evaluates eager so `spec.sds.enabled` resulted in a `nil` exception. By nesting the if, the problem is resolved * Also fix the role Co-authored-by: Alex Nederlof <alex@nederlof.com> Co-authored-by: Yangmin Zhu <ymzhu@google.com> Co-authored-by: Rama Chavali <rama.rao@salesforce.com> Co-authored-by: Oliver Liu <yonggangl@google.com> Co-authored-by: Istio Automation <istio.testing@gmail.com> Co-authored-by: Yan Xue <3491507+yxue@users.noreply.github.com> Co-authored-by: Jonh Wendell <jonh.wendell@redhat.com> Co-authored-by: knrc <knrc@users.noreply.github.com> Co-authored-by: xuzhonghu <xuzhonghu@huawei.com> Co-authored-by: John Howard <howardjohn@google.com> Co-authored-by: Eric Van Norman <ericvn@us.ibm.com> Co-authored-by: Joshua Blatt <jblatt@google.com> Co-authored-by: gargnupur <gargnupur@google.com> Co-authored-by: Yuchen Dai <silentdai@gmail.com> Co-authored-by: jacob-delgado <38300436+jacob-delgado@users.noreply.github.com> Co-authored-by: Alex Nederlof <alex@nederlof.com>

* authz: fix the validation for request.headers (istio#21284) * manual backport of 21513 (istio#21514) Signed-off-by: Rama Chavali <rama.rao@salesforce.com> * Fix PSP sample file to allow NET_RAW. (istio#21533) * [release-1.4] remove use_downstream_protocol for gateway (istio#21727) * remove use_downstream_protocol for gateway * comment * writing Co-authored-by: Yan Xue <3491507+yxue@users.noreply.github.com> * Update dependencies (istio#21765) * Make iptables script and output consistent with golang (istio#21871) Co-authored-by: knrc <knrc@users.noreply.github.com> * [release-1.4] resign certificate (istio#22131) * resign certificate * test Co-authored-by: xuzhonghu <xuzhonghu@huawei.com> * Stop linting links of 1.4 branch (istio#22132) This is going to continue to break as istio.io changes. We already disabled this on 1.5/master. * [release-1.4] Build: Honor GOBUILDFLAGS variable also in test mode (istio#22171) We already honor it in `go build`, we should do the same in `go test`. Manual backport of istio#22163 * Update base image for release 1.4 (istio#22165) * Fix GOBUILDFLAGS usage in codecov scripts (istio#22182) This variable needs to be set before used (set -u) and must not be quoted, otherwise it will become an empty arg for go test if it's empty. Co-authored-by: Jonh Wendell <jonh.wendell@redhat.com> * Update proxy to pickup fixes for ISTIO-SECURITY-2020-003. (istio#22257) * Fix extra .Value in deployment file for stackdriver tracing vars (istio#22506) Co-authored-by: gargnupur <gargnupur@google.com> * Update operator SHA (istio#22523) * clone LbEndpoint to prevent data race (istio#22023) (istio#22528) (cherry picked from commit fdc6dd4) Co-authored-by: Zhonghu Xu <xuzhonghu@huawei.com> * cni: update SHA (istio#22569) Signed-off-by: Yuchen Dai <silentdai@gmail.com> * Make sure to use CNI 1.4 image when running tests under CNI (istio#23035) * Make sure to use CNI 1.4 image when running tests under CNI * Fix lint * [release-1.4] Update dependencies with update_deps.sh (istio#23010) * Update deps with update_deps.sh * Dep updates again * [release-1.4] Run update_deps.sh (istio#23051) * Run update_deps.sh Had to do go get istio.io/operator@5f8ecc70a0f4059bfd4d0f7867d4dc76407f2f08 since update_deps.sh was not updating it. * Run go mod tidy * Update with another cni change (istio#23061) * [release-1.4] Fix Gateway helm chart for helm 3 (istio#23077) * Fix Gateway helm chart for helm 3 Helm 3 threw an error that `$spec.sds` is not a function. Removing the parentheses led to the next problem: the and evaluates eager so `spec.sds.enabled` resulted in a `nil` exception. By nesting the if, the problem is resolved * Also fix the role Co-authored-by: Alex Nederlof <alex@nederlof.com> * Citadel completely ignores namespaces opted out. (istio#23223) * Add Kiali fix to Helm (istio#23445) * Update operator for istio/operator#777 (istio#23540) * bump base (istio#23655) * Update cni sha for release-1.4 branch (istio#24216) * [release-1.4] Update jquery and nodejs (istio#24407) * Add files from cherrypick * Update yaml files to pull 1.15.1 images Co-authored-by: Brian Avery <bavery@redhat.com> * Update proxy sha (istio#24721) * Bump bookinfo images * Update vendor Co-authored-by: Yangmin Zhu <ymzhu@google.com> Co-authored-by: Rama Chavali <rama.rao@salesforce.com> Co-authored-by: Oliver Liu <yonggangl@google.com> Co-authored-by: Istio Automation <istio.testing@gmail.com> Co-authored-by: Yan Xue <3491507+yxue@users.noreply.github.com> Co-authored-by: knrc <knrc@users.noreply.github.com> Co-authored-by: xuzhonghu <xuzhonghu@huawei.com> Co-authored-by: John Howard <howardjohn@google.com> Co-authored-by: Eric Van Norman <ericvn@us.ibm.com> Co-authored-by: Joshua Blatt <jblatt@google.com> Co-authored-by: gargnupur <gargnupur@google.com> Co-authored-by: Yuchen Dai <silentdai@gmail.com> Co-authored-by: jacob-delgado <38300436+jacob-delgado@users.noreply.github.com> Co-authored-by: Alex Nederlof <alex@nederlof.com> Co-authored-by: Brian Avery <bavery@redhat.com> Co-authored-by: Martin Ostrowski <mostrowski@google.com> Co-authored-by: stewartbutler <stewartbutler@google.com> Co-authored-by: Istio Automation <istio-testing-bot@google.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

yangminzhu requested a review from a team February 19, 2020 23:15

istio-policy-bot added the area/security label Feb 19, 2020

googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 19, 2020

yangminzhu assigned liminw Feb 19, 2020

istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 19, 2020

authz: fix the validation for request.headers

85b738d

yangminzhu force-pushed the request-headers branch from 781ba84 to 85b738d Compare February 20, 2020 01:22

liminw approved these changes Feb 20, 2020

View reviewed changes

howardjohn reviewed Feb 20, 2020

View reviewed changes

howardjohn approved these changes Feb 20, 2020

View reviewed changes

istio-testing merged commit 2960fd7 into istio:release-1.4 Feb 20, 2020

yangminzhu deleted the request-headers branch February 20, 2020 22:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[release-1.4] authz: fix the validation for request.headers #21284

[release-1.4] authz: fix the validation for request.headers #21284

Uh oh!

yangminzhu commented Feb 19, 2020 •

edited

Loading

Uh oh!

yangminzhu commented Feb 19, 2020

Uh oh!

yangminzhu commented Feb 19, 2020

Uh oh!

yangminzhu commented Feb 20, 2020

Uh oh!

yangminzhu commented Feb 20, 2020

Uh oh!

liminw left a comment

Uh oh!

yangminzhu commented Feb 20, 2020

Uh oh!

howardjohn left a comment

Uh oh!

howardjohn Feb 20, 2020

Uh oh!

yangminzhu Feb 20, 2020

Uh oh!

Uh oh!

[release-1.4] authz: fix the validation for request.headers #21284

[release-1.4] authz: fix the validation for request.headers #21284

Uh oh!

Conversation

yangminzhu commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yangminzhu commented Feb 19, 2020

Uh oh!

yangminzhu commented Feb 19, 2020

Uh oh!

yangminzhu commented Feb 20, 2020

Uh oh!

yangminzhu commented Feb 20, 2020

Uh oh!

liminw left a comment

Choose a reason for hiding this comment

Uh oh!

yangminzhu commented Feb 20, 2020

Uh oh!

howardjohn left a comment

Choose a reason for hiding this comment

Uh oh!

howardjohn Feb 20, 2020

Choose a reason for hiding this comment

Uh oh!

yangminzhu Feb 20, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yangminzhu commented Feb 19, 2020 •

edited

Loading