Detect remote DNS server does not support EDNS as per RFC 6891

[eriklax]

This pull request will change the behavior to not unflag the EDNS flag on the channel (resolver context) on errors, only for the current request. Because not all errors are related to EDNS there is no reason to disable it globally. This was for me a very unexpected behavior. I can see how the current request should/could be retried without for compatibility with old/broken resolvers.

I suspect this was an optimization so that the following request potentially would not hit an EDNS related error on consecutive requests to the same resolver. However I believe it should be sticky on the channel if explicitly enabled (and if it causes problem, it should then be disabled when initialing the channel).

I'm not even sure if c-ares should make a retry without EDNS by default as the retry will often also fail, maybe it could be a flag to control this behavior (in many cases it would be an optimization to disable this behavior). There have been some new recommendations regarding EDNS failures https://dnsflagday.net

[coveralls]

Coverage decreased (-0.8%) to 88.643% when pulling d5656af on halon:edns_flag_unflagged into 602aaec on c-ares:master.

[bradh352]

As per https://tools.ietf.org/html/rfc6891#section-7 FORMERR would be required to be returned with no OPT record if EDNS is not supported (which presumably should trigger a re-try without EDNS from the requestor). If an OPT record was returned with FORMERR though, its a hard failure.

I do see the c-ares is currently also triggering on NOTIMP and SERVFAIL, and also not looking for an OPT record, most likely because some old legacy completely non-compliant servers would return as such. (C-ares has been around quite a while).

I think the more proper fix is to follow the spec ... only look for FORMERR, then validate if an OPT record was present, if not, then disable EDNS on the entire channel and retry the query. If an OPT record was present, its a critical failure (no retry). With that logic, we've guaranteed that we know the server doesn't support EDNS so it makes sense to kill it on the channel.

Such a change, however, does mean we drop support for old non-compliant servers ... but maybe that's not a bad thing.

[eriklax]

I totally agree with that, I've updated the pull request accordingly.

[bradh352]

could you evaluate the failures in the EDNS retry test cases as reported by travis? I haven't looked at the test cases myself, its possible they're written in a way that doesn't properly test the new behavior.

[bradh352]

@eriklax ping