feature request: make server log an option

[liudongmiao]

Currenty, ModSecurity-nginx call msc_set_log_cb anyway.

I'd like to make serverLog an option in connector. For myself, I'd like to turn it off.

[martinhsv]

Are you proposing something akin to SecAuditEngine, where you could do something like this?:

SecServerLog Off

Or do you mean a SecRule action? If so, we already have log/nolog? (Granted, there does appear to be at least one use case that does not seem to work quite right in v3, as described in #2698 .)

[liudongmiao]

@martinhsv We use nolog in modsecurity rules. However, there are still callback.
I will figure out why.

[liudongmiao]

@martinhsv Yes, we need such option.

The code in ngx_http_modsecurity_process_intervention like this.
Which always means that there would be log.

    if (msc_intervention(transaction, &intervention) == 0) {
        dd("nothing to do");
        return 0;
    }

    log = intervention.log;
    if (intervention.log == NULL) {
        log = "(no log message was specified)";
    }

    ngx_log_error(NGX_LOG_ERR, (ngx_log_t *)r->connection->log, 0, "%s", log);

[liudongmiao]

And in ModSecurity, there are always log for deny, drop, redirect.

[martinhsv]

My point was that we already have functionality that is supposed to do what you are suggesting (i.e. 'nolog') -- unless I've misunderstood what you meant.

As I said, I agree that that there is a bug in the 'nolog' functionality as is described in owasp-modsecurity/ModSecurity#2698.

However, for the purpose of this ModSecurity-nginx issue that you have created, assuming that 2698 gets resolved, is this ticket supposed to represent some functionality beyond that?

If this ticket just describes the functionality that is supposed to be provided by 'nolog' then this should be closed as a duplicate of 2698.

[FedericoHeichou]

My point was that we already have functionality that is supposed to do what you are suggesting (i.e. 'nolog') -- unless I've misunderstood what you meant.

As I said, I agree that that there is a bug in the 'nolog' functionality as is described in SpiderLabs/ModSecurity#2698.

However, for the purpose of this ModSecurity-nginx issue that you have created, assuming that 2698 gets resolved, is this ticket supposed to represent some functionality beyond that?

If this ticket just describes the functionality that is supposed to be provided by 'nolog' then this should be closed as a duplicate of 2698.

Hi, it's not a ModSecurity's bug, in ngx_http_modsecurity_module.c's ngx_http_modsecurity_process_intervention there is:

    log = intervention.log;
    if (intervention.log == NULL) {
        log = "(no log message was specified)";
    }

    ngx_log_error(NGX_LOG_ERR, (ngx_log_t *)r->connection->log, 0, "%s", log);

With disruptive actions the intervention.log is filled but the callback is not performed if nolog is set. I think a serverLog option is needed to prevent this ngx_log_error. What if we could set the log level from NGX_LOG_ERR to a customizable one? In this way I could set it to NGX_LOG_DEBUG to prevent it. The better solution would be read it from the transaction's rule severity but I don't know if there is a way without parsing the log

[martinhsv]

Closing for reasons listed previously.

[FedericoHeichou]

Wait @martinhsv, it is not a ModSecurity's bug, but a ModSecurity-nginx's one. Did you read my comment?
Why it logs without the callback? I think it is because you want do it immediately instead waiting phase 5, but it should not be supposed to do it when nolog is set. nolog only prevents the execution of callback, not the population of the transaction's logs string.

If you want to keep that logging you should add an option di disable it or, better, an option to change the logging level.

[martinhsv]

@FedericoHeichou ,

I have not made any detailed assessment of your comments. I am a bit skeptical though; my suspicion is that the eventual fix will still be in ModSecurity itself rather than the connector.

The reason for closing this item is because it doesn't fundamentally represent a different issue to the already-open ModSecurity issue referenced above. And it is undesirable to have multiple github issues open that are essentially duplicates.

If I'm wrong, and it does eventually turn out that the fix for #2698 is a change in ModSecurity-nginx, rather than ModSecurity that's not a grave matter -- it wouldn't be the first time that a fix for an issue in one project is actually an implementation in the other.

[FedericoHeichou]

@martinhsv the only way to fix it in ModSecurity is not populating the intervention.log attribute if nolog is set, but I think that field is used by auditlog too, so it is not possible (it will still flood (no log message was specified)) with the current code.
ModSecurity populates the intervention.log, then calls the callback if nolog is not set. But in ModSecurity-nginx there is call to a nginx's logging function out of the actual callback handled by ModSecurity that logs things if intervention.log is populated.

The snipped of code I pasted before is not inside the callback function, but in ngx_http_modsecurity_process_intervention. Maybe in different connectors can be useful the intervention.log even tough the nolog is set.

I don't know, your choice, personally I only changed the logging level in that line, which logs without the callback