Create a new Message-like class that includes space for EDNS information

[exarkun]

EDNS extends DNS with a couple new statically defined fields and an arbitrary extension mechanism. This information is logically part of the DNS Message itself, but is represented on the network as a new record type.

We should introduce a new message class which exposes the additional information present in an OPT record. It should be safe to deserialize any DNS message using this class, even regular DNS messages (non-EDNS messages). When deserializing EDNS messages, though, the fields of the OPT record should be made available directly on the Message itself and the OPT record should not be present in the list of additional records (where it might end up in a naive implementation, since that's where it appears in the wire format).

It may make sense for some portion of the implementation of this new class to be shared with the implementation of the existing message class. It may make sense for these parts to be factored into a new shared base class. Some parts of the existing message class are undesirable (the redundancy between encode/decode and fromStr/toStr, the failure to use class methods where appropriate, the obscurely named attributes), thus I do not recommend having the new class subclass the old class directly.

[rwall]

(In [38441]) Branching to 'edns-message-5675'

[rwall]

Unfinished implementation of EDNSMessage which wraps and can be substituted for Message

[rwall]

Ready for review in attachment:edns-message-5675-1.patch

This isn't finished and I don't want a full code review. I'd just like some guidance / feedback on the following:

1. EDNSMessage
   1. attribute names / constructor args: I originally tried to improve the API by using more meaningful attribute names (eg responseCode instead of rCode)
      1. But this makes it difficult to drop EDNSMessage in place of Message (eg in dns.DNSMixin, server.DNSFactory) without making lots of changes elsewhere
      2. Is it worth choosing good names and providing some sort of adapter?
   2. decode / encode: I've left all the decode and encode logic in the original Message class. Does this make sense or should I / can I refactor Message and put the decode / encode stuff in a Mixin maybe?
   3. branch scope: I'm thinking that this branch should provide just enough to allow twistd dns to properly reject EDNS queries. That might sound strange, but it is described in  https://tools.ietf.org/html/rfc6891#section-7
      1. Currently I found that twistd dns returns your EDNS message to you in the NXDOMAIN case. This is because it doesn't reset the additional records list before sending the message back.
      2. So I thought it would be nice goal for this branch to fix that bug and simply send back the proper FORMERR rcode when an EDNS query is received - and defer proper EDNS responses to another branch.
2. Testing
   1. My tests have got quite convoluted.
   2. I'm trying to run as many tests as possible against both Message and EDNSMessage. Is that a worthwhile goal? Are there better methods than putting them in a mixin? The indirection of self.messageFactory and self.messageDecoder, makes the tests difficult to read...I think.
   3. I've also put the test data (bytes / dns wireformat and the corresponding Message constructor args) in a separate class. I thought it would be useful to be able to reuse that data (the byte string examples at least). Are there better ways of doing this?
   4. The existing Message tests make assertions about each of the Message attributes (eg assertEqual(m1.rCode, m2.rCode)) where as I'm just relying on the FancyEqMixin. Is that Ok?
   5. I've added FancyEqMixin to the original Message for ease of testing. Is that against the compatibility policy?

Anyway, that's all for now.

If anyone has time to comment I'd appreciate some feedback.

-RichardW.

[rwall]

(In [38826]) Branching to 'edns-message-5675-2'

[rwall]

Ready for review in log:branches/edns-message-5675-2

Following on from my comment:2, I've now trimmed and simplified things a bit.

1. EDNSMessage is now private so that I'm free to make changes in future EDNS related branches.
2. I've narrowed the EDNSMessage API down to just toStr and fromStr, plus the various attributes that were already present.
   1. There didn't seem any need to implement IEncodable and internally, DNSDatagramProtocol and DNSProtocol are using the to/fromStr API anyway.
3. I've simplified the tests. Breaking them into multiple test cases and where possible also running the tests against dns.Message.
4. Build Results
   1.  http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/edns-message-5675-2
   2. All the tests seem to pass and _EDNSMessage does seem to be a good drop in replacement for Message.

Next I'm going to actually start implementing more of the EDNS specific attributes.

But first I might go back to #5668 and re-implement OPTRecord as a wrapper around an RRHeader instance.

I'd still appreciate some feedback / guidance if anyone has time to glance at my branch.

-RichardW.

[rwall]

Start using _OPTHeader from source:branches/opt-record-5668-3