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

Modesecurity works in "DetectionOnly" mode even if I specify SecRuleEngine to 'On' in my configMap #6307

Closed PuneeshMotwani closed 2 years ago

PuneeshMotwani commented 4 years ago

NGINX Ingress controller version: v0.35.0

Kubernetes version (use kubectl version): 1.16.9

Environment:

What happened: I can see the modsecurity logging my requests but it does not block the requests. Below is my configMap for nginx-ingress-controller:

 apiVersion: v1

data:
   enable-modsecurity: "true"
   enable-owasp-modsecurity-crs: "false"
   modsecurity-snippet: |-
      SecRuleEngine On
      SecAuditEngine RelevantOnly
      SecRuleRemoveById 911100 949110 920350
      SecAuditLogParts ABIJDEFHZ
      SecAuditLog /var/tmp/modsec_audit.log
      SecAuditLogStorageDir /var/tmp/
      Include /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf
kind: ConfigMap

I can see that the modsecurity-snippet is correctly updated in the file /etc/nginx/nginx.conf

 modsecurity on;

        modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;

        modsecurity_rules '
        SecRuleEngine On
        SecAuditEngine RelevantOnly
        SecRuleRemoveById 911100 949110 920350
        SecAuditLogParts ABIJDEFHZ
        SecAuditLog /var/tmp/modsec_audit.log
        SecAuditLogStorageDir /var/tmp/
        Include /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf
        ';

But when I try to perform a test for crs ex. sql injection on my url -> https://MYURL?username=1%27%20or%20%271%27%20=%20%27

It logs in the /var/tmp/modsec_audit.log but it does not blocks.

When I look into this file /etc/nginx/modsecurity/modsecurity.conf inside the controller container, I see that SecRuleEngine is set to "DetectionOnly"

I also tried adding an annotation below on my ingress, but this does not help.:-

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/modsecurity-snippet: |
      SecRuleEngine On

What you expected to happen: Since the directive SecRuleEngine is set to 'On', this should block the request and not just log the requests.

How to reproduce it:

Anything else we need to know:

/kind bug

TSASM commented 4 years ago

I am also having this issue.

My set up is as follows:

Cloud provider: GCP k8s version: 1.15.12-gke.2 Nginx controller details:

  Release:       0.28.0
  Build:         git-1f93cb8f3
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.17.7

In order to reproduce this you just need to spin up the controller like normal, then follow this page https://kubernetes.github.io/ingress-nginx/user-guide/third-party-addons/modsecurity/ which says to add two lines to the controller's configuration config map:

enable-modsecurity: "true"
enable-owasp-modsecurity-crs: "true"

Then cat the modsecurity.conf file to see the rule engine is stuck on detection mode

zerkms commented 4 years ago

The same here: nginx-ingress-controller:0.32.0 ingress-nginx: ingress-nginx-2.3.0

With the following configmap:

  enable-modsecurity: "True"
  modsecurity-snippet: |-
    SecRuleEngine On
    SecRule REQUEST_HEADERS:User-Agent \"fern-scanner\" \"log,deny,id:107,status:403,msg:\'Fern Scanner Identified\'\"

it still runs in a detection only mode

UPD:

I updated to controller:v0.40.2 and ingress-nginx-3.7.1: still - detection only.

zerkms commented 4 years ago

It looks like it's been broken for quite a while: https://github.com/kubernetes/ingress-nginx/issues/4902#issuecomment-631450480

zerkms commented 4 years ago

After a small research I have found that the location generation was fixed at https://github.com/kubernetes/ingress-nginx/pull/5315

But nginx.tmpl still has the wrong order: https://github.com/kubernetes/ingress-nginx/blob/fb6a03ffb42cfbf682c3c7f400f46fb4802caa1c/rootfs/etc/nginx/template/nginx.tmpl#L151

danfromtitan commented 3 years ago

I came across similar issues when trying to enable/disable audit logs from SecAuditEngine. The root cause sits with modsecurity config ordering as noted in SpiderLabs/ModSecurity-nginx#183

It seems the first encounter of a config value takes precedence over the ones following after. Now, to make the matters really confusing, the order of source configs is altered in the process of nginx.conf creation (by the nginx config parser?):

modsecurity on; modsecurity_rules ' [...] '; modsecurity_transaction_id "$request_id"; modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;`

I guess that will produce the outcome desired (i.e. overriding configs from modsecurity_rules snippet), but I'd feel much better maintaining modsecurity_rules_file from a ConfigMap that I have under full configuration control (#6545).

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

zerkms commented 3 years ago

/remove-lifecycle stale

superkartoffel commented 3 years ago

We are facing the same issue. It works fine if you specify the modsecurity configuration in the ingress configuration as annotations, but it doesn't if you use a configmap for global configuration because the resulting configuration is not in the correct order. as described in https://github.com/kubernetes/ingress-nginx/issues/6307#issuecomment-714211944

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

zerkms commented 3 years ago

/remove-lifecycle stale

iamNoah1 commented 3 years ago

Hi @PuneeshMotwani @TSASM @zerkms @danfromtitan can you guys confirm that the issue still exists with newer versions of ingress-nginx?

besha100 commented 2 years ago

@iamNoah1 This issue still exist with the latest version helm-chart-4.0.13 I just created this PR which will fix this https://github.com/kubernetes/ingress-nginx/pull/8021

iamNoah1 commented 2 years ago

/reopen /triage accepted /priority important-longterm

k8s-ci-robot commented 2 years ago

@iamNoah1: Reopened this issue.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/6307#issuecomment-989058663): >/reopen >/triage accepted >/priority important-longterm 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.
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/6307#issuecomment-1120270415): >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.