Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XFR clients have to be able to query SOA at apex #315

Merged
merged 1 commit into from Feb 12, 2024

Conversation

wtoorop
Copy link
Member

@wtoorop wtoorop commented Jan 29, 2024

Clients that are allowed to transfer a zone because they match a provide-xfr acl, should also be able to query for the SOA at the apex (because such a client does that query before it starts the actual transfer).
This PR allows transfer clients to do a SOA query at the apex, even if queries are blocked for the zone with the allow-query option.

@k0ekk0ek
Copy link
Contributor

Why solve the problem this way? I mean, the logic is correct IMHO, but wouldn't it be easier to just add the address to the allow_query list when configuring? If the client queries for AXFR or IXFR it gets the entire zone anyway(?)

@wtoorop
Copy link
Member Author

wtoorop commented Jan 30, 2024

Why solve the problem this way? I mean, the logic is correct IMHO, but wouldn't it be easier to just add the address to the allow_query list when configuring? If the client queries for AXFR or IXFR it gets the entire zone anyway(?)

Part of it is convenience (or user friendliness). Not everybody may know a SOA query is needed for a transfer (or may have forgotten about it or not aware of it while configuring). It is also convenient that the TSIG requirements are copied from provide-xfr.
There is also a privacy enhancement when transfer is only allowed over TLS. An on path eavesdropper can otherwise still sniff the entire zone with spoofed UDP queries. Also, the TCP handshake (in the actual transfer) provides stronger guarantees that the sender is genuine.

@k0ekk0ek
Copy link
Contributor

I think there's some miscommunication? I meant: automatically add it to the allow_query acl when parsing the configuration file.

@wtoorop
Copy link
Member Author

wtoorop commented Jan 30, 2024

I think there's some miscommunication? I meant: automatically add it to the allow_query acl when parsing the configuration file.

Ah, I thought that you meant that the user could configure it herself.

My arguments w.r.t. privacy still holds though. We have an extra restriction to allow only TYPE_SOA at the apex queries with the provide-xfr acls, this restriction would not be there if the rules would simply be copied. The extra restriction prevents an on path eavesdropper to see the whole zone by sending queries wit spoofed source IP.

I also think the specific debugging message about the match mentioning provide-xfr has value.

@k0ekk0ek
Copy link
Contributor

My main concern is that it's executed in the hotpath and the acls are iterated in linear fashion(?) And if TSIG is not required the same party could simple send an AXFR query, which would be much easier than trying to traverse the entire zone with normal queries and guessing?

@wtoorop
Copy link
Member Author

wtoorop commented Jan 30, 2024

My main concern is that it's executed in the hotpath and the acls are iterated in linear fashion(?) And if TSIG is not required the same party could simple send an AXFR query, which would be much easier than trying to traverse the entire zone with normal queries and guessing?

But that AXFR would be over TCP, yes? Then the TCP handshake prevents spoofing.

It's also not that much in the hotpath. The whole branch is only evaluated if(q->zone->opts && q->zone->opts->pattern && q->zone->opts->pattern->allow_query) on line 1330 and the provide-xfr acls only if the allow-query acls didn't allow the query in the first place.

Copy link
Member

@wcawijngaards wcawijngaards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good. The allowed SOA query should help with operations.

@wtoorop wtoorop merged commit f2abcca into master Feb 12, 2024
4 checks passed
wtoorop added a commit that referenced this pull request Feb 12, 2024
wtoorop added a commit that referenced this pull request Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants