Closed niftylettuce closed 3 years ago
I think that instead of catching them as failures, the best approach is tempfail
.
I propose the following fix:
def check_dkim(msg, dnsfunc=None):
try:
d = DKIM(msg)
if(dnsfunc):
res = d.verify(dnsfunc=dnsfunc) and 'pass' or 'fail'
else:
res = d.verify() and 'pass' or 'fail'
except DKIMException as e:
res = 'fail'
+ except DNSException as e:
+ res = 'tempfail'
+ except Exception as e:
+ res = 'fail'
header_i = d.signature_fields.get(b'i', b'').decode('ascii')
header_d = d.signature_fields.get(b'd', b'').decode('ascii')
return DKIMAuthenticationResult(result=res, header_d=header_d, header_i=header_i)
I have also similarly added a fix here:
def check_arc(msg, logger=None, dnsfunc=None):
""" Compute the chain validation status of an inbound message.
@param msg: an RFC822 formatted message (with either \\n or \\r\\n line endings)
@param logger: An optional logger
@param dnsfunc: An optional dns lookup function (intended for testing)
"""
a = ARC(msg)
try:
if(dnsfunc):
cv, results, comment = a.verify(dnsfunc=dnsfunc)
else:
cv, results, comment = a.verify()
except DKIMException as e:
cv, results, comment = CV_Fail, [], "%s" % e
+ except DNSException as e:
+ cv, results, comment = CV_Fail, [], "%s" % e
+ except Exception as e:
+ cv, results, comment = CV_Fail, [], "%s" % e
return ARCAuthenticationResult(result=cv.decode('ascii'))
Correction to my earlier comment here https://github.com/ValiMail/authentication-headers/issues/18#issuecomment-670843354:
def check_dkim(msg, dnsfunc=None): try: d = DKIM(msg) if(dnsfunc): res = d.verify(dnsfunc=dnsfunc) and 'pass' or 'fail' else: res = d.verify() and 'pass' or 'fail' except DKIMException as e: res = 'fail'
- except DNSException as e:
- res = 'tempfail'
- except Exception as e:
res = 'fail'
header_i = d.signature_fields.get(b'i', b'').decode('ascii') header_d = d.signature_fields.get(b'd', b'').decode('ascii')
return DKIMAuthenticationResult(result=res, header_d=header_d, header_i=header_i)
Instead of tempfail
it should just be fail
. Also, we might just want to consolidate it into one line except Exception as e
to catch-all exceptions for dummy-proofing.
I think you've got it right here. According to https://www.iana.org/assignments/email-auth/email-auth.xhtml#email-auth-result-names there is no temporary error result fro ARC.
For DKIM, it does have a temperror, so I think that's fine the way you have it.
For example, this code does not catch the following errors:
dns.resolver.NoNameservers: All nameservers failed to answer the query ...
dns.exception.Timeout: The DNS operation timed out after 5.00025124217713 seconds