ModSec Audit logs in JSON format - not JSON?

[paul-hammant]

Describe the bug

I'm running ModSec as a library for testing purposes (with GoTestWAF, but that's not important I don't think). It is printing JSON audit files (one per request) to the file system. Field time_stamp looks corrupted though as I gaze and the files with JSON that won't parse cos "time_stamp": "\u0014\u00108....\u0006" is choking subsequent parsers (like the one in browsers).

Source code question

In src/transaction.cc, should c_str() be called twice?

    std::string ts = utils::string::ascTime(&m_timeStamp).c_str();

    .. snip .. 

    LOGFY_ADD("time_stamp", ts.c_str());

[airween]

Hi @paul-hammant,

could you show your relevant configuration? I've never faced any issue like this - and I think the regression test framework covers all scenario (haven't checked this specific one), so I would wonder if this is a new issue (if it is). But with a config snippet I try to check that.

[paul-hammant]

I'm using ModSec.so from Java in a corporate setting. GoTestWAF is scoring it quite highly, but in analysing the audit logs I see this unparsable JSON. I don't have a need to have machine-processable JSON logs, so it'll never become an issue for me. I would've assumed that it was encoded as UTF-8 as almost everything is these days. It's not though. If I need to in the short term I can swap the file's epoch time for the sequence by bytes in between " and ".

Another day I'll try to make the local change and see if it works. I'll report back then.

[paul-hammant]

After some trial and error, this looks like it: paul-hammant@7010ddd

Still to report back from an actual deployment.

[airween]

Hi @paul-hammant,

thanks - well, I don't see any relevant difference so if this solves your issue that would be a surprise 😄

[airween]

Hi @paul-hammant,

I've already inspected this issue, and need some clarification.

In your initial comment you wrote:

Field time_stamp looks corrupted though as I gaze and the files with JSON that won't parse cos "time_stamp": "\u0014\u00108....\u0006" is choking subsequent parsers (like the one in browsers).

Was this just a random example, or a real occurrence? I mean it's a bit weird that something formats the entity with more leading zeroes, I mean \u00108 - that could be \u0108, or couldn't? Or is that a sequence, and the 8 is the next character, and the entity is just \u0010?

In src/transaction.cc, should c_str() be called twice?

   std::string ts = utils::string::ascTime(&m_timeStamp).c_str();

No, I think this c_str() is unnecessary. But I'm not sure that the cause is this. If the time_stamp value is looks like a trash, then probably one of these function calls are failed:
localtime()
strftime()

You mentioned your fix:

After some trial and error, this looks like it: paul-hammant@ 7010ddd

With this modification, what do you get in the field time_stamp?

[paul-hammant]

I left the employer that was seeing the issue in their scaled in-house deployment. As it happens they fixed it for themselves. My suggestions toward code changes were rubbish of course. They'll pass the fix to yourselves or drop their patch and use your canonical fix.

[airween]

Ah, thanks for letting me know this.

Then can we close this issue?

[airween]

Closing as there is no further interest. We can reopen if anyone faces this issue again.