search

trusteddomainproject / OpenDKIM

Other
97 stars 52 forks source link

Make the standard resolver DNSSEC aware #1

Closed jablko closed 6 years ago

jablko commented 7 years ago

Would you consider this patch, to make the standard resolver implementation DNSSEC aware, like the libunbound one?

jablko commented 6 years ago

I think it's a good idea to add bind support for DNSSEC, but shouldn't we keep backward compatibility with the res_n*() API?

I tested this change with the res_n*() API (HAVE_RES_NINIT enabled) and it worked as expected (set DKIM_DNSSEC_SECURE for a protected key, DKIM_DNSSEC_INSECURE for an unsigned one) ... Did you run in to problems?

Also, for non-DNSSEC versions of bind, shouldn't we still exclude values that make no sense (e.g., BogusKey, UnprotectedKey)?

The RES_USE_DNSSEC option just sets the D0 bit on the query; some servers set the AD response bit only if the D0 query bit is set, but the standard doesn't prescribe it:

The name server side SHOULD set the AD bit if and only if the resolver side considers all RRsets in the Answer section and any relevant negative response RRs in the Authority section to be authentic.

(regardless of the D0 bit, RFC 4035, section 3.2.3). So say you're running a validating recursive resolver on localhost, that's compliant and sets the AD bit whether the D0 bit (RES_USE_DNSSEC) is present or not, then I'd expect OpenDKIM to behave the same with a stub resolver that lacks RES_USE_DNSSEC as with one that has it ...

It's a hypothetical point: I don't have a server that sets the AD bit regardless of the D0 bit, nor a stub resolver that lacks RES_USE_DNSSEC, so I'm fine making BogusKey and UnprotectedKey conditional on #if defined(USE_UNBOUND) || defined(RES_USE_DNSSEC), if you like?

mskucherawy commented 6 years ago

Sorry for the delay replying.

jablko commented 6 years ago

Thanks for merging this!