dnsmasq: Add ipset support which adds resolved IP addresses of domains automatically to an external pf alias

[Monviech]

Implements the following:

--ipset=/<domain>[/<domain>...]/<ipset>[,<ipset>...]
Places the resolved IP addresses of queries for one or more domains in the specified Netfilter IP set.

There have been quite some user reports who use this successfully in the past on github and the forum and it works with pf correctly.

This is the set I tested it with:

root@opn03:/usr/local/etc/dnsmasq.conf.d # cat ipset.conf
ipset=/google.com/dnsmasq_google
ipset=/google.de/dnsmasq_google

Result of alias:

root@opn03:/usr/local/etc/dnsmasq.conf.d # pfctl -t dnsmasq_google -T show
   142.251.36.174
   142.251.36.195

[fichtner]

Interesting! Isn't this emulating what the hosts aliases do?

Also I want to mention https://forum.opnsense.org/index.php?topic=47582.0 as a side note, where "alias" is used as terminology already so maybe that should be renamed while at it to avoid more confusion.

[Monviech]

@fichtner As far as I read in the past it can be used to create wildcard host entries, which you cannot do with a normal host alias.

So e.g. you could add *.cloudflare.com as ipset and it would track all IP addresses that clients request of subdomains under cloudflare.com.

https://github.com/opnsense/core/issues/4145

[fichtner]

Ah ok nice

[swhite2]

~~Has https://github.com/opnsense/core/issues/4145#issuecomment-1209391936 been considered?~~

~~> An apparent limitation of this dnsmasq functionality is that the TTL for DNS records is not considered, and IPs added to the set remain in the set forever. Used with a CDN service, the table may become huge. I'd guess it may have a negative impact on performance at some point depending on hardware, the number of records in the table, and the frequency and number of requests made by clients.~~

Edit: mentioned in the description

[Monviech]

@swhite2 - @AdSchellevis told me he wants to implement:

"pfctl -t xxx -T expire 86400"

This could prevent ipsets that are note quite as selective to grow indefinitely.

Though the usecase for this alias type should be more selective, as its for a very specific usecase.

[swhite2]

@swhite2 - @AdSchellevis told me he wants to implement:

"pfctl -t xxx -T expire 86400"

This could prevent ipsets that are note quite as selective to grow indefinitely.

Though the usecase for this alias type should be more selective, as its for a very specific usecase.

https://github.com/opnsense/core/issues/4145#issuecomment-1209648932 mentions:

While pfctl can remove old entries (-T expire 86400 for over 24 hours old), these are not safe to remove. These "old" entries may have been recently served by dnsmasq even seconds ago. After dnsmasq ipset adds an IP address, the table timestamp will not be updated again on subsequent queries.

I might be interpreting this incorrectly, but I assume dnsmasq fully backs off on responsibility for the entry in the table after insertion?

[AdSchellevis]

ok, funny, apparently Unbound's ipset option also supports pf since last year (https://github.com/NLnetLabs/unbound/blob/master/doc/Changelog , https://github.com/NLnetLabs/unbound/blob/master/doc/README.ipset.md)

[ItsHarper]

I would recommend testing this feature with the latest unreleased Dnsmasq commits. 98189ff988 seems like a significant change to how this feature works, and if I'm right about the fact that it breaks building on FreeBSD (see my forum post ), it clearly wasn't implemented with much thought to how it will work on FreeBSD, much less has been tested there.

If this feature has been broken upstream, it seems much better to get ahead of that and make sure it's fixed before the next Dnsmasq release.

I think fixing the build should be as simple as restoring the deleted preprocessor checks, so testing this shouldn't be too hard.

[ItsHarper]

It's interesting that dnsmasq's add_to_ipset() function has a parameter to make it remove entries rather than add them, but it's always set to 0. I wonder how much work it would be to make it auto-remove entries for expired leases.

[ItsHarper]

I wonder how much work it would be to make it auto-remove entries for expired leases.

I actually think it should be pretty straightforward. See my notes in this commit: https://github.com/ItsHarper/dnsmasq/commit/00cafb53f799d4943b3495ee95b90b5ccb56c727

I think it might actually be a mistake to add this feature without the auto-removal support. If auto-removal were to be implemented, I think it would become pretty important that Dnsmasq was the only modifier of any Aliases that it touches (because otherwise it might remove things it shouldn't), and that those Aliases had names prefixed with dnsmasq- or something. That way, if a user migrates from Dnsmasq to another DHCP server like Kea, it will become clear that their firewall rules are referencing something that only works in the old setup.

This seems like a case where it's way better to wait to ship until you can ship an actually coherent solution (I'm aware this PR is just a draft and not about to ship, I'm just sharing my thoughts).

[Monviech]

For the upstream related issued, I'll leave these up to @fichtner to sight as he has way more experience than me with them. Thank you @ItsHarper for looking into it so far.

[Monviech]

ok, funny, apparently Unbound's ipset option also supports pf since last year (https://github.com/NLnetLabs/unbound/blob/master/doc/Changelog , https://github.com/NLnetLabs/unbound/blob/master/doc/README.ipset.md)

@AdSchellevis

https://github.com/opnsense/tools/pull/481 https://github.com/opnsense/ports/pull/231

The build works for me so I gonna try an implementation there too so dnsmasq and unbound both have ipset support.

[Monviech]

@fichtner This one would be ready for master. I rebased it without the Unbound commits.