Opened 8 years ago

Last modified 8 years ago

#6664 enhancement new

Implement dns.Record_DNSKEY record class

Reported by: Richard Wall Owned by: Richard Wall
Priority: normal Milestone: DNSSEC_Client
Component: names Keywords:
Cc: Branch: branches/dnskey-record-6664
branch-diff, diff-cov, branch-cov, buildbot
Author: rwall


Implement only the DNSKEY record type and tests.

Add an example DNSKEY record to source:trunk/doc/names/howto/listings/names/

Build upon one of the implementations attached to #4610 and #5454 or combine the two.

#5454 includes a new DNSKEY lookup method. That can be deferred to another ticket.

#4610 includes extra methods for verification of signatures. That can also be deferred to another ticket.


Change History (7)

comment:1 Changed 8 years ago by Richard Wall

Owner: set to Richard Wall
Status: newassigned

comment:2 Changed 8 years ago by Richard Wall

Author: rwall
Branch: branches/dnskey-record-6664

(In [39456]) Branching to 'dnskey-record-6664'

comment:3 Changed 8 years ago by Richard Wall

Keywords: review added
Owner: Richard Wall deleted
Status: assignednew

Ready for review in log:branches/dnskey-record-6664

  1. Implemented Record_DNSKEY
    1. Based on original patches from BobNovas and PhilMayers
    2. Added more tests
    3. Added more documentation
    4. Instead of a general flags argument, I added arguments for each of the current standard flags.
    5. Added base64 encoding of the key in repr
    6. This is just a container record. I have not implemented any of the verification / validation logic in this branch.
  1. Implemented lookupDNSKEY
    1. I had intended to do this in another branch
    2. But I wanted to see if the Record_DNSKEY worked and lookupDNSKEY wasn't much extra work.
  1. Added lookupDNSKEY to the example script to demonstrate client usage.
  1. Added DNSKEY record to to demonstrate authoritative server usage. (At this stage the user would have to write code to generate suitable keys etc)
  1. Thoughts:
    1. Need to think about how these records are going to be used.
    2. How to and represent keys.
    3. Are there things I can borrow from t.i.ssl.
    4. I'm thinking about something like Record_DNSKEY.fromKey(ssl.KeyPair(...))
    5. Can this be deferred to a separate ticket.
  1. Build Results:
    2. Twisted Checker Warnings
      1. "empty docstring" warnings - the docstrings are inherited from IResolver and IEncodable interfaces.
      2. "missing return" warnings - tests don't need to document returned deferreds.
      3. "Comments should begin with one whitespace" - its getting confused by the hash part of a URL link.
      4. It also complains about my DNSKEY_TEST_DATA class name. Coding standard doesn't say anything about it, but the examples in use this style.

comment:4 Changed 8 years ago by Richard Wall

See also #6797 which is based on this branch.

comment:5 Changed 8 years ago by Richard Wall

Milestone: DNSSEC: Security Aware, Validating Client

comment:6 Changed 8 years ago by Richard Wall

See also #6664 which is based on this branch.

comment:7 Changed 8 years ago by Tom Prince

Keywords: review removed
Owner: set to Richard Wall
  1. Consider using twisted.python.constants.Flags instead of a bunch of booleans.
  2. Is there any reason for making the protocol public? Since it must always be 3, shouldn't it not be something that you can change?
  3. test_iencodableInterface: It would be better if this only made one assertion. One way to do this, would to loop over all the types, add the failures to a list, and self.failing if there are any.
    • I guess this is true of all the tests there.
  4. __repr__: I would think that the key data should be a string (and probably b). The data is bytes, so it seems reasonable to represent it that way.
  5. DNSKEY_TEST_DATA doesn't follow the naming standard.
    • I might be inclined s/Dns/DNS/ in some of the names.
  6. test_revokedDefaultAttribute: The bit of the docstring describing revoked seems like it belongs elsewhere.
  7. The key should definately be of some structured type. I'm not sure if there is a way to define things so that this can land before there exists a structured type. Perhaps you could define a private type with no public interface, as a temporary measure. 8.

XXX: enforce the following "A DNSKEY RR with the SEP set and the Zone Key flag not set MUST NOT be used to verify RRSIGs that cover RRsets."

This seems like it belongs in a different ticket.


shouldn't they be immutable?

Yes. But the interface the implement unfortunately doesn't support that (namely .decode, which ideally would have been a class method).

Note: See TracTickets for help on using tickets.