Another crash on IP address ending in newline

[Adi Pircalabu]

Aug 30 06:09:43 central1 policyd-spf[29530]: Traceback (most recent call last):
Aug 30 06:09:43 central1 policyd-spf[29530]: File "/usr/bin/policyd-spf", line 809, in <module>
Aug 30 06:09:43 central1 policyd-spf[29530]: instance_dict, configData, peruser, peruserconfigData)
Aug 30 06:09:43 central1 policyd-spf[29530]: File "/usr/bin/policyd-spf", line 623, in _spfcheck
Aug 30 06:09:43 central1 policyd-spf[29530]: mres = mfromquery.check()
Aug 30 06:09:43 central1 policyd-spf[29530]: File "/usr/lib/python3.4/site-packages/spf.py", line 547, in check
Aug 30 06:09:43 central1 policyd-spf[29530]: rc = self.check1(spf, self.d, 0)
Aug 30 06:09:43 central1 policyd-spf[29530]: File "/usr/lib/python3.4/site-packages/spf.py", line 586, in check1
Aug 30 06:09:43 central1 policyd-spf[29530]: return self.check0(spf, recursion)
Aug 30 06:09:43 central1 policyd-spf[29530]: File "/usr/lib/python3.4/site-packages/spf.py", line 906, in check0
Aug 30 06:09:43 central1 policyd-spf[29530]: if self.cidrmatch([arg], cidrlength): break
Aug 30 06:09:43 central1 policyd-spf[29530]: File "/usr/lib/python3.4/site-packages/spf.py", line 1348, in cidrmatch
Aug 30 06:09:43 central1 policyd-spf[29530]: for netwrk in [ipaddress.ip_network(ip) for ip in ipaddrs]:
Aug 30 06:09:43 central1 policyd-spf[29530]: File "/usr/lib/python3.4/site-packages/spf.py", line 1348, in <listcomp>
Aug 30 06:09:43 central1 policyd-spf[29530]: for netwrk in [ipaddress.ip_network(ip) for ip in ipaddrs]:
Aug 30 06:09:43 central1 policyd-spf[29530]: File "/usr/lib64/python3.4/ipaddress.py", line 84, in ip_network
Aug 30 06:09:43 central1 policyd-spf[29530]: address)
Aug 30 06:09:43 central1 policyd-spf[29530]: ValueError: '119.18.39.164\n' does not appear to be an IPv4 or IPv6 network

# pip3.4 show pyspf
DEPRECATION: Python 3.4 support has been deprecated. pip 19.1 will be the last one supporting it. Please upgrade your Python as Python 3.4 won't be maintained after March 2019 (cf PEP 429).
Name: pyspf
Version: 2.0.12
Summary: SPF (Sender Policy Framework) implemented in Python.
Home-page: http://pymilter.sourceforge.net/
Author: Stuart D. Gathman
Author-email: <email address hidden>
License: Python Software Foundation License
Location: /usr/local/lib/python3.4/dist-packages
Requires:
Required-by:
# pip3.4 show pypolicyd-spf
DEPRECATION: Python 3.4 support has been deprecated. pip 19.1 will be the last one supporting it. Please upgrade your Python as Python 3.4 won't be maintained after March 2019 (cf PEP 429).
Name: pypolicyd-spf
Version: 2.0.2
Summary: pypolicyd-spf SPF Postfix policy server implemented in Python.
Home-page: https://launchpad.net/pypolicyd-spf
Author: Scott Kitterman
Author-email: <email address hidden>
License: UNKNOWN
Location: /usr/local/lib/python3.4/dist-packages
Requires:
Required-by:

[Adi Pircalabu (apircalabu)]

The crash is caused by this txt record:
"v=spf1 ip4:203.143.89.146 ip4:203.27.138.188 ip4:119.18.39.164\010 include:spf.protection.outlook.com ~all"

[Scott Kitterman (kitterman)]

pyspf is where the retrieval and processing of the record is done, so it's a pyspf bug. It's moved to github, so please file an issue here:

https://github.com/sdgathman/pyspf/issues

[Adi Pircalabu (apircalabu)]

