PowerDNS / pdns

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

auth: Cannot enforce TSIG for DNS updates #12234

Closed cmouse closed 6 months ago

cmouse commented 1 year ago

Short description

If you want to allow TSIG verififed DNS updates, but cannot check or don't want to check sender IP, you configure PowerDNS to allow DNS updates from any source, and use per-zone TSIG keys to verify the updates.

This has a problem though that when you provision new zone, or have zones that you would prefer not to be updated, it is easy to forget to prevent DNS updates for these zones.

Few ways to fix this:

Environment

Steps to reproduce

  1. Set allow-dnsupdate-from = 0.0.0.0/0 ::/0
  2. Configure two zones
  3. Add tsig key to one of the zones
  4. You can now update the other zone too

Expected behaviour

I suppose the expected behaviour might be that the other zone wouldn't go wide open by accident.

Actual behaviour

You can update any zone with no TSIG key attached.

Other information

N/A

mind04 commented 1 year ago

Step 1, allow dnsupdates for the world....

The documentation is pretty clear about this: "allow-dnsupdate-from - A list of IP ranges that are allowed to perform updates on any domain."

With the default value of 'allow-dnsupdate-from' this will not bite you and you can control the updates any way you like with the ALLOW-DNSUPDATE-FROM and TSIG-ALLOW-DNSUPDATE metadata options

For example, 'ALLOW-DNSUPDATE-FROM = 0.0.0.0/0 ::/0' and 'TSIG-ALLOW-DNSUPDATE = tsigname' will allow updates with a valid tsig from everywhere.

hlindqvist commented 1 year ago

Indeed, allow-dnsupdate-from=0.0.0.0/0,::/0 seems like a configuration that really should be avoided.

I suppose it's also technically correct that there's nothing unexpected about the observed behavior for the perfectly informed user, it does indeed match the documentation, etc. However, as I see it, the ticket ultimately boils down to that putting the main focus on IP-based ACLs, in combination with the global IP-based ACL allow-dnsupdate-from encourages the creation of insecure configurations.

I think that a "TSIG-first" approach to authentication really would make for a much better baseline for security than the current "IP-first with optional TSIG if configured" approach. With that in mind, I think that something along the lines of the dnsupdate-require-tsig setting proposed in the ticket could provide a means for users to move themselves in a more secure direction without breaking existing configurations that rely on the old "IP-first" behavior. Having such a setting and encouraging dnsupdate-require-tsig=yes would make it easier for users to do things right (better yet, maybe ultimately using that mode by default at some stage). I would certainly appreciate being able to enforce that globally in the configuration, anyway.

Sidenote: I also think that allow-dnsupdate-from= would be a more reasonable default than the current 127.0.0.0/8,::1. In the current mode, the default of allowing anything over loopback to edit all zones without further authentication seems like a potential footgun in itself (for instance, if combined with a local proxy).

Habbie commented 1 year ago

I also think that allow-dnsupdate-from= would be a more reasonable default than the current 127.0.0.0/8,::1.

This makes a lot of sense to me.

Ramalama2 commented 9 months ago

This is an ultra stupid default Behaviour, i just camed across that either.

I need for example dnsupdates from everywhere, but secured with a TSIG Key.

This means, i have to configure pdns: allow-dnsupdate-from=0.0.0.0/0,::/0

So if the Zone contains an TSIG-ALLOW-DNSUPDATE, its fine, we can update then only with the key. But if the Zone doesnt contains an TSIG-ALLOW-DNSUPDATE entry, everyone can update everything!!!

Now people would say now, yeah add simply an TSIG-ALLOW-DNSUPDATE with a key to ALL Zones... This is not possible, since a lot of zones here are Automatically generated, with pdns-admin and so on...

So in the end this is extremely unsecure. An global dnsupdate-require-tsig Option would fix this.

Cheers

Ramalama2 commented 9 months ago

it comes worse: https://doc.powerdns.com/authoritative/dnsupdate.html#tsig-allow-dnsupdate If any TSIG keys are listed in a zone’s TSIG-ALLOW-DNSUPDATE metadata, one of them is required for updates. If ALLOW-DNSUPDATE-FROM is also set, both requirements need to be satisfied before an update will be accepted.

I read this as if TSIG-ALLOW-DNSUPDATE allows updating from everywhere without setting ALLOW-DNSUPDATE-FROM. Otherwise "one of them is required for updates" makes no sense.

hlindqvist commented 9 months ago

