Introduce ENABLE_DNSBL env

[casperklein]

Description

This PR makes the usage of the postfix/postscreen DNS block lists optional.

Fixes issues like #1927

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (non-breaking change that does improve existing functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (README.md or the documentation under docs/)
• If necessary I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[georglauterbach]

Not sure whether Postscreen should be affected by this too or not, but you could think about these lines in Postfix's configuration as well:

docker-mailserver/target/postfix/main.cf

Lines 58 to 66 in 701037d

	postscreen_dnsbl_sites = zen.spamhaus.org*3
	bl.mailspike.net
	b.barracudacentral.org*2
	bl.spameatingmonkey.net
	dnsbl.sorbs.net
	psbl.surriel.com
	list.dnswl.org=127.0.[0..255].0*-2
	list.dnswl.org=127.0.[0..255].1*-3
	list.dnswl.org=127.0.[0..255].[2..3]*-4

[casperklein]

Good catch. I've overlooked that, because in my setup postscreen_dnsbl_sites is empty due a custom postfix-main.cf.

[georglauterbach]

Just a short question: Why is the variable called ENABLE_DNSBL and the function that is executed disables the variables? Wouldn't it make much more sense to remove the lists from the configuration files and add them if ENABLE_DNSBL=1?

[casperklein]

It did it this way, because currently there is no DISABLE_VAR. This was just done, to align with the existing namings.

[georglauterbach]

It did it this way, because currently there is no DISABLE_VAR. This was just done, to align with the existing namings.

I'm very much fine with the naming of the variable, just the implementation is logic-wise implemented in exactly the opposite way compared to how I would have done it :) I'd expect the variable to add the lists, not to disable them :D

[casperklein]

The result is the same. Background: I mainly migrated my personal "remove blocklists" stuff from my user-patches. That's why it's not the other way around.

[polarathene]

Other than concerns raised earlier, LGTM 👍

[polarathene]

TLS cipherlist test was failing similar to what @casperklein was experiencing in a separate issue I think, might be an issue with upstream testssl image (still failing).

EDIT: Ran the tests locally and inspected the JSON, noticed that the fs field we would normally check has been changed to serverPreferences instead. Found the relevant PR that introduced that change upstream and opened up a PR to adapt our test to that change. Once that's merged the test will pass again.

[github-actions]

Documentation preview for this PR is ready! 🎉

Built with commit: 54ce77d