Implement dns.Record_CERT record class

[Aaron Spike]

Implement the DNS CERT resource record following cues in #6664 by rwall.

​http://tools.ietf.org/html/rfc4398

The goal of this effort is to provide a DNS server implementation that is compatible with the needs of the Direct Project.

​http://directproject.org/

[Aaron Spike]

Patch implementing DNS CERT resource record

[Aaron Spike]

version of patch with tests

[Aaron Spike]

Version of patch with tests. Sorry about the last file (which was a copy of the dnskey tests).

[Richard Wall]

(In [40424]) Branching to 'cert-record-6797'

[Richard Wall]

(In [40425]) apply twisted_names_trunk_CERT.2.diff from acspike. Refs #6797.

[Richard Wall]

(In [40426]) Branching to 'cert-record-6797-2'

[Richard Wall]

(In [40427]) Really apply twisted_names_trunk_CERT.2.diff from acspike. Refs #6797.

[Richard Wall]

Thanks acspike!

The patch looks very good. Here are some notes and points.

Notes:

• I applied your patch to a new branch started some builds.
  • log:branches/cert-record-6797-2 (NB: I messed up the first branch, so we're using "-2" in the branch name)
  • ​https://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/cert-record-6797-2

• The coverage seems complete

Points:

1. twistedchecker warnings (​https://buildbot.twistedmatrix.com/builders/twistedchecker/builds/1439/steps/run-twistedchecker/logs/new%20twistedchecker%20errors)
   1. Most of the warnings are spurious, but there are one or two that need fixing
   2. "W9010:1607,0: Trailing whitespace found in the end of line"
   3. "W9012:1905,0: Expected 2 blank lines, found 3"
   4. "W9013:2676,0: Expected 3 blank lines, found 2"

2. branches/cert-record-6797-2/twisted/names/dns.py
   1. _base64Format
      1. Missing fullstop on first docstring line.
   2. Record_CERT
      1. Typo "recommende"
   3. hash
      1. Typo "A has allowing"

3. branches/cert-record-6797-2/twisted/names/test/test_names.py
   1. testCERT
      1. New test methods should use the "test_CERT" naming convention.

4. branches/cert-record-6797-2/twisted/names/test/test_dns.py
   1. test_cert
      1. docstring should be wrapped at 80 columns

5. I guess that some of these points will also apply to my DNSKEY branch (#6664) so I should probably go back and fix them there too.

6. I haven't properly read RFC4398 so treat this as a cosmetic review.

7. I don't yet understand how this fits in with DANE (​http://tools.ietf.org/html/rfc6698) and TLSA -- have you got any good comparisons / explanations. I found this summary:

1. ​https://bugzilla.mozilla.org/show_bug.cgi?id=589537#c16

8. I'm interested to know how you intend to create the CERT records. Do you think Record_CERT is usable as is, or do we need to create some sort of factory method which accepts a PEM certificate file or an OpenSSL.crypto.X509 instance. eg
   1. https://twistedmatrix.com/trac/browser/branches/ssl-endpoint-description-docs-6744/doc/core/howto/listings/ssl/make_certificate.py?rev=39996

9. I found this page from your project ​http://wiki.directproject.org/DNS+Configuration+Guide but couldn't find any live examples of CERT records. I wanted to see how dig formats CERT records and eventually found this:

   $ dig @8.8.8.8 danm.prime.gushi.org CERT +tcp +short
   PGP 0 0 mQGiBDnY2vERBAD3cOxqoAYHYzS+xttvuyN9wZS8CrgwLIlT8Ewo/CCF I11PEO+gJyNPvWPRQsyt1SE60reaIsie2bQTg3DYIg0PmH+ZOlNkpKes PULzdlw4Rx3dD/M3Lkrm977h4Y70ZKC+tbvoYKCCOIkUVevny1PVZ+mB 94rb0mMgawSTrct03QCg/w6aHNJFQV7O9ZQ1Fir85M3RS8cEAOo4/1AS Vudz3qKZQEhU2Z9O2ydXqpEanHfGirjWYi5RelVsQ9IfBSPFaPAWzQ24 nvQ18NU7TgdDQhP4meZXiVXcLBR5Mee2kByf2KAnBUF9aah5s8wZbSrC 6u8xEZLuiauvWmCUIWe0Ylc1/L37XeDjrBI2pT+k183X119d6Fr1BACG fZVGsot5rxBUEFPPSrBqYXG/0hRYv9Eq8a4rJAHK2IUWYfivZgL4DtrJ nHlha+H5EPQVYkIAN3nGjXoHmosY+J3Sk+GyR+dCBHEwCkoHMKph3igc zCEfxAWgqKeYd5mf+QQq2JKrkn2jceiIO7s3CrepeEFAjDSGuxhZjPJV m7QoRGFuaWVsIFAuIE1haG9uZXkgPGRhbm1AcHJpbWUuZ3VzaGkub3Jn PohiBBARAgAiAhkBBQJMGHtPBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIX gAAKCRD7vlowYkuySf9RAKD2qK8LjgLZHzMsVZgJPhtS1+JMfACgmJgQ x0bd6kMaMmipvzLzF6gMPaO0MkRhbmllbCBNYWhvbmV5IChTZWNvbmRh cnkgRW1haWwpIDxndXNoaUBndXNoaS5vcmc+iGIEExECACICGyMCHgEC F4AFAkwYe1sGCwkIBwMCBhUIAgkKCwQWAgMBAAoJEPu+WjBiS7JJcFgA nR2PHl/bXj8uinpYufdKCd28ANzPAJwIXJF8osm+EhuGNPq2Ugbk94jq SrkCDQQ52Nr0EAgA9kJXtwh/CBdyorrWqULzBej5UxE5T7bxbrlLOCDa AadWoxTpj0BV89AHxstDqZSt90xkhkn4DIO9ZekX1KHTUPj1WV/cdlJP PT2N286Z4VeSWc39uK50T8X8dryDxUcwYc58yWb/Ffm7/ZFexwGq01ue jaClcjrUGvC/RgBYK+X0iP1YTknbzSC0neSRBzZrM2w4DUUdD3yIsxx8 Wy2O9vPJI8BD8KVbGI2Ou1WMuF040zT9fBdXQ6MdGGzeMyEstSr/POGx KUAYEY18hKcKctaGxAMZyAcpesqVDNmWn6vQClCbAkbTCD1mpF1Bn5x8 vYlLIhkmuquiXsNV6TILOwACAgf/Xho8xdoA4elOyO9sf+m0nZRMcdi7 yBfdjmSnNEueSDkuC4M7Ox235tWji+KCbe8AYPeMZBeHHq8c+8vEfSfp NxjZdeCjo22GjAIda3cXQM4pGDB9adYUu/BjLcMZMuoxOXp/OwRhjJp2 wvOCZccDfjA+3YrvA9BpII4/6cTqd9g+YxHtNsAT1YxU6RSyY6RZ4i1G OgKIUQxHUrmcFj7qClVoaXlpGrDZ+aoMBsg0RG16cj7FNNgZMBOCYhrP iTDHTp/SjIeXcYrsLDDPYB4kGUt5kjQQSj1iOWV7HUrVRb2qY39hVBQ1 y1G00Tj79V4an9Lu2GDkRZ1nlbb8yiMVWohGBBgRAgAGBQI52Nr0AAoJ EPu+WjBiS7JJQVoAnje8/cZOdsv2qGgrheoWG9Hft5PfAKDEcbx7lyNT XNhV2P8euT8B4lG2mA==
   

   1. I note that dig presents the "Mnemonic" certificate type. We could do something similar in our repr if we defined and used twisted.python.constants for the various certificate types and algorithms. I might do the same in #6664 for DNSKEY.

10. Missing news file
    1. https://twistedmatrix.com/trac/wiki/ReviewProcess#Newsfiles

Anyway, that's all for now. Please address or answer the numbered points above and submit another patch against the new branch.

I'm waiting for the DNSKEY branch #6664 to be reviewed and I expect that the feedback I get there will also be applicable here.

Also note that twisted branches are mirrored on github at:

• ​https://github.com/twisted/twisted/tree/cert-record-6797-2

...which might be useful if you prefer git.

Thanks again.

-RichardW.

[Aaron Spike]

Thanks for the thorough review. I will address the all of the needed code changes in the branch.

#7 I'm not familiar with DANE and so I cannot comment on the relationship.

#8 I wondered if generating the record from a PEM would be the best solution. (Or at the very least generating the keyTag.) I decided that I'm not familiar enough with the standards to avoid limiting the usefulness of the implementation by accident. (Sysadmins have to generate this configuration for bind in some way too.) So I figured the most compatible and flexible implementation would allow all of the properties to be directly specified. Here is a tool (implemented using dnspython) which generates the bind configuration: ​http://www.videntity.com/2013/08/how-to-serve-public-certificates-in-bind-for-the-direct-project/

#9 I'm just a concerned citizen, not a representative of the Direct Project. (This applies to my answer to #7 equally.) So I can't offer any explanation on why this particular record type was chosen, but it is being used in the wild. Here are a few examples: dig -t CERT healthvault.direct.healthvault.com dig -t CERT sharelinkexchange.com

#9.1 I wondered what about best practices for implementing those enumerations in twisted.

[Aaron Spike]

Sorry for the wait. I have addressed the cosmetic issues in the following change set: ​https://github.com/acspike/twisted/compare/cert-record-6797-2