Bug: Too many newlines removed from amavis DKIM config in 60-dkim causes extra DKIM entries to be lost

DKIM entries in amavisd are configured in the file 60-dkim which looks like this:

dkim_key('domain1.com', 'default', '/var/lib/amavis/dkim/domain1.com.private');
dkim_key('domain2.com', 'default', '/var/lib/amavis/dkim/domain2.com.private');
dkim_key('domain3.com', 'default', '/var/lib/amavis/dkim/domain3.com.private');
dkim_key('domain4.com', 'default', '/var/lib/amavis/dkim/domain4.com.private');
dkim_key('domain5.com', 'default', '/var/lib/amavis/dkim/domain5.com.private');

In server/plugins-available/mail_plugin_dkim.inc.php:311 the function remove_from_amavis uses the following regex to remove domains from DKIM signing because DKIM is disabled or they are deleted:

/(\n|\r)?dkim_key.*".$key_domain.".*(\n|\r)?/

This regex removes the newline from both the beginning and the end of the string. This causes the preceding and subsequent entries to be concatenated on a single line. Using the above config as an example, if domain3.com was removed using the regex, the resulting config looks like this:

dkim_key('domain1.com', 'default', '/var/lib/amavis/dkim/domain1.com.private');
dkim_key('domain2.com', 'default', '/var/lib/amavis/dkim/domain2.com.private');dkim_key('domain4.com', 'default', '/var/lib/amavis/dkim/domain4.com.private');
dkim_key('domain5.com', 'default', '/var/lib/amavis/dkim/domain5.com.private');

This is unintentional however amavis can still parse the config correctly as it is valid perl. The more serious bug arises if domain2.com is deleted. In that case, the regex is too greedy and munges domain4.com because the previous deletion put it on the same line. The final config looks like this:

dkim_key('domain1.com', 'default', '/var/lib/amavis/dkim/domain1.com.private');dkim_key('domain5.com', 'default', '/var/lib/amavis/dkim/domain5.com.private');

With domain4.com deleted by accident and domain1.com and domain5.com on the same line ready to trigger the bug again.

After finding this bug, I checked one one of our mail servers and out of 159 domains with DKIM enabled, only 119 were present in the config file. 40 domains had disappeared and were not being DKIM signed due to this overzealous regex.

I can submit a patch for the regex which will prevent future occurrences, but any servers running amavis should check if they have been affected. Re-syncing mail domains might fix it, I haven't tried that yet I'm still triaging.

To identify how many domains amavis is configured for the easiest method is to run:

# Path to amavisd.conf may vary - tested on AlmaLinux 8
amavisd -c /etc/amavisd/amavisd.conf testkeys

Then compare that number with:

SELECT `domain` FROM `mail_domain` WHERE `server_id` = ? AND `active` = "y" AND `dkim` = "y";

Where server_id is the mail server under test. You should also be able to ls -al /var/lib/amavis/dkim/*.private | wc -l which seems to give the same answer as the SQL above because the key installation is not affected by this bug.