dkimpy

DNS exception when verifying DKIM signature

Bug #2018646 reported by David on 2023-05-06

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	dkimpy	Fix Released	Medium	Scott Kitterman	dkimpy 1.1.4

Bug Description

I updated to version 1.1.3. I have dnspython==2.3.0 installed.

I load a message from file and then call dkim.verify(). This initially fails due to SERVFAIL.

dns.resolver.NoNameservers: All nameservers failed to answer the query default._domainkey.<redacted>. IN TXT: Server 8.8.8.8 UDP port 53 answered SERVFAIL; Server 8.8.4.4 UDP port 53 answered SERVFAIL

However, the bug appears to be:

Traceback (most recent call last):
  File "<APP_PATH>\dkim_dns_test.py", line 6, in <module>
    print(dkim.verify(data))
  File "<PYTHON_PATH>\site-packages\dkim\__init__.py", line 1390, in verify
    return d.verify(dnsfunc=dnsfunc)
  File "<PYTHON_PATH>\site-packages\dkim\__init__.py", line 970, in verify
    return self.verify_sig(sig, include_headers, sigheaders[idx], dnsfunc)
  File "<PYTHON_PATH>\site-packages\dkim\__init__.py", line 805, in verify_sig
    except dns.exception.Timeout as e:
NameError: name 'dns' is not defined

In this commit some exception handling for timeouts is added: https://git.launchpad.net/dkimpy/commit/dkim/__init__.py?id=b74452d9da4878de67682d8f2cefbeecefc90c19

However, __init__.py does not import dns, therefore I'm not sure how this is supposed to work.

Perhaps the error handling should be here: https://git.launchpad.net/dkimpy/tree/dkim/dnsplug.py?id=233a9699ed8cb33854be418bd06a39e6d8a85610#n35 with a return of the new dkim.DnsTimeoutError exception for internal handling?

Or perhaps there is something unique about my environment which is causing this to fail.

Revision history for this message

David (edeca) wrote on 2023-05-06:

0001-Additional-error-handling.patch Edit (660 bytes, text/plain)

Add some additional error handling. Note this masks the second exception reported in this ticket by swallowing the first exception earlier.

There is likely still a bug in __init__.py when trying to catch exceptions in the dns.* namespace.

Revision history for this message

Scott Kitterman (kitterman) wrote on 2023-05-06:

I need to think about this one.

I agree with the specific point that dns isn't imported, so that's a bug.

I'm not sure I agree with your proposed patch though. dns.resolver.NoNameservers is indicative of a configuration problem at the verifier, so I'm not convinced dkimpy should just treat it the same as no response.

Imagine a case where your server's DNS configuration was completely broken so there really were no name servers available. I'm not certain the desired behavior would be to silently claim every DKIM verification failed.

A SERVFAIL for a TXT record is pretty unusual unless the DNS server in question is broken. Does the <redacted> domain still get a SERVFAIL from Google if you query it? If so, would you please pass the domain name in question to me privately so that I can investigate further.

I think it might be better to treat it as a timeout since that at least is a DNS related error condition.

I'm open to additional input on this if you or anyone else has thoughts on the matter.

Changed in dkimpy:
importance:	Undecided → Medium
status:	New → Confirmed
milestone:	none → 1.1.4

Revision history for this message

David (edeca) wrote on 2023-05-07:

The selector in this case is default[.]_domainkey[.]rotls[.]top. Despite signing some mail, this domain now has no nameservers.

This is not a problem with my local configuration. Wireshark shows the query goes to Google's DNS resolver with the recursive bit set in the query. The DNS server responds with 0 answer RRs and SERVFAIL.

dnspython explains the exception as "All nameservers failed to answer the query.". I am not sure of the total set of problems which could cause this, but the situation here is definitely something that should be handled.

I agree that other errors like dns.resolver.NoResolverConfiguration should not be caught, as they are definitely indicative of a local problem.

Regarding the underlying bug, perhaps catching reasonable exceptions from the abstracted DNS resolver libraries in dnslug.py and then raising a dkimpy specific exception might work. This would allow plug and play with various DNS libraries, keeping the implementation details in dnsplug.py.

Revision history for this message

Scott Kitterman (kitterman) wrote on 2023-05-07:

Interesting. Thanks for researching. That's a little different than I had assumed based on the name. I think your proposal is an appropriate way to go about it. I also agree that this error should be treated like a timeout.

Revision history for this message

Scott Kitterman (kitterman) wrote on 2023-05-12:

I have this fixed along the lines discussed in the bug. I'll release 1.1.4 shortly with this fix in it.