Add support for OPENPGPKEY RRTYPE. #2354

jhcloos · 2015-03-13T22:47:09Z

OPENPGPKEY is defined in draft-ietf-dane-openpgpkey.

The IANA has assigned RRTYPE 61.

Its content is a single binary blob, its presentation is a single hex blob.

Thanks to Aki Tuomi, JP Mens and Peter van Dijk for bug reports and insights.

Signed-off-by: James Cloos cloos@jhcloos.com

OPENPGPKEY is defined in draft-ietf-dane-openpgpkey. The IANA has assigned RRTYPE 61. Its content is a single binary blob, its presentation is a single hex blob. Thanks to Aki Tuomi, JP Mens and Peter van Dijk for bug reports and insights. Signed-off-by: James Cloos <cloos@jhcloos.com>

Habbie · 2015-03-16T12:47:08Z

Doc says 'since 3.0', which should probably be 3.5.0

Habbie · 2015-03-16T12:47:49Z

d_cert, is that the best name for the var? The draft says that the field holds a key.

Habbie · 2015-03-16T12:48:31Z

And finally, did you actually test this?

Specify an accurate version in the ‘Since’ tag. Signed-off-by: James Cloos <cloos@jhcloos.com>

jhcloos · 2015-03-16T14:04:29Z

Docs still say 'since 3.0', which should probably be 3.5.0

Yes. I wasn't sure what the next version would be, exactly, and forgot
about that by the time I provided the pull request. ☹

Updated to 3.5.0 per that suggestion.

-JimC

James Cloos cloos@jhcloos.com OpenPGP: 0x997A9F17ED7DAEA6

Signed-off-by: James Cloos <cloos@jhcloos.com>

jhcloos · 2015-03-16T17:56:43Z

"PD" == Peter van Dijk notifications@github.com writes:

PD> d_cert, is that the best name for the var?

Do you perfer something like d_keyring?

The draft says:

,----
| 2.1. The OPENPGPKEY RDATA component
|
| The RDATA (or RHS) of an OPENPGPKEY Resource Record contains a single
| value consisting of a [RFC4880] formatted OpenPGP public keyring.
`----

-JimC

James Cloos cloos@jhcloos.com OpenPGP: 0x997A9F17ED7DAEA6

jhcloos · 2015-03-16T23:01:54Z

"PvD" == Peter van Dijk notifications@github.com writes:

PvD> And finally, did you actually test this?

Yes. I copied my existing db over to a test box, replaced my TYPE61 row
with the equiv OPENPGPKEY row (removing the '# 4042 ' from content and
changing the type) and dig(1) had no problem getting that record.

-JimC

James Cloos cloos@jhcloos.com OpenPGP: 0x997A9F17ED7DAEA6

Habbie · 2015-03-17T07:20:45Z

Yes, d_keyring sounds fine!

If you change to d_keyring, and Travis passes again, I'm happy to merge this.

cmouse · 2015-03-17T07:22:04Z

Please provide test-dnsrecords_cc.cc test suite! =)

Habbie · 2015-03-17T07:22:16Z

@cmouse that is actually a fair point!

jhcloos · 2015-03-17T12:48:10Z

"AT" == Aki Tuomi notifications@github.com writes:

AT> Please provide test-dnsrecords_cc.cc test suite! =)

That will take some time to generate. I was hoping it could be done in
two pull requests instead of together.

I'll need to generate some keys of various sizes for the tests.

Are there any objections to using names @powerdns.org for them?

-JimC

James Cloos cloos@jhcloos.com OpenPGP: 0x997A9F17ED7DAEA6

cmouse · 2015-03-17T12:49:34Z

Well, sure, but the test suite in test-dnsrecords_cc.cc only tests for parseability, not key validity. Anything goes as long as it's syntaxically valid.

If you want to make regression-tests to ensure that keys are really valid and work, that's another thing.

Habbie · 2015-03-17T12:50:37Z

I would like 'parses fine' for test-dnsrecords_cc.cc in this pull if we can. I would love to have a deeper test in another pull later!

Signed-off-by: James Cloos <cloos@jhcloos.com>

jhcloos · 2015-03-17T17:31:01Z

So is something like:

 (CASE_S(QType::OPENPGPKEY, "ABCDEF", "\xAB\xCD\xEF",false))

enough, then?

If so, I pushed one like that but with a real keyring (albeit w/o xtra
sigs) (2264 octets of rdata). Is there need for a larger one, too?

(What does the bool in signify?)

-JimC

James Cloos cloos@jhcloos.com OpenPGP: 0x997A9F17ED7DAEA6

Habbie · 2015-03-17T17:50:40Z

When the bool is set to 'true', this signifies that we know this test is currently failing, and tolerate that with a warning. Please keep yours on false if you can. This test looks fine; perhaps also add one with embedded whitespace (assuming the draft allows this; it should! if not, file a bug with the draft!).

With those two tests in place, I think I'm happy to merge this!

Habbie · 2015-03-17T18:56:37Z

Build failed. Did you test this one locally, including the test? (it involves passing --enable-unit-tests to configure and running make check or ./testrunner in the pdns dir)

jhcloos · 2015-03-17T19:06:27Z

I just noticed that I misread one aspect of the draft.

The presentation is base64, not hex.

(I must have gotten hex in my head from the raw TYPE61 format.)

I'll have to re-do it again.

-JimC

James Cloos cloos@jhcloos.com OpenPGP: 0x997A9F17ED7DAEA6

Also add a test which has whitespace within the tested presentation. Signed-off-by: James Cloos <cloos@jhcloos.com>

jhcloos · 2015-03-17T23:07:26Z

One of those failures is due to the spaces.

Does conv.xfrHexBlob() ignore spaces when converting the b64
presentation to binary for on the wire?

For the test with spaces in the base64, does it require CASE_L, with and
w/o the spaces?

I presume the other fail is that I put in incomplete wire data. I take
it that the initial lenght octets are required? The TLSA tests do not
have any extra octets, which is why I didn't add any. And neither do
the DHCID tests, which look also to be a single blob with base64
presentation.... But the base64 and \xXX string match in the no-space
openpgpkey test, so don't understand the failure....

-JimC

James Cloos cloos@jhcloos.com OpenPGP: 0x997A9F17ED7DAEA6

Lennie · 2015-03-23T16:23:27Z

It seems to me support for OPENPGPKEY support would be incomplete with this patch. It is missing a critical part of the implementation of OPENPGPKEY record support:
https://tools.ietf.org/html/draft-ietf-dane-openpgpkey-02#section-4

Because of how easily the size of this record type can grow it's considered a possible of DDOS amplification attacks. It is strongly recommended to never respond to a request from an unverified source.

In practice this means for UDP:
"answer the query with an empty Answer Section and the truncation flag set ("TC=1)."

Which means that way the data is always send over TCP.

( I know about this because I was listening to the audio recording of the ICANN52 DNSSEC workshop and Paul Wouters mentioned it: http://singapore52.icann.org/en/schedule/wed-dnssec at 3 hours, 19 min. and 26 sec. )

pieterlexis · 2015-03-23T16:26:26Z

@Lennie this section is a recommendation, by no means a mandatory part of the specification.

Lennie · 2015-03-23T16:40:43Z

Pieter I hope you can see how it is important to still add this to be a good netizen.

How complicated would it be (I don't remember enough of the code to know this from the top of my head) ?

cmouse · 2015-03-23T16:51:20Z

It is not trivial. Basically you're adding a special case that sets TC=1 when qtype matches to special list. You could do this with Lua filter if this is important now.

cmouse · 2015-03-29T10:03:50Z

docs/markdown/types.md

@@ -93,7 +96,7 @@ Since 2.9.21. The SSHFP record type, used for storing Secure Shell (SSH) fingerp
 SRV records can be used to encode the location and port of services on a domain name. When encoding, the priority field is used to encode the priority. For example, '\_ldap.\_tcp.dc.\_msdcs.conaxis.ch SRV 0 100 389 mars.conaxis.ch' would be encoded with 0 in the priority field and '100 389 mars.conaxis.ch' in the content field.

 ## TLSA
-Since 3.0. The TLSA record, specified in [RFC 6698](http://tools.ietf.org/html/rfc6698), are used to bind SSL/TLS certificate to named hosts and ports.
+Since 3.0. The TLSA records, specified in [RFC 6698](http://tools.ietf.org/html/rfc6698), are used to bind SSL/TLS certificate to named hosts and ports.


This change is not related to your feature, please rever it.

Habbie · 2015-05-06T15:09:05Z

Please move the doc typo commit (in the TLSA section) to a separate commit
Please change the target version to 4.0.0 (sorry about that)
Please then make sure Travis passes

Note that the OPENPGPKEY draft appears to have stalled, and is generally undeployable in its current form - we are not yet sure we want to merge this if the draft has the potential to just expire.

Thank you!

pieterlexis · 2015-07-10T08:25:11Z

closing, will create a ticket

Update docs for new OPENPGPKEY RR.

d449b10

Specify an accurate version in the ‘Since’ tag. Signed-off-by: James Cloos <cloos@jhcloos.com>

Use the variable name d_keyring for the OPENPGPKEY RDATA.

3fd06ce

Signed-off-by: James Cloos <cloos@jhcloos.com>

Add regression test for OPENPGPKEY.

003c18d

Signed-off-by: James Cloos <cloos@jhcloos.com>

OPENPGPKEY RData presentaion is Base64.

ad24586

Also add a test which has whitespace within the tested presentation. Signed-off-by: James Cloos <cloos@jhcloos.com>

cmouse reviewed Mar 29, 2015
View reviewed changes

pieterlexis closed this Jul 10, 2015

pieterlexis mentioned this pull request Jul 10, 2015

Support OPENPGPKEY RRType #2636

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for OPENPGPKEY RRTYPE. #2354

Add support for OPENPGPKEY RRTYPE. #2354

jhcloos commented Mar 13, 2015

Habbie commented Mar 16, 2015

Habbie commented Mar 16, 2015

Habbie commented Mar 16, 2015

jhcloos commented Mar 16, 2015

jhcloos commented Mar 16, 2015

jhcloos commented Mar 16, 2015

Habbie commented Mar 17, 2015

cmouse commented Mar 17, 2015

Habbie commented Mar 17, 2015

jhcloos commented Mar 17, 2015

cmouse commented Mar 17, 2015

Habbie commented Mar 17, 2015

jhcloos commented Mar 17, 2015

Habbie commented Mar 17, 2015

Habbie commented Mar 17, 2015

jhcloos commented Mar 17, 2015

jhcloos commented Mar 17, 2015

Lennie commented Mar 23, 2015

pieterlexis commented Mar 23, 2015

Lennie commented Mar 23, 2015

cmouse commented Mar 23, 2015

cmouse Mar 29, 2015

Habbie commented May 6, 2015

pieterlexis commented Jul 10, 2015

Add support for OPENPGPKEY RRTYPE. #2354

Add support for OPENPGPKEY RRTYPE. #2354

Conversation

jhcloos commented Mar 13, 2015

Habbie commented Mar 16, 2015

Habbie commented Mar 16, 2015

Habbie commented Mar 16, 2015

jhcloos commented Mar 16, 2015

-JimC

jhcloos commented Mar 16, 2015

-JimC

jhcloos commented Mar 16, 2015

-JimC

Habbie commented Mar 17, 2015

cmouse commented Mar 17, 2015

Habbie commented Mar 17, 2015

jhcloos commented Mar 17, 2015

-JimC

cmouse commented Mar 17, 2015

Habbie commented Mar 17, 2015

jhcloos commented Mar 17, 2015

-JimC

Habbie commented Mar 17, 2015

Habbie commented Mar 17, 2015

jhcloos commented Mar 17, 2015

-JimC

jhcloos commented Mar 17, 2015

-JimC

Lennie commented Mar 23, 2015

pieterlexis commented Mar 23, 2015

Lennie commented Mar 23, 2015

cmouse commented Mar 23, 2015

cmouse Mar 29, 2015

Choose a reason for hiding this comment

Habbie commented May 6, 2015

pieterlexis commented Jul 10, 2015