bugfix: change Rspamd DKIM default config location #3597

georglauterbach · 2023-10-22T17:09:21Z

Description

Instead of using /etc/rspamd/override.d/dkim_signing.conf, we will now be using /tmp/docker-mailserver/rspamd/override.d/dkim_signing.conf. The new location is persisted (and linked again during startup) and hence better suited.

Fixes #3593

Type of change

Bug fix (non-breaking change which fixes an issue)

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

Instead of using `etc/rspamd/override.d/dkim_signing.conf`, we will now be using `/tmp/docker-mailserver/rspamd/override.d/dkim_signing.conf`. The new location is persisted (and linked again during startup) and hence better suited.

georglauterbach · 2023-10-22T17:15:54Z

The test not ok 42 [Fail2Ban] ban ip on multiple failed login in 133164ms is starting to get annoying.

georglauterbach · 2023-10-23T18:48:26Z

Just FYI: the reason this PR fixes the mentioned race-condition is because the keys are now directly created in the persisted directory (which is linked to /etc/rspamd/overrid.d/ when DMS is rebooted at the latest). Previously, it could happen that /tmp/docker-mailserver/rspamd/override.d/ was not present, hence the configuration is not persisted at all.

I will add a commit in a second that provides

users with more detail about a possible misconfiguration
maintainers with more details on why the script does what it does (by adding a comment).

polarathene

Approving since rspamd integration is your domain and we often have conflicting opinions on how you approach that 😝

I raised some questions / concerns, but no major issues 👍

I'm still in favor of a unified DKIM generator personally, only difference then for rspamd support really is the additional .conf file, similar to how opendkim has its own configs.

Beyond that we've got two similar generator commands to maintain, and with the rspamd bug reports lately I'm not confident in fallback services being dropped any time soon.

polarathene · 2023-10-24T00:10:24Z

target/scripts/startup/setup.d/security/rspamd.sh

@@ -70,7 +70,7 @@ function __rspamd__run_early_setup_and_checks() {
  readonly RSPAMD_OVERRIDE_D='/etc/rspamd/override.d'
  readonly RSPAMD_DMS_D='/tmp/docker-mailserver/rspamd'

-  local RSPAMD_DMS_OVERRIDE_D="${RSPAMD_DMS_D}/override.d/"
+  local RSPAMD_DMS_OVERRIDE_D="${RSPAMD_DMS_D}/override.d"


Ok so here we have runtime setup referencing the config volume for rspamd, then later it symlinks the location instead of a copy?

If this is just config files, shouldn't that copy to an internal location instead? That way runtime isn't reliant on the config volume location which is more straight-forward?

Only benefit that comes to mind is avoiding check-for-changes.sh monitoring the config volume for rspamd config to sync over via copy? If those changes would require a reload of rspamd, you'd not have any benefit beyond being more implicit here by avoiding support via check-for-changes.sh?

I am not sure why I didn't actually initially constructed it like that. I will merge this PR and provide a follow-up because then it is easier to track the changes later! Thank you for making me aware of the obvious (again) ❤️

target/bin/rspamd-dkim

This was unnecessary, as explained in #3597 (comment)

@polarathene

* outsource Rspamd ENVs into explicit helper This will allow us to uniformly source the helper and get the values from everywhere consistently. This is more than desirable since we will be using these values not only for the Rspamd setup, but also for DKIM management and during change-detection. * integrate Rspamd into changedetection We outsource one more function to reside in the helper script for Rspamd so that we can call this function from the Rspamd setup and from the changedetection functionality too. * realize deprecation of old commands file for Rspamd THIS IS A BREAKING CHANGE! This change realizes the log message: "Using old file location now (deprecated) - this will prevent startup in v13.0.0" Startup will now fail. * added '--force' option to Rspamd DKIM script * use new helper to get ENVs for Rspamd in DKIM script * remove the need for linking directories This was unnecessary, as explained in #3597 (comment) * Apply suggestions from code review review by @polarathene * apply more review feedback from @polarathene - <#3599 (comment)> - <#3599 (comment)> * update documentation --------- Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>

@polarathene

* outsource Rspamd ENVs into explicit helper This will allow us to uniformly source the helper and get the values from everywhere consistently. This is more than desirable since we will be using these values not only for the Rspamd setup, but also for DKIM management and during change-detection. * integrate Rspamd into changedetection We outsource one more function to reside in the helper script for Rspamd so that we can call this function from the Rspamd setup and from the changedetection functionality too. * realize deprecation of old commands file for Rspamd THIS IS A BREAKING CHANGE! This change realizes the log message: "Using old file location now (deprecated) - this will prevent startup in v13.0.0" Startup will now fail. * added '--force' option to Rspamd DKIM script * use new helper to get ENVs for Rspamd in DKIM script * remove the need for linking directories This was unnecessary, as explained in docker-mailserver#3597 (comment) * Apply suggestions from code review review by @polarathene * apply more review feedback from @polarathene - <docker-mailserver#3599 (comment)> - <docker-mailserver#3599 (comment)> * update documentation --------- Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>

georglauterbach added 2 commits October 22, 2023 19:06

update Rspamd DKIM helper for config location

b4a8f63

Instead of using `etc/rspamd/override.d/dkim_signing.conf`, we will now be using `/tmp/docker-mailserver/rspamd/override.d/dkim_signing.conf`. The new location is persisted (and linked again during startup) and hence better suited.

updated Rspamd DKIM test

b6641c3

georglauterbach added area/scripts service/security/rspamd kind/bug/fix A fix (PR) for a confirmed bug labels Oct 22, 2023

georglauterbach added this to the v13.0.0 milestone Oct 22, 2023

georglauterbach requested a review from polarathene October 22, 2023 17:09

georglauterbach self-assigned this Oct 22, 2023

georglauterbach changed the title ~~bugfic: change Rspamd DKIM default config location~~ bugfix: change Rspamd DKIM default config location Oct 22, 2023

georglauterbach added 2 commits October 23, 2023 20:51

provide better warning messages and comments

523c9f7

fix typos

a1bb845

This comment was marked as outdated.

Sign in to view

polarathene approved these changes Oct 24, 2023

View reviewed changes

georglauterbach merged commit cb62ce2 into master Oct 24, 2023
7 checks passed

georglauterbach deleted the rspamd/dkim-fix-config-location branch October 24, 2023 08:31

georglauterbach added a commit that referenced this pull request Oct 24, 2023

remove the need for linking directories

09b6103

This was unnecessary, as explained in #3597 (comment)

georglauterbach added a commit that referenced this pull request Oct 24, 2023

remove the need for linking directories

6284863

This was unnecessary, as explained in #3597 (comment)

georglauterbach added a commit that referenced this pull request Oct 24, 2023

remove the need for linking directories

9f6e023

This was unnecessary, as explained in #3597 (comment)

polarathene mentioned this pull request Nov 27, 2023

bug report: Test on rspamd private key files not working as expected #3665

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bugfix: change Rspamd DKIM default config location #3597

bugfix: change Rspamd DKIM default config location #3597

georglauterbach commented Oct 22, 2023 •

edited

georglauterbach commented Oct 22, 2023

georglauterbach commented Oct 23, 2023 •

edited

This comment was marked as outdated.

polarathene left a comment •

edited

polarathene Oct 24, 2023

georglauterbach Oct 24, 2023

bugfix: change Rspamd DKIM default config location #3597

bugfix: change Rspamd DKIM default config location #3597

Conversation

georglauterbach commented Oct 22, 2023 • edited

Description

Type of change

Checklist:

georglauterbach commented Oct 22, 2023

georglauterbach commented Oct 23, 2023 • edited

This comment was marked as outdated.

polarathene left a comment • edited

Choose a reason for hiding this comment

polarathene Oct 24, 2023

Choose a reason for hiding this comment

georglauterbach Oct 24, 2023

Choose a reason for hiding this comment

georglauterbach commented Oct 22, 2023 •

edited

georglauterbach commented Oct 23, 2023 •

edited

polarathene left a comment •

edited