corazawaf / coraza-spoa

EXPERIMENTAL: A wrapper around the OWASP Coraza WAF for HAProxy's SPOE filters
Apache License 2.0
83 stars 18 forks source link

Support json format as loglevel in config.yaml #91

Closed devasmith closed 1 year ago

devasmith commented 1 year ago

Summary

It would be nice if we could log these events as json so that for example Kibana can digest them in that format. Other logs are consumed in this way and it would be good to follow the same standard if possible. I discussed this briefly with @jptosso on the OWASP Slack.

Basic example

Maybe we can add a option so that we can change the log format https://github.com/corazawaf/coraza-spoa/blob/main/config.yaml.default#L29-L32

Example:

# The log level configuration, one of: debug/info/warn/error/panic/fatal
log_level: info
# The log file path
log_file: /dev/stdout
# The log format
log_format: json

I got referred to this block https://github.com/corazawaf/coraza-spoa/blob/6a0d9a3ba67379892eafa105c636d85016f8ef21/internal/spoa.go#L126 as it would be good to implement this feature here.

The current output that needs to be converted can look like this:

[client "10.1.1.1"] Coraza: Warning. Inbound Anomaly Score Exceeded (Total Score: 5) [file "/etc/coraza-spoa/rules/REQUEST-949-BLOCKING-EVALUATION.conf"] [line "9415"] [id "949110"] [rev ""] [msg "Inbound Anomaly Score Exceeded (Total Score: 5)"] [data ""] [severity "emergency"] [ver "OWASP_CRS/4.0.0-rc1"] [maturity "0"] [accuracy "0"] [tag "anomaly-evaluation"] [hostname "10.1.1.2"] [uri "/test"] [unique_id "xyz"]

We could for example add the following to rule_match.go. Did some testing earlier.

func (mr MatchedRule) JSONErrorLog() map[string]interface{} {
    var msg string
    var data string

    for _, md := range mr.MatchedDatas_ {
        if md.Data() != "" {
            data = md.Data()
        }
        // Use 1st set message of rule chain as message
        if md.Message() != "" {
            msg = md.Message()
        }
        if data != "" && msg != "" {
            break
        }
    }
    if len(msg) > maxSizeLogMessage {
        msg = msg[:maxSizeLogMessage]
    }

    m := make(map[string]interface{})
    m["client"] = mr.ClientIPAddress_
    m["message"] = msg
    m["disruptive"] = mr.Disruptive_
    m["file"] = mr.Rule_.File()
    m["line"] = mr.Rule_.Line()
    m["id"] = mr.Rule_.ID()
    m["rev"] = mr.Rule_.Revision()
    m["msg"] = msg
    m["data"] = data
    m["severity"] = mr.Rule_.Severity()
    m["ver"] = mr.Rule_.Version()
    m["maturiy"] = mr.Rule_.Maturity()
    m["accuracy"] = mr.Rule_.Accuracy()

    m["tag"] = mr.Rule_.Tags()
    m["hostname"] = mr.ServerIPAddress_
    m["uri"] = mr.URI_
    m["unique_id"] = mr.TransactionID_
    m["phase"] = mr.Rule_.Phase()

    switch mr.DisruptiveAction_ {
    case DisruptiveActionAllow:
        m["message"] = "Coraza: Access allowed."
    case DisruptiveActionDeny:
        m["message"] = "Coraza: Access denied."
    case DisruptiveActionDrop:
        m["message"] = "Coraza: Access dropped."
    case DisruptiveActionPass:
        m["message"] = "Coraza: Warning."
    case DisruptiveActionRedirect:
        m["message"] = "Coraza: Access redirected."
    default:
        m["message"] = "Coraza: Custom disruptive action triggered."
    }

    return m
}

Test run

=== RUN   TestJSONErrorLogMessages/no_disruptive_action
{"level":"info","ts":1694003837.95571,"caller":"corazarules/rule_match_test.go:235","msg":"test","data":"","maturiy":0,"accuracy":0,"client":"","file":"","id":1234,"rev":"","msg":"","message":"Coraza: Custom disruptive action triggered.","disruptive":false,"severity":"emergency","ver":"","unique_id":"","hostname":"","uri":"","phase":0,"line":0,"tag":[]}

Motivation

Having logs in json is great and it is easy to parse.

jptosso commented 1 year ago

Although I don't see any problem adding a json output to MatchedRules, I do see a problem adding additional configuration to connectors. Logs are already too complicated for users, we have error, audit and debug, now we would add error log configurations IMHO it's better if each user parses the coraza lot as it is, then he can transform it into json

xonvanetta commented 1 year ago

I don't quite understand connectors in this context but I assume it apps that use coraza as a library.

Would it be the right place to add a json formatter here based on a config flag? Don't know if this also is considered a connector or is there a better place to put json logging on the application side.

jcchavezs commented 1 year ago

+1 to custom serializers, -1 to adding log level to the config. As of now you can configure that in the seclang config, supporting log config somewhere else will be problematic and cumbersome as you need to define a priority that users need to be aware of and still that does not clarify anything.

devasmith commented 1 year ago

+1 to custom serializers, -1 to adding log level to the config. As of now you can configure that in the seclang config, supporting log config somewhere else will be problematic and cumbersome as you need to define a priority that users need to be aware of and still that does not clarify anything.

Can you give any example configurating this via seclang? Is it something we would have to define here?

jcchavezs commented 1 year ago

check https://coraza.io/docs/seclang/directives/#secdebuglog

On Thu, Sep 7, 2023 at 9:02 AM Anders Smith @.***> wrote:

+1 to custom serializers, -1 to adding log level to the config. As of now you can configure that in the seclang config, supporting log config somewhere else will be problematic and cumbersome as you need to define a priority that users need to be aware of and still that does not clarify anything.

Can you give any example configurating this via seclang? Is it something we would have to define here https://github.com/corazawaf/coraza/blob/main/coraza.conf-recommended?

— Reply to this email directly, view it on GitHub https://github.com/corazawaf/coraza-spoa/issues/91#issuecomment-1709586929, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAXIGHFMQLKRNLLSFNTXZFWPPANCNFSM6AAAAAA4NXHEYQ . You are receiving this because you commented.Message ID: @.***>

devasmith commented 1 year ago

check https://coraza.io/docs/seclang/directives/#secdebuglog

I don't really get it. How can we use this? Is this a replacement for the logging at https://github.com/corazawaf/coraza-spoa/blob/main/config.yaml.default#L29-L32?

zc-devs commented 1 year ago

I don't quite understand connectors in this context but I assume it apps that use coraza as a library.

Yup, you're right.

Is this a replacement for the logging at https://github.com/corazawaf/coraza-spoa/blob/main/config.yaml.default#L29-L32?

Of course, no :)


Should be something like that on coraza's side for Debug and Error logs, I think.

@devasmith, while your hack works for Coraza's error log, I would like to attract attention to Connector's logs as well as other libraries.

devasmith commented 1 year ago

I just noticed this change: https://github.com/corazawaf/coraza-spoa/pull/90/files#diff-d0effb22dac9e11bf39bf7ede9c5a9802da85a46a39fe39fe2c81fec1a3ddda6R22-R23

This is what I was after and it seems to be implemented in that draft.

zc-devs commented 1 year ago

Oh, OK. I guess I won't say anything about #90. Wish you luck 🤝