New colorized logging causes fail2ban to stop matching

[mrsdizzie]

For upcoming Gitea 1.9, there has been new logging implemented as described here:

https://docs.gitea.io/en-us/logging-configuration/

One thing that happens is that it enables colorized file logs by default, which it turns out will break things like fail2ban and potentially other software that is parsing logs for a specific regex. This specific case was reported in Discord earlier.

In our own fail2ban example here:

https://docs.gitea.io/en-us/fail2ban-setup/

It says to use failregex = .*Failed authentication attempt for .* from <HOST>

Which used to work, since the log file looked like this:

2019/05/13 21:57:53 [I] Failed authentication attempt for mrsdizzie from 24.46.193.14

But with colorized logs it looks like:

ESC[36m2019/05/14 02:14:53 ESC[0mESC[32mrouters/user/auth.go:159:ESC[32mSignInPost()ESC[0m ESC[1;32m[I]ESC[0m Failed authentication attempt for ESC[1mmrsdizzieESC[0m from ESC[1m24.46.193.14ESC[0m

A few things happen. This breaks the <HOST> match in fail2ban. In the example above, <HOST>would now be ESC[1m24.46.193.14ESC[0m

But worse, the log entry isn't matched at all because it now doesn't match any of the default date template patterns since it starts with ESC[36m2019/05/14 02:14:53

For reference, this is the fail2ban pattern gitea logs match now:
{^LN-BEG}ExYear(?P<_sep>[-/.])Month(?P=_sep)Day(?:T| ?)24hour:Minute:Second(?:[.,]Microseconds)?(?:\s*Zone offset)?

You can test these with:

fail2ban-regex -v --print-all-matched /var/lib/gitea/log/gitea.log '.*Failed authentication attempt for .* from <HOST>'

on master

Not really sure the best way to go about this. I think if we keep COLORIZE = true as a default it would be a bad breaking change in these situations.

Making Colorize false by default will keep things working, but still leaves people who do want to colorize with no simple fail2ban option.

It could also just never colorize those Authentication lines, which is a fix for people who are currently checking that. It would still break any other rules people may have created based on other log lines (and be inconsistent/not ideal), but would probably mitigate the most common case.

Suggestions welcome, perhaps theres an obvious solution I don't know.

[lunny]

Keep COLORIZE = true when in console, otherwise false.

[zeripath]

AFAIU colorizing logs is becoming the norm - but I can see that it might be awkward for some.

However, in terms of fail2ban configuration - it probably doesn't make sense to run it on the main log output from Gitea anyway. Fail2ban users should either use the new access log which logs in the standard NCSA common log format used by Apache et al, or should have a separate file log that prescreens using expression and sets the logging flags, colorize etc nicely for it.

Anyway if we're changing the default colorize we must change it very soon. I wasn't sure when I did this if this was the correct thing.

[sapk]

We can maybe indicate this in the beaking part of 1.9.0 release ?
In this case, we will need to add the tag kind/breaking on #6095

[mrsdizzie]

I enabled the access.log with ENABLE_ACCESS_LOG = true but it is still colorized by default and it did not contain theFailed authentication attempt entries you would need for this purpose. These are currently logged as:

log.Info("Failed authentication attempt for %s from %s", form.UserName, ctx.RemoteAddr())

Even if it did, it seems you would still need to turn off colorization on the file level, as it can't be enabled/disabled per log file?

If so I think @lunny suggestion if disabling it by default for file (and keeping true for console) will probably avoid issues when 1.9 is released.

I think that would still leave people who do enable COLORIZE = true for file log without a solution for fail2ban etc.., but that can be noted and thought on for the future.

[lunny]

@sapk added kind/break on #6095 and I think maybe we could send another break PR like @mrsdizzie said.