-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Make echoheaders nginx container escape &<> to prevent XSS. #2629
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED The following people have approved this PR: rmmh Needs approval from an approver in each of these OWNERS Files: We suggest the following people: |
Please add Since this is just a toy container, I'm fine with those going in as follow ups whenever you have time. LGTM. |
@aledbf might also have some sage suggestions for the follow up |
Those headers won't help, this is specifically a problem with an overly-sensitive XSS scanner with rules about old IE content sniffing, which doesn't support CSP. The page is served as @bprashanth can you do /approve too? |
Replaced with escape function from a Lua templating engine. It was simpler to inline than to figure out the packaging system for nginx. |
/lgtm cancel //PR changed after LGTM, removing LGTM. @aledbf @bprashanth @mml @rmmh |
Ah, no i mean apply the headers for the case they would help (i.e typical xss). |
@rmmh @bprashanth we moved the image to https://github.com/kubernetes/ingress/tree/master/images/echoheaders |
this is the new output kubernetes/ingress-nginx#785 |
@rmmh can you open the PR in the new repository? |
Ported to kubernetes/ingress-nginx#865 |
Tested locally by doing
make container; docker run --rm -p 8080:8080 gcr.io/google_containers/echoserver:1.4
and thencurl 'localhost:8080/xss-<&>'
.Intentionally not bumping the image tag because that would require multiple patches against old branches to pick up the newer version, and this change is relatively small and safe!