search

YunoHost-Apps / wallabag2_ynh

Wallabag v2 package for YunoHost
https://www.wallabag.org/
GNU Affero General Public License v3.0
62 stars 14 forks source link

Add Fail2ban support #65

Closed lapineige closed 5 years ago

lapineige commented 5 years ago

Problem

Solution

The number of retry is set to 5, I think it allows users to make a reasonable number of errors while restricting brute force possibilities.

Warning : this PR will drop support for version older than 3.5, in particular Yunohost 2.7 (Debian 8).

PR Status

Validation

Minor decision

lapineige commented 5 years ago

Changed the base branch to master, as it is an high priority issue, we can't wait for testing to be merged.

lapineige commented 5 years ago

I fixed the regex conf, at first I did not understand how to configure it correctly.

I tried a fresh install + removal, it works. Fail2ban seems to be active: after 5 failed login attempts, the login page doesn't load.

It's ready for review.

maniackcrudelis commented 5 years ago

Adding fail2ban is a good improvement, not a security issue needed to be quickly fixed.

lapineige commented 5 years ago

Users password are not protected right now, and it's the same as the SSO password, so it would be a lot better to prevent brute force attack. It's not a security flaw, but still pretty important.

maniackcrudelis commented 5 years ago

User passwords are protected by https. Brute force attack aren't prevented indeed and that's a pretty good improvement. Not a security issue.

JimboJoe commented 5 years ago

The CI job is failing... :thinking:

lapineige commented 5 years ago

Do you know why ? (where can I find the logs ?)

JimboJoe commented 5 years ago

Yep, you click on the build badge, then "last build", then "console output".

lapineige commented 5 years ago

This page ? https://yunohost.github.io/ci/

maniackcrudelis commented 5 years ago

There https://ci-apps-hq.yunohost.org/jenkins/job/wallabag2_ynh%20PR65/5/console

lapineige commented 5 years ago

Ok, only upgrade from this version fails…

I found the issue: I fixed the regex syntax in the install script, but not in the upgrade one Should be fixed now.

JimboJoe commented 5 years ago

Upgrade is stille failing on the CI.

Warning: yunohost.hook <lambda> - [3344.1] Job for fail2ban.service failed because the control process exited with error code.
Warning: yunohost.hook <lambda> - [3344.1] See "systemctl status fail2ban.service" and "journalctl -xe" for details.
Warning: yunohost.hook <lambda> - [3344.1] !!
Warning: yunohost.hook <lambda> - [3344.1]   wallabag2's script has encountered an error. Its execution was cancelled.
Warning: yunohost.hook <lambda> - [3344.1] !!
Warning: yunohost.hook <lambda> - [3344.1] ./upgrade: line 61: ynh_backup_after_failed_upgrade: command not found
lapineige commented 5 years ago

But we don't have the log for fail2ban… :(

JimboJoe commented 5 years ago

Isn't the problem reproducible with package_check in your environment?

lapineige commented 5 years ago

How do I use it ?

JimboJoe commented 5 years ago

As an app maintainer, you will love package_check! Have a look at the README, you can install it on your test server/VM. It's been developed mainly by @maniackcrudelis, and it's what produces the results of the CI.

lapineige commented 5 years ago

I don't have any VM or test server for that :/

JimboJoe commented 5 years ago

Then you'll be interested by this forum post :wink:

lapineige commented 5 years ago

Ok, I tried again on my main server (backup + remove old wallabag + install master + upgrade to this branch), the error is:

No file(s) found for glob /var/www/wallabag2/var/logs/prod.log

I fixed that in the install script, but not in the upgrade script :sweat_smile: I'll do it.

JimboJoe commented 5 years ago

By the way, more relevant link here to use the CI available for app packagers :wink:

lapineige commented 5 years ago

CI is succeeding right now.

Let's merge ?

JimboJoe commented 5 years ago

Can be merged in 3 days (if @maniackcrudelis confirms his code review after late changes)

JimboJoe commented 5 years ago

Can be merged in 3 days.

lapineige commented 5 years ago

Thanks @maniackcrudelis for the improvements :)