search

kubernetes / ingress-nginx

Ingress NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.47k stars 8.25k forks source link

PATCH method dies with `enable-modsecurity: "true"` default configuration #5723

Closed PaulCharlton closed 2 years ago

PaulCharlton commented 4 years ago

Summary Observations: PATCH method with JSON request body sent to ingress receives no response bytes on connection and connection is left open.

Connection is closed only when NGINX ingress is bounced due to server reload (on change of ConfigMap, pod deletion ... etc)

when Debug logging is enabled with SecDebugLog /dev/stdout SecDebugLogLevel 4

the debug log shows that phase 1 of the Modsecurity (a) recognized the "application/json", and that (b) phase 2 assigned the JSON attributes into ARGS for subsequent scanning.

** Changing from "DetectOnly" to rule enforcement does not change the behavior. Adding the CRS ruleset does not change the behavior.

** PATCH method works correctly when enable-modsecurity: "false"

==================================== Version info:

kubectl version Server Version: version.Info{Major:"1", Minor:"16", GitVersion:"v1.16.9", GitCommit:"a17149e1a189050796ced469dbd78d380f2ed5ef", GitTreeState:"clean", BuildDate:"2020-04-16T23:15:50Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}

PaulCharlton commented 4 years ago

Additional info: The PATCH request body is a single JSON object, not terminated with a newline. The size in bytes of that JSON object matches exactly the Content-Length header. and Content-Type: application/json;charset=utf-8

PaulCharlton commented 4 years ago

Additional info: adding EOL to the payload (and increasing the content length accordingly) is misbehaving in the same way

PaulCharlton commented 4 years ago

additional triage notes in : https://github.com/SpiderLabs/ModSecurity/issues/2341

summary of what I have observed (POST/PUT/PATCH):

1) Nginx ingress with no modsecurity/external_auth. -> works 2) Nginx ingress with either modsecurity or external_auth. -> works 3) Nginx ingress with both modsecurity and external_auth. -> hangs session until broken by client 3a) modsecurity logs warning "invalid request body" (Rule #920170) with external auth location/path upon session break by client 3b) modsecurity causes failure (session hang) even when ALL modsec rules are removed

I am imagining that something in the control flow nginx->modsecurity->nginx->external_auth->nginx->proxied_server is corrupting session state at the boundary where modsecurity returns to nginx. [or doing a req_read, not realizing that req is already at EOF/(content-length) boundary]

PaulCharlton commented 4 years ago

definitive workaround: add use-http2: false to the Nginx listen block (or Nginx-ingress ConfigMap)

At this point, we can write a unit test that sets conditions: (1) "POST + non-empty body", (2) side-car sub request Auth, (3) modsecurity enabled, (4) force HTTP2 + TLS session. iterate until the unit test works.

fejta-bot commented 4 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 4 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

fejta-bot commented 3 years ago

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /close

k8s-ci-robot commented 3 years ago

@fejta-bot: Closing this issue.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/5723#issuecomment-732734816): >Rotten issues close after 30d of inactivity. >Reopen the issue with `/reopen`. >Mark the issue as fresh with `/remove-lifecycle rotten`. > >Send feedback to sig-testing, kubernetes/test-infra and/or [fejta](https://github.com/fejta). >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
PaulCharlton commented 3 years ago

/reopen /remove-lifecycle rotten

k8s-ci-robot commented 3 years ago

@PaulCharlton: Reopened this issue.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/5723#issuecomment-733066440): >/reopen >/remove-lifecycle rotten > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

PaulCharlton commented 3 years ago

/remove-lifecycle stale

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

sujith-subbiah-ey commented 3 years ago

/remove-lifecycle stale

strongjz commented 3 years ago

/priority backlog

iamNoah1 commented 3 years ago

Hi @PaulCharlton @sujith-subbiah-ey is that still an issue for you guys? Which ingress-nginx version did you use and did you try using newer versions?

k8s-triage-robot commented 3 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

PaulCharlton commented 3 years ago

/remove-lifecycle stale

I will try with latest versions. There is some discussion on the modsecurity board about http2 related patch which may help

iamNoah1 commented 2 years ago

/triage accepted

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot commented 2 years ago

@k8s-triage-robot: Closing this issue.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/5723#issuecomment-1126705051): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues and PRs according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue or PR with `/reopen` >- Mark this issue or PR as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.