trusteddomainproject / OpenDKIM

Other
97 stars 52 forks source link

NXDOMAIN handling in stub resolver #49

Open sp-andwei opened 5 years ago

sp-andwei commented 5 years ago

Hi everyone,

I noticed that with the default resolver active, libopendkim reports DKIM_SIGERROR_KEYFAIL if it receives a NXDOMAIN reply. This is not distinguishable from a SRVFAIL "reply", which would translate into a TEMPFAIL according to RFC 6376. To my understanding, a DKIM_SIGERROR_NOKEY would be the more appropriate reaction, which then should be translated into a PERMFAIL in terms of DKIM results.

The default resolver in dkim-dns.c uses res_(n)query, which returns a failure and sets h_errno to HOST_NOT_FOUND in case of a NXDOMAIN reply. This is never checked however, so the evaluation of results in dkim-key.c justs sees a DKIM_DNS_ERROR and reports DKIM_STAT_KEYFAIL (which translates to DKIM_SIGERROR_KEYFAIL), which indicates a temporary failure (retry again later), which does not really make sense for NXDOMAIN.

Main problem is that the res_(n)query documentation does not cleary state what happens with the passed buffer in case of a NXDOMAIN response and that libopendkim does not define a explicit DKIM_DNS_ERROR_NOTFOUND.

The implementations I've seen all return -1 but still fill the buf with the actual DNS response packet. This could be used by copying the header data (maybe even setting hdr->rcode = NXDOMAIN for good measure) and returning DKIM_DNS_SUCCESS, as well as checking for NXDOMAIN earlier in dkim_get_key_dns (see workaround)

If someone with the capability to merge agrees to that solution or has ideas for improvement I'm happy to provide a PR.