it comes worse: https://doc.powerdns.com/authoritative/dnsupdate.html#tsig-allow-dnsupdate If any TSIG keys are listed in a zone’s TSIG-ALLOW-DNSUPDATE metadata, one of them is required for updates. If ALLOW-DNSUPDATE-FROM is also set, both requirements need to be satisfied before an update will be accepted.

I read this as if TSIG-ALLOW-DNSUPDATE allows updating from everywhere without setting ALLOW-DNSUPDATE-FROM. Otherwise "one of them is required for updates" makes no sense.

"One of them" would appear to refer to the TSIG keys listed in TSIG-ALLOW-DNSUPDATE?

Ramalama2 commented 9 months ago

it comes worse: https://doc.powerdns.com/authoritative/dnsupdate.html#tsig-allow-dnsupdate If any TSIG keys are listed in a zone’s TSIG-ALLOW-DNSUPDATE metadata, one of them is required for updates. If ALLOW-DNSUPDATE-FROM is also set, both requirements need to be satisfied before an update will be accepted. I read this as if TSIG-ALLOW-DNSUPDATE allows updating from everywhere without setting ALLOW-DNSUPDATE-FROM. Otherwise "one of them is required for updates" makes no sense.

"One of them" would appear to refer to the TSIG keys listed in TSIG-ALLOW-DNSUPDATE?

you're right, after rereading (after your comment) it indeed meant for TSIG-ALLOW-DNSUPDATE keys.

i readed it somehow like this: "one of them is required for updates. If ALLOW-DNSUPDATE-FROM is also set, both requirements need to be satisfied"

I think im not alone, it's a bit confused written, maybe an <br> would help to separate the sentences.

However an dnsupdate-require-tsig option would be really appriciated, then i could open allow-dnsupdate-from for everything.

I have an small question tho, is it possible to set TSIG-ALLOW-DNSUPDATE=mykeyname in the config globally? This could make things here more secure, since my zones are automatically generated xD The docs mention only the metadata of the zone, but im not 100% sure, because the docs doesn't say that its not allowed in the config globally either.

Cheers :-)

hlindqvist commented 9 months ago

I have an small question tho, is it possible to set TSIG-ALLOW-DNSUPDATE=mykeyname in the config globally? This could make things here more secure, since my zones are automatically generated xD The docs mention only the metadata of the zone, but im not 100% sure, because the docs doesn't say that its not allowed in the config globally either.

There is no global option for allowing updates by TSIG at this point.

My understanding is that the current least error prone approach for TSIG-only update authentication would be something like this:

Or possibly doing something creative with lua-dnsupdate-policy-script

Ramalama2 commented 9 months ago

I have an small question tho, is it possible to set TSIG-ALLOW-DNSUPDATE=mykeyname in the config globally? This could make things here more secure, since my zones are automatically generated xD The docs mention only the metadata of the zone, but im not 100% sure, because the docs doesn't say that its not allowed in the config globally either.

There is no global option for allowing updates by TSIG at this point.

My understanding is that the current least error prone approach for TSIG-only update authentication would be something like this:

  • allow-dnsupdate-from= (setting the global ACL to empty)
  • TSIG-ALLOW-DNSUPDATE=mykeyname + ALLOW-DNSUPDATE-FROM=0.0.0.0/0,::/0 on a zone-by-zone basis. (And taking particular care to not for instance remove only the key later.)

Or possibly doing something creative with lua-dnsupdate-policy-script

I didn't thought about that, you're absolutely right again! i forgot or didn't thought about to set ALLOW-DNSUPDATE-FROM in the zone-meta itself...

Thats actually the solution to my whole Problem, thank you! i feel stupid now xD

Ramalama2 commented 9 months ago

@hlindqvist Sorry, this is completely offtopic, but i cannot find an answer to my last question. If i switch vlans, and both vlans have the same ddns domain, kea fails to update the new ip for the already existing record:

pdns-auth:
UPDATE (57350) from 172.17.0.10 for iot.fri.mydomain.int: Failed PreRequisites check for test-776.iot.fri.mydomain.int, returning Name Exists when it should not

kea:
kea-dhcp-ddns: ERROR DHCP_DDNS_FORWARD_REPLACE_REJECTED DNS Request ID 00000108904D2DED89B87535140C193EDD5062F2E7484AB721E14AFBED2BAEF493A987: Server, 172.17.1.10 port:5301, rejected a DNS update request to replace the address mapping for FQDN, test-776.iot.fri.mydomain.int., with an RCODE: 8

Is there a Zone-Meta Key to allow it on a Zone base? Sorry again, thx :-)

Habbie commented 9 months ago

Please use Discussions for questions