Opened 13 months ago

Last modified 9 months ago

#6664 enhancement new

Implement dns.Record_DNSKEY record class

Reported by: rwall Owned by: rwall
Priority: normal Milestone: DNSSEC_Client
Component: names Keywords:
Cc: Branch: branches/dnskey-record-6664
(diff, github, buildbot, log)
Author: rwall Launchpad Bug:

Description

Implement only the DNSKEY record type and tests.

Add an example DNSKEY record to source:trunk/doc/names/howto/listings/names/example-domain.com

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.

See:

Change History (7)

comment:1 Changed 13 months ago by rwall

  • Owner set to rwall
  • Status changed from new to assigned

comment:2 Changed 13 months ago by rwall

  • Author set to rwall
  • Branch set to branches/dnskey-record-6664

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

comment:3 Changed 13 months ago by rwall

  • Keywords review added
  • Owner rwall deleted
  • Status changed from assigned to new

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 testdns.py example script to demonstrate client usage.
  1. Added DNSKEY record to example-domain.com 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:
    1. http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/dnskey-record-6664
    2. Twisted Checker Warnings
      1. "empty docstring" warnings - the docstrings are inherited from IResolver and IEncodable interfaces. https://bugs.launchpad.net/twistedchecker/+bug/1132540
      2. "missing return" warnings - tests don't need to document returned deferreds. https://bugs.launchpad.net/twistedchecker/+bug/1094942
      3. "Comments should begin with one whitespace" - its getting confused by the hash part of a URL link. https://bugs.launchpad.net/twistedchecker/+bug/1186378
      4. It also complains about my DNSKEY_TEST_DATA class name. Coding standard doesn't say anything about it, but the examples in https://twistedmatrix.com/documents/current/core/howto/constants.html use this style.

comment:4 Changed 10 months ago by rwall

See also #6797 which is based on this branch.

comment:5 Changed 10 months ago by rwall

  • Milestone set to DNSSEC: Security Aware, Validating Client

comment:6 Changed 9 months ago by rwall

See also #6664 which is based on this branch.

comment:7 Changed 9 months ago by tom.prince

  • Keywords review removed
  • Owner set to rwall
  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.

8.

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.