Scott, I think there are 2 issues here:
1. The bug in pyspf, that causes the policyd-spf crash
2. The fact that policyd-spf crashes on an invalid record. Wouldn't a try/except block be a better approach? Something like:

try:
    mres = mfromquery.check()
except Exception as e:
    e = sys.exc_info()
    exceptionmessage = "Exception: %s, locals: %s" %(e, locals())
    syslog.syslog("Ouch, caught exc: %s" %exceptionmessage)
    return(( 'dunno', exceptionmessage, instance_dict, None))

[Adi Pircalabu (apircalabu)]

Filed the pyspf issue to https://github.com/sdgathman/pyspf/issues/15

[Scott Kitterman (kitterman)]

On Friday, August 30, 2019 12:18:10 AM EDT you wrote:
> Scott, I think there are 2 issues here:
> 1. The bug in pyspf, that causes the policyd-spf crash
> 2. The fact that policyd-spf crashes on an invalid record. Wouldn't a
> try/except block be a better approach? Something like:
>
> try:
> mres = mfromquery.check()
> except Exception as e:
> e = sys.exc_info()
> exceptionmessage = "Exception: %s, locals: %s" %(e, locals())
> syslog.syslog("Ouch, caught exc: %s" %exceptionmessage)
> return(( 'dunno', exceptionmessage, instance_dict, None))

In theory I could do that, but this isn't a case where an error is ever an
appropriate response. pyspf should detect that this is an invalid record by
catching the ipaddress error.

Scott K

[Adi Pircalabu (apircalabu)]

From a coding perspective you're absolutely right, not to mention that trying to catch all possible exceptions is time consuming. pyspf needs to be fixed, no argument here.
But from a user & sysadmin perspective, all that's visible is a "451 4.3.5 Server configuration problem; [...]" translated at the other end to a nearly useless bounce message *only* after the queue lifetime of the message expires on the remote MTA. No hint in that bounce on what the actual problem might be. On the server running python-spf the sysadmin sees the stacktrace, yet this still isn't sufficient.
Regardless of where the issue is, the filter must NOT crash under any circumstance. There's of course the discussion on what should the filter return? Perhaps "dunno" or "reject"? If choosing "reject" the exception message can be customized so that the sender, when receiving the bounce, will be able to contact their service (DNS/email/etc) provider and offer them *some* useful information.
Currently the crash causes the MTA to return 451, which is probably the worst option in this particular case.

[Stuart Gathman (stuart-gathman)]

Scott is correct. This is a pyspf bug.

For sendmail, pymilter lets you specify how to handle untrapped exceptions. Hopefully, postfix also implements this API.

https://pythonhosted.org/pymilter/namespacemilter.html#a0cb75ce0c66ae67b2e4413e883b8ecf0

Note, I can't update pythonhosted.org - it is zombified. I haven't moved docs to a new platform yet. But the 1.0 docs are still accurate for set_exception_policy().

[Scott Kitterman (kitterman)]

The challenge is that the default is to 450 in this case and almost no one changes it. I think the point from the earlier comment (#6 if you're on the web UI) is reasonable and I'll look into doing something to safeguard against this sort of thing in the policy server.

Note that pypolicyd-spf was renamed to spf-engine (since it now includes a milter option as well). Any bug fixes will be applied there.

Scott K

[Stuart Gathman (stuart-gathman)]

Not able to reproduce with 2.0.12 or 2.0.13.

[Stuart Gathman (stuart-gathman)]

I found an reproducer for 2.0.12 with python3. But 2.0.13 works and 2.0.12 works with python2. Suggest apircalabu update to 2.0.13 prerelease.

[Scott Kitterman (kitterman)]

Fixed for the next release.

[Scott Kitterman (kitterman)]

--- 2.9.1 (2019-10-06)
  * Use /run instead of /var/run
  * Fix-up sysv init so it works
  * Catch pyspf tracebacks so at least we don't die (thanks to Adi Pircalabu
    for both the report and the suggested fix) (LP: #1842005)
  * Correct over-writing of SPF identity by SPF reason for HELO checks and the
    reverse for Mail From (LP: #1822685)