-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add nextcloud-auth and nextcloud-domain filters #3581
base: master
Are you sure you want to change the base?
Conversation
Do substitutions have to be enclosed in |
@Deric-W: Have you seen comments on your PR? |
Sorry for the very long delay, I totally forgot about this PR. |
The filters are based on the one in the hardening guide but split to allow different jails.
I noticed that the group regex I fixed it by modifying the group regex with |
yes, because now - (?:(?:,?\s*"\w+":(?:"(?:[^"\\]|\\"|\\\\)*"|\w+))*)
+ (?:(?:,?\s*"\w+":(?:"(?:[^"\\]+|\\.)*"|\w+))*) (also note the |
It was moved to `nextcloud-common.conf`
Thanks for catching the mistake 👍 |
One problem with using this filter - though not a problem with the regex or anything here per se - is that it is prone to false positives because the logs don't actually say anything about why the login failed. It could be because of a genuine invalid login and/or brute force attempt, sure, but the logs will look identical if there is some sort of problem with the backend authentication provider, triggering a slew of false alarms. The logged statement is true in both cases, in that the login did indeed fail, but some more detail (like what eg. Dovecot does) would allow to differentiate between these two very different problems, and make it possible to adjust the filter regex accordingly to achieve higher accuracy. Not something that can be fixed here of course, as this is clearly a Nextcloud bug (or at least I would consider it to be one), but perhaps something to note as a comment, a possible gotcha worth being aware of? I at least had to disable my Nextcloud jail because I could not find any way to prevent this from happening. |
I guess this PR makes #1913 outdated. Does it? |
I think so, but I noticed that the mentioned PR seems to expect a different nextcloud log format (probably because it is 7 years old). |
@yitzhaq Feel free to bring up your suggestions at the Nextcloud Server (if logging behavior changes are required) and/or Documentation (if rule changes are required) repositories. :-) |
Since nextcloud has a hardening guide which contains a fail2ban filter I decided that it would be useful for including it here (split into two filters to allow different jails).
Since the original filter fails when using the systemd journal I modified it to accept the prefix line (optional) from
common.conf
.Before submitting your PR, please review the following checklist:
against certain release version, choose
0.9
,0.10
or0.11
branch,for dev-edition use
master
branchfailregex
for filterX
with sample log lineswithin
fail2ban/tests/files/logs/X
file