New colorized logging causes fail2ban to stop matching #3328

New Issue

GiteaMirror · 2025-11-02T05:08:38-06:00

GiteaMirror commented

2025-11-02 05:08:38 -06:00

Originally created by @mrsdizzie on GitHub (May 14, 2019).

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.

Originally created by @mrsdizzie on GitHub (May 14, 2019). 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.

GiteaMirror closed this issue

2025-11-02 05:08:39 -06:00

GiteaMirror commented

2025-11-02 05:08:40 -06:00

@lunny commented on GitHub (May 14, 2019):

Keep COLORIZE = true when in console, otherwise false.

@lunny commented on GitHub (May 14, 2019): Keep `COLORIZE = true` when in console, otherwise `false`.

GiteaMirror commented

2025-11-02 05:08:40 -06:00

@zeripath commented on GitHub (May 14, 2019):

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.

@zeripath commented on GitHub (May 14, 2019): 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.

GiteaMirror commented

2025-11-02 05:08:41 -06:00

@sapk commented on GitHub (May 14, 2019):

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

@sapk commented on GitHub (May 14, 2019): 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

GiteaMirror commented

2025-11-02 05:08:42 -06:00

@mrsdizzie commented on GitHub (May 14, 2019):

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.

@mrsdizzie commented on GitHub (May 14, 2019): I enabled the access.log with ```ENABLE_ACCESS_LOG = true``` but it is still colorized by default and it did not contain the```Failed 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.

GiteaMirror commented

2025-11-02 05:08:42 -06:00

@lunny commented on GitHub (May 14, 2019):

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

@lunny commented on GitHub (May 14, 2019): @sapk added kind/break on #6095 and I think maybe we could send another break PR like @mrsdizzie said.

GiteaMirror referenced this issue

2025-11-02 12:21:22 -06:00

[PR #3328] [MERGED] General refactor of the cmd package #16886

GiteaMirror referenced this issue

2025-11-02 12:21:34 -06:00

[PR #3337] [MERGED] Remove useless fmt.Errorf("%v", err) #16894

Sign in to join this conversation.