SPF check fails in exim4 with "failed to expand ACL string"

Should be fixed by https://git.exim.org/exim.git/commit/44b6e099b76f403a55e77650821f8a69e9d2682e which requires backporting work due to https://git.exim.org/exim.git/commit/d8fbda7da9eb20f98b89f625e8a77eacc443757d

cu Andreas

Thanks Andreas; from the upstream discussion sounds like there is an issue with early expansion of internally bracketized elements, e.g. "${quote:1}". The upstream patch looks sensible and probably not terribly difficult to backport.

Thomas, can you attach (not paste) your exim4 configuration files to this bug report (please remove any passwords, host/network names, or other private info that may be in there).

autogenerated config based on exim4 default config

/etc/exim4/update-exim4.conf.conf

/etc/exim4/exim4.conf.template

config is based on a fresh installed 22.10 VM (configtype 'internet', domain 'local.test', private 10.0.0.0/24 ip range), only added line "CHECK_RCPT_SPF = true" to exim4.conf.template

error in logs:
2022-12-06 14:06:55 H=(nucleus) [10.0.0.20] F=<email address hidden> temporarily rejected RCPT <email address hidden>: failed to expand ACL string "${run{/usr/bin/spfquery.mail-spf-perl --ip ${quote:$sender_host_address} --identity ${if def:sender_address_domain {--scope mfrom --identity ${quote:$sender_address}}{--scope helo --identity ${quote:$sender_helo_name}}}}{no}{${if eq {$runrc}{1}{yes}{no}}}}": Expansion of "${quote:$sender_host_address" from command "/usr/bin/spfquery.mail-spf-perl --ip ${quote:$sender_host_address" in ${run} expansion failed: missing } at end of string

Thanks, Thomas.

I was able to reproduce the bug here. This is what I did:

$ lxc network create test-exim4
$ lxc launch ubuntu:kinetic exim4-spf-1 --network test-exim4
$ lxc launch ubuntu:kinetic exim4-spf-2 --network test-exim4

Perform an upgrade and install exim4 on both containers. Run "dpkg-reconfigure exim4-config" and choose:

General type of mail configuration: internet site
System mail name: exim-spf-{1,2}.lxd (depending on the container you're running)
IP-addresses to listen on for incoming SMTP connections: blank
Other destinations for which mail is accepted: exim-spf-{1,2}.lxd (depending on the container you're running)
Domains to relay mail for: blank
Machines to relay mail for: blank
Keep number of DNS-queries minimal (Dial-on-Demand)? No
Delivery method for local mail: mbox format in /var/mail/
Split configuration into small files? No
Root and postmaster mail recipient: blank

On the first container:

Edit /etc/exim4/exim4.conf.template, remove the IP range of your local LXD network from the ROUTER_DNSLOOKUP_IGNORE_TARGET_HOSTS list. Restart exim4.service.

On the second container:

Edit /etc/exim4/exim4.conf.template, remove the IP range of your local LXD network from the ROUTER_DNSLOOKUP_IGNORE_TARGET_HOSTS list. Define "CHECK_RCPT_SPF = true".

# apt install spf-tools-perl

Restart exim4.service

Now you should be able to send an email from the first to the second container.

On the first container:

# echo TEST | mail -s test <email address hidden>

On the second container, inspect /var/log/exim4/mainlog. You should see the error mentioned by Thomas.

I haven't tested to check if the patch mentioned above fixes the problem.

This bug exists on debian also. Trying to build exim 4.96.x or install any 4.96.X deb package from debian repositories leads to this spf check error. Downgrade to 4.95 works ok.

This bug affects not only spf expressions but other expressions in configs too.

I think I have a partial fix. Andreas is right that 44b6e099 does not apply cleanly, due to the refactoring work from d8fbda. The refactoring patch is rather big and neither does it apply cleanly, but if I understand it correctly in essence it's changing the calling semantics for the expand_string_internal() call. I've adjusted 44b6e099 to use the old calling semantics, packaged up the fix and stuck it into a PPA here:

    https://launchpad.net/~bryce/+archive/ubuntu/exim4-sru-lp1998678

Next, I followed the exact steps outlined in the SRU text of this bug's description. I.e., constructed a pair of containers with exim4 and sent an email from one to the other. Sure enough, without my PPA it reproduces the error 'expansion failed: missing } at end of string', and with the PPA installed it does not. Unfortunately, it instead gives a different error: 'expansion failed: condition name expected, but found ""'

I've tinkered around with the exim4.conf.template, and as before it works if I remove any of the internally expanded {} stuff. (I think I also found an unrelated bug in ubuntu's template, but that's a separate topic...)

So... there seems to be more going on than *just* 44b6e099, unless I've done the backport wrong. Perhaps there's more than just refactoring work in the refactoring patch? Or perhaps there's another patch in upstream trunk that's required?

I'd appreciate advice, and especially if others could check my PPA and if it also fails to figure out what's going on. Meanwhile, I'm going to re-test the testcase on mantic, and if that's also no good, next action would be to take another shot at the backport, or perhaps package a git snapshot of the current upstream trunk.

The separate topic I mentioned, which seems to be an unrelated side issues, is that I notice --identity appears to be duplicated in our template:

    condition = ${run{/usr/bin/spfquery.mail-spf-perl --ip \
                   ${quote:$sender_host_address} --identity \
                   ${if def:sender_address_domain \
                       {--scope mfrom --identity ${quote:$sender_address}}\
                       {--scope helo --identity ${quote:$sender_helo_name}}}}\
                   {no}{${if eq {$runrc}{1}{yes}{no}}}}

Thomas stripped that down to this valid code, which he found ran correctly (I did similarly and also confirm it works):

    condition = ${run{/usr/bin/spfquery.mail-spf-perl --ip \
                     $sender_host_address --identity \
                     --scope mfrom --identity sender_address}\
                     {no}{${if eq {$runrc}{1}{yes}{no}}}}

From `spfquery.mail-spf-perl --help` it looks like --identity expects an argument and appears intended to be used just once so I'm guessing the first --identity there is a stray typo. It doesn't seem to cause any problems, but am I correct that it shouldn't be there to begin with?

Another side question, but this kind of pertains to the problem at hand. Should Ubuntu even be providing this config snippet? If I understand correctly, we are disabling spf since the lib it requires is in universe, and evidently not really maintaining/testing the snippet ourselves; would it be better to leave this config work to users who want the feature, and not ship the snippet? Thomas, I'd be keen to hear your opinion on what we should do here.

I'm also not opposed to investigating if we should consider promoting the spf lib to main. I assume there were good reasons why we opted to disable the lib rather than do that, but those reasons seem lost to the mists of time. Also it sounds like it's a bit of an edge case in terms of actual usefulness, so maybe isn't that crucial to have, but if anyone can make a good case for favoring it, I can look into it.

I think providing the snipplet is good. There is a lot stuff to take care about, when running an SMTP server and the Ubuntu/Debian exim config covers a lot of it, that can be simply enabled. Split config strategy makes it easy to extend and reduces work dist upgrades. Covering SPF is basic, I think it should be kept.

Thanks, noted. So perhaps we could look at cleaning up that snippet once this bug is resolved.

I added the latest exim4 from mantic to my PPA with the patch, and verified it too shows the same problem as on lunar:

2023-06-08 21:41:30 H=exim4-sru-lp1998678-mantic-spf-1.lxd [10.167.37.174] X=TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256 CV=no F=<email address hidden> temporarily rejected RCPT <email address hidden>: failed to expand ACL string "${run{/usr/bin/spfquery.mail-spf-perl --ip ${quote:$sender_host_address} --identity ${if def:sender_address_domain {--scope mfrom --identity ${quote:$sender_address}}{--scope helo --identity ${quote:$sender_helo_name}}}}{no}{${if eq {$runrc}{1}{yes}{no}}}}": Expansion of "${if" from command "/usr/bin/spfquery.mail-spf-perl --ip ${quote:$sender_host_address} --identity ${if def:sender_address_domain {--scope mfrom --identity ${quote:$sender_address}}{--scope helo --identity ${quote:$sender_helo_name}}}" in ${run} expansion failed: condition name expected, but found ""

Attached is the backported patch used for this. Any advice on what I'm missing would be appreciated.

Bryce wrote:
> So... there seems to be more going on than *just* 44b6e099, unless I've done the backport wrong. Perhaps there's more than just refactoring work in the refactoring patch? Or perhaps there's another patch in upstream trunk that's required?

Hello Bryce,

afaict this is simply a (previously) unreported bug which is still open in GIT. See https://bugs.exim.org/show_bug.cgi?id=3008

[EDIT 2023-06-11] Actually it is not a bug. $run expansion was changed in 4.96 with ...
----------
JH/24 The ${run} expansion item now expands its command string elements after
      splitting. Previously it was before; the new ordering makes handling
      zero-length arguments simpler. The old ordering can be obtained by
      appending a new option "preexpand", after a comma, to the "run".
----------
... and the exim.conf-snippet needs to be changed accordingly.

cu Andreas

Thanks Andreas for discussing this with upstream. I'm very glad to hear it identified as a separate unrelated issue, which I think may allow moving ahead with fixing the closing-bracket bug at least. The bad news being that this upstream resolution leaves Ubuntu's config snippet in a bit of a pickle.

Other than using preexpand, I toyed around with use of a temporary local variable to store the scope parameters, however from what I can understand exim4's config file format only permits pre-defined permanent global variables. I think with a bit more tinkering I could force-fit some kind of workaround. But that would feel like a brittle solution to the problem.

It strikes me though that, as I mentioned above, this situation apparently arose because in the past Ubuntu _disabled_ SPF support in its main distribution. Since this snippet only comes into play for people re-enabling it (i.e. going beyond stock Ubuntu), it lives outside the borders of regular support, and evidently has been festering bugs as a result. So even if we do repair things nicely this time, who's to say this situation won't arise again in a few years. Clearly this needs a more sustainable solution...

Here's what I'd like to propose.

I'll take the task of moving ahead with this fix. I'll need to redefine the SRU test case to include a step that replaces the problematic snippet with one that *does* trip the '}' regression but doesn't trip the second regression. I'll land that in mantic presently, and work on pushing the SRU through to get the fix into kinetic and lunar.

It's been mentioned that this '}' affects Debian as well. I'll also take the task of forwarding the issue (and Ubuntu's fix) to Debian.

The second part is the configuration snippet. As I outlined above, I don't think this really belongs in Ubuntu, and I suspect it makes more sense to carry it in Debian. That's where the config file is actively maintained, and since Debian includes spf by default they'd likely be testing it more actively than we do. So, Andrew and Thomas, I'd like to ask you two to contact Debian and work with them on how to improve the spf configuration support there. Meanwhile, since it doesn't work now anyway, I'll drop the old snippet since it's busted anyway and maybe save users some confusion.

How does that plan sound?

Hopefully, if Debian takes the '}' fix (or gets it via packaging a new upstream release), and addresses the configuration logic, then Ubuntu can get back into sync and provide a better, more sustainable solution for Ubuntu users wishing to enable spf.

Hello,

I think this should work with (bugfix patched 4.96-16)

-------------
 condition = ${run{/usr/bin/spfquery.mail-spf-perl --ip \
               ${quote:$sender_host_address} \
        --scope \
               "${if def:sender_address_domain \
                   {mfrom}\
                   {helo}}"\
               --identity \
               "${if def:sender_address_domain \
                   {$sender_address}\
                   {$sender_helo_name}}"\
               }\
               {no}{${if eq {$runrc}{2}{yes}{no}}}}
-------------
(Yes this also drops the first --identity)

This bug was fixed in the package exim4 - 4.96-15ubuntu2

---------------
exim4 (4.96-15ubuntu2) mantic; urgency=medium

  * d/p/fix-run--arg-parsing.patch: Fix argument parsing for ${run }
    expansion. Previously, when an argument included a close-brace
    character (e.g. it itself used an expansion) an error occurred.
    (LP: #1998678)
  * d/d/c/a/30_exim4-config_check_rcpt: In SPF config snippet,
    drop support for helo scope.

 -- Bryce Harrington <email address hidden> Wed, 07 Jun 2023 22:28:04 -0700

Hello Thomas, or anyone else affected,

Accepted exim4 into lunar-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/exim4/4.96-14ubuntu1.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-lunar to verification-done-lunar. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-lunar. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Hello Thomas, or anyone else affected,

Accepted exim4 into kinetic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/exim4/4.96-3ubuntu1.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-kinetic to verification-done-kinetic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-kinetic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

I've verified the -proposed updates in LXC containers for both kinetic and lunar, running through the Test Case both without the fix, and with it.

Lunar:
2023-07-05 22:33:08 1qHB3c-0001jF-1v <= <email address hidden> H=exim4-sru-lp1998678-lunar-spf-a.lxd [10.253.102.152] P=esmtps X=TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256 CV=no K S=997 <email address hidden>
2023-07-05 22:33:08 1qHB3c-0001jF-1v => ubuntu <email address hidden> R=local_user T=mail_spool
2023-07-05 22:33:08 1qHB3c-0001jF-1v Completed

Kinetic:
2023-07-05 22:33:10 1qHB3e-0001pu-2R <= <email address hidden> H=exim4-sru-lp1998678-kinetic-spf-a.lxd [10.253.102.65] P=esmtps X=TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256 CV=no K S=1078 <email address hidden>
2023-07-05 22:33:10 1qHB3e-0001pu-2R => ubuntu <email address hidden> R=local_user T=mail_spool
2023-07-05 22:33:10 1qHB3e-0001pu-2R Completed

This bug was fixed in the package exim4 - 4.96-14ubuntu1.1

---------------
exim4 (4.96-14ubuntu1.1) lunar; urgency=medium

  * d/p/fix-run--arg-parsing.patch: Fix argument parsing for ${run }
    expansion. Previously, when an argument included a close-brace
    character (e.g. it itself used an expansion) an error occurred.
    (LP: #1998678)

 -- Bryce Harrington <email address hidden> Fri, 10 Feb 2023 00:17:40 -0800

The verification of the Stable Release Update for exim4 has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package exim4 - 4.96-3ubuntu1.2

---------------
exim4 (4.96-3ubuntu1.2) kinetic; urgency=medium

  * d/p/fix-run--arg-parsing.patch: Fix argument parsing for ${run }
    expansion. Previously, when an argument included a close-brace
    character (e.g. it itself used an expansion) an error occurred.
    (LP: #1998678)

 -- Bryce Harrington <email address hidden> Tue, 11 Apr 2023 18:16:12 -0700