[enh] Add fail2ban helpers

[JimboJoe]

Problems

To enhance applications protection against hackers/spammers, etc., we can propose helpers to ease the creating of fail2ban jails.

Solution

Add ynh_add_fail2ban_config and ynh_remove_fail2ban_config helpers.
A successful implementation example is the piwigo app.

PR Status

Working, but opinion welcome!
And should be implemented in other applications to validate its principle.

Validation

• Principle agreement 0/2 :
• Quick review 0/1 :
• Simple test 0/1 :
• Deep review 0/1 :

[Psycojoker]

As anyone tested those? I have no knowledge of fail2ban.

[JimboJoe]

At the moment, only the (nearly official) Piwigo application implemented it. So maybe should we wait for a couple of other successful implementations to merge it...?

[alexAubin]

As far as I know, fail2ban's conf format changed in Stretch so that might affect these helpers :s

[Psycojoker]

At the moment, only the (nearly official) Piwigo application implemented it. So maybe should we wait for a couple of other successful implementations to merge it...?

Any update on that?

[JimboJoe]

It's currently being implemented in Wordpress, and has evolved here.

[alexAubin]

About this :

we should be careful that the fail2ban conf format changed a bit in stretch and is backward-incompatible. I don't know if these helpers are affected... but right now, the step in the stretch migration is to remove all existing fail2ban conf and regenerate it from scratch :s

[alexAubin]

Bumpitybump, is there any update on this ? Are the helpers stable enough to be considered merged in the core ? (Do they work on Stretch ?)

[JimboJoe]

I've updated the helpers to be on-par with the ones from Experimental_helpers.
I've tested again piwigo on Stretch and they work as advertised.
I understand you had to delete all existing fail2ban jails when migrating from Jessie to Stretch, so that means existing apps won't be protected by fail2ban until their next upgrade (which I find perfectly acceptable)?

[alexAubin]

I understand you had to delete all existing fail2ban jails when migrating from Jessie to Stretch, so that means existing apps won't be protected by fail2ban until their next upgrade

Yes indeed :s

[JimboJoe]

This helper currently only works on Stretch...

[zamentur]

Not a problem if it only works on Stretch, we could merged it only in stretch...

[Psycojoker]

We are now on stretch, shouldn't we merge this?

[maniackcrudelis]

Is anyone know why is there a '1' as a second argument for this helper ?
I can't find any version of ynh_backup_if_checksum_is_different with a second argument.

This is crashing getopts, which is another problem to fix

[maniackcrudelis]

@YunoHost/apps

[alexAubin]

Bump @YunoHost/apps , it'd be nice to have this for 3.5 :s (in particular, @Josue-T , it was noted "Josue will have a look at integrating fail2ban in his apps, which requires to have several regex instead of just one. He'll iterate on the design." 😅 so if you happen to have time to work on this that'd be great !)

[Josue-T]

Yes, I might find some time for that in theses next 2 weeks.
If not I think we should merge this as it is because I might not find the time for the theses next month

[Josue-T]

Now I updated the helper.

Tested on synapse

[maniackcrudelis]

@Josue-T that's a completely different helper !
You break getopts usage as well as usage of this helper in one line without template.

Maybe for synapse you need to use a template, but most of the apps don't need that and works perfectly with the previous version of this helper.
So, if you want to ADD a template usage for this helper, why not, but I don't think that to force to use a template is a good idea.

[Josue-T]

It's just more like the helper nginx.

I think it's the last time change the way how this helper work. And I think that probably many apps need more flexibility than what this helper provided.

[Josue-T]

If you really want to keep first idea. We can make a helper ynh_add_fail2ban_config with the old idea. And a other helper ynh_add_fail2ban_template with my idea.

[maniackcrudelis]

nginx doesn't have a standard config, each app has a different config.
You could have said that it's like php, which is true. And actually, this helper could use a template less architecture. But the helper was built over the way we were handling php by that time.

Anyway, except synapse, no app is using a template for fail2ban. And so far we don't need to add that in apps. So we should keep that helper as simple to use as we can. So, like it was.

Again, if you need a template feature for synapse with fail2ban, I agree to ADD this to the helper. But, this shouldn't break the way the helper was working so far.
Nor breaking getopts...

So, add 2 arguments to the helper, one for using a template, another for adding other variables. That's a good thing for those who could need a specific fail2ban config. But do not break the helper because you need a specific way to handle the config.

[Josue-T]

Now there are the 2 possibility

[maniackcrudelis]

LGTM, but not tested.

[maniackcrudelis]

Bump

@YunoHost/apps

[alexAubin]

Not really reviewed but trusting you guys ... Would merge in a few days if no opposition (still would be nice to have other opinions from @YunoHost/apps )

[maniackcrudelis]

Let's merge in 3 days

[alexAubin]

Thanks for the review folks, finally sounds like we gonna merge this \o/