PowerDNS / pdns

PowerDNS Authoritative, PowerDNS Recursor, dnsdist
https://www.powerdns.com/
GNU General Public License v2.0
3.69k stars 906 forks source link

NSECx for apex is including the DNSKey type for zones without published keys #8921

Closed mind04 closed 4 years ago

mind04 commented 4 years ago

In a zone with active keys but without published keys the NSECx for the apex is including the DNSKEY type

Environment

Steps to reproduce

  1. pdnsutil secure-zone example.com
  2. pdnsutil unpublish-zone-key example.com 1
  3. dig DNSKEY example.com

Expected behaviour

A denial without DNSKEY at apex

Actual behaviour

; <<>> DiG 9.11.4-P2-RedHat-9.11.4-9.P2.el7 <<>> +dnssec +time=1 +tries=1 +norec -p5300 @127.0.0.1 dnskey example.com
; (1 server found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 58919
;; flags: qr aa; QUERY: 1, ANSWER: 0, AUTHORITY: 4, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags: do; udp: 1232
;; QUESTION SECTION:
;example.com.           IN  DNSKEY

;; AUTHORITY SECTION:
example.com.        86400   IN  SOA ns1.example.com. ahu.example.com. 2847484148 28800 7200 604800 86400
example.com.        86400   IN  RRSIG   SOA 13 2 100000 20200319000000 20200227000000 14848 example.com. PKGnKtz7VgV5519dTbNjVpcyB50JWAUm5LdfbKFXYuYf1JVBfWy0zdG2 IgCGAaTNMS5K5HxAnL4gcBFEpy6aWQ==
example.com.        86400   IN  NSEC    _imap._tcp.example.com. NS SOA MX RRSIG NSEC DNSKEY
example.com.        86400   IN  RRSIG   NSEC 13 2 86400 20200319000000 20200227000000 14848 example.com. 4k+5YYb7P1FkC872oQgpvrn6U96orS31Rpk6wgxkGJtviCdDnngpdwg7 2sjZnAuEluDdS5XXdrqSK+jSJMz8qA==

;; Query time: 1 msec
;; SERVER: 127.0.0.1#5300(127.0.0.1)
;; WHEN: Wed Mar 11 13:40:32 CET 2020
;; MSG SIZE  rcvd: 343

This issue is also present in the axfr output

Other information

It is unlikely this will result in operational issues since the zone is technically not signed when this is happening.

RobinGeuze commented 4 years ago

Ah so there is an issue introduced by my MR. Will take a look.

mnordhoff commented 4 years ago

...Did the patch really work?

https://builder.powerdns.com/#/builders/99/builds/1219

Mar 16 17:05:26 clover pdns_server[16331]: PowerDNS Authoritative Server 4.4.0-alpha0.89.master.g148a1b87a (C) 2001-2020 PowerDNS.COM BV
$ dig +dnssec +norecurse @ns10.mattnordhoffdns.org unpublished.mattnordhoffdns.work dnskey

; <<>> DiG 9.16.0 <<>> +dnssec +norecurse @ns10.mattnordhoffdns.org unpublished.mattnordhoffdns.work dnskey
; (2 servers found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 61919
;; flags: qr aa; QUERY: 1, ANSWER: 0, AUTHORITY: 4, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags: do; udp: 1232
;; QUESTION SECTION:
;unpublished.mattnordhoffdns.work. IN   DNSKEY

;; AUTHORITY SECTION:
unpublished.mattnordhoffdns.work. 3600 IN SOA   pdns0.mattnordhoff.net. rname.mn0.us. 2020031300 1800 900 2678400 3600
unpublished.mattnordhoffdns.work. 3600 IN RRSIG SOA 13 3 3600 20200326000000 20200305000000 4819 unpublished.mattnordhoffdns.work. JU9ViMj3YUDk6PjB9DFT1CN0JJVvvbVbh9TuKzppETC2wkTXtOwa8WiR RanDqQ2R1g/lK1ZTqc9oGwgNIWq/Hw==
unpublished.mattnordhoffdns.work. 3600 IN NSEC  unpublished.mattnordhoffdns.work. NS SOA RRSIG NSEC DNSKEY CDS CDNSKEY
unpublished.mattnordhoffdns.work. 3600 IN RRSIG NSEC 13 3 3600 20200326000000 20200305000000 4819 unpublished.mattnordhoffdns.work. Uz41XF1Pg9etKDIPMUYo/rCC8twhRTUQ9zBzmdiXT5nciFBXroeRghRa VyTR7g1tINqsr9r/MVKXhVxsVLbzsA==

;; Query time: 3 msec
;; SERVER: 2600:3c00::2:b422#53(2600:3c00::2:b422)
;; WHEN: Mon Mar 16 17:09:45 UTC 2020
;; MSG SIZE  rcvd: 443

I am confuse.

Habbie commented 4 years ago

Reopened for investigation. I've decided to not brown bag 4.3.0-RC2 over a small bug in a new feature.

mnordhoff commented 4 years ago

The code is checking if !keyset.empty(), but I think DNSSECKeeper::getKeys returns every key, whether it's published or not?

In which case it would be necessary to loop over keyset and check the published attributes of each DNSSECPrivateKey or KeyMetaData or whatever.

RobinGeuze commented 4 years ago

Yes indeed, I made a derp. I will create a fix and add some test case tomorrow for this.