Coraza support

[mac-chaffee]

This PR adds a new config value, modsecurity-use-coraza, which changes the WAF/SPOE configuration to support https://github.com/corazawaf/coraza-spoa

Specifically, this config matches the "experimental" branch of coraza-spoa. Slight changes in the args may be required in the future as the API stabilizes, so we do not provide default args in the source code (only in the docs).

To use, just set these values in the helm chart:

controller:
  image:
    repository: <must use an image based on this branch>
    tag: <must use an image based on this branch>
  config:
    modsecurity-use-coraza: "true"
    modsecurity-endpoints: 127.0.0.1:12345
    modsecurity-args: "app=hdr(host) id=unique-id src-ip=src src-port=src_port dst-ip=dst dst-port=dst_port method=method path=path query=query version=req.ver headers=req.hdrs body=req.body"
    waf: modsecurity
  extraVolumes:
    - name: coraza-config
      configMap:
        name: coraza-config
  extraContainers:
    - name: coraza-spoa
      image: <you must supply your own image based on the "experimental" branch with this PR applied https://github.com/corazawaf/coraza-spoa/pull/36 >
      ports:
      - containerPort: 12345
        name: spop
        protocol: TCP
      resources:
        limits:
          memory: 500Mi
        requests:
          cpu: 1000m
          memory: 250Mi
      volumeMounts:
      - name: coraza-config
        mountPath: /config.yaml
        subPath: config.yaml
        readOnly: true
      livenessProbe:
        tcpSocket:
          port: 12345
        failureThreshold: 3
        initialDelaySeconds: 30
        periodSeconds: 5
        successThreshold: 1
        timeoutSeconds: 4

The above values will mount a ConfigMap for Coraza's config.yaml, which must be created separately. Here is that configmap:

apiVersion: v1
kind: ConfigMap
metadata:
  name: coraza-config
  namespace: haproxy-ingress
data:
  config.yaml: |
    bind: 127.0.0.1:12345
    default_application: default_app
    applications:
      default_app:
        include:
          - /etc/coraza-spoa/coraza.conf
          - /etc/coraza-spoa/crs-setup.conf
          - /etc/coraza-spoa/rules/*.conf

        transaction_ttl: 60000
        transaction_active_limit: 100000
        log_level: debug
        log_file: /dev/stdout

Note the default_application line, that is from this PR: corazawaf/coraza-spoa#36
That PR is needed because I set the app arg to be the Host header, so you need a default_application to configure WAF rules for all hosts. Otherwise, you'd need a different config block for every single Ingress hostname in your cluster.

[jcmoraisjr]

Great job. I still need to have a closer look and want to run it myself on a local env, but for now I have two suggestions:

• please rebase on top of master since I saw you built this pr on top of your previous one, which is already merged;
• do not change where defaults is stored along with all the other changes, but instead let's postpone this move to another pr (btw why did you change?)

[sealneaward]

@jcmoraisjr I can help with some testing too.

[mac-chaffee]

Ah sorry forgot to mention the defaults.go thing. If I move the file back to where it was, I get an import cycle error:

$ go test -tags cgo ./...
# github.com/jcmoraisjr/haproxy-ingress/pkg/haproxy
package github.com/jcmoraisjr/haproxy-ingress/pkg/haproxy
	imports github.com/jcmoraisjr/haproxy-ingress/pkg/converters/ingress: import cycle not allowed in test

The cycle is "instance_test.go" imports "pkg/converters/ingress" and "pkg/converters/ingress/ingress.go" imports "pkg/haproxy", which is where instance_test.go is.

Seems either defaults.go needs to move or instance_test.go needs to move so neither gets accidentally imported when the parent folder is imported. Open to other ideas though!

[sealneaward]

Ok, so I set this up on my cluster, and verified the configuration suggested works. I am able to intercept attacks and read them in the coraza sidecar container.

Loading 1 applications
time="2022-11-21T19:21:07Z" level=info msg="spoe: listening on [::]:9000"
{"level":"error","ts":1669058760.188159,"msg":"[client \"10.100.0.172\"] Coraza: Warning. OS File Access Attempt [file \"/etc/coraza-spoa/rules/REQUEST-930-APPLICATION-ATTACK-LFI.conf\"] [line \"0\"] [id \"930120\"] [rev \"\"] [msg \"OS File Access Attempt\"] [data \"Matched Data: etc/passwd found within ARGS:p: /etc/passwd\"] [severity \"critical\"] [ver \"OWASP_CRS/4.0.0-rc1\"] [maturity \"0\"] [accuracy \"0\"] [tag \"application-multi\"] [tag \"language-multi\"] [tag \"platform-multi\"] [tag \"attack-lfi\"] [tag \"paranoia-level/1\"] [tag \"OWASP_CRS\"] [tag \"capec/1000/255/153/126\"] [tag \"PCI/6.5.4\"] [hostname \"\"] [uri \"/?p=/etc/passwd\"] [unique_id \"260e3d15-dd04-41b7-a634-45d17f0a8390\"]\n"}

One thing I noticed is that it is not passing the hostname to the logs, but I don't know if that needs to be addressed in the SPOA code vs the ingress controller code.
Otherwise, looks good. This is very exciting.

[mac-chaffee]

@sealneaward ah good catch, I will investigate that

[jcmoraisjr]

Thanks again for the contribution! See some comments below. Mostly good, just bothering a bit about where to place the use-coraza config key.

I'm also proposing not to move the defaults.go file, which would create another large commit, so try to squash everything into a single commit - or a few more commits with incremental steps if you prefer.

[mac-chaffee]

@sealneaward The missing hostname field in the logs is indeed a Coraza bug, filed this issue about it: corazawaf/coraza#515

[mac-chaffee]

@jcmoraisjr sorry for the force-push spam. I removed the custom args like we discussed (user must supply them now) and expanded the modsecurity.md example docs to include a section on Coraza.

[jcmoraisjr]

Awesome thanks! A few other comments below.

sorry for the force-push spam

No problem at all, keep pushing =)

[jcmoraisjr]

A few other findings after run an e2e locally. Other than that LGTM and we're ready to go.

[jcmoraisjr]

Lgtm, thanks! Merging now.