On Wed, Aug 09, 2023 at 09:17:36PM +0200, Thomas Haller wrote: > On Wed, 2023-08-09 at 12:20 +0200, Phil Sutter wrote: > > On Tue, Aug 08, 2023 at 10:05:19PM +0200, Thomas Haller wrote: > > > On Tue, 2023-08-08 at 15:24 +0200, Phil Sutter wrote: > > > > On Thu, Aug 03, 2023 at 09:35:16PM +0200, Thomas Haller wrote: > > > > > getaddrinfo() blocks while trying to resolve the name. Blocking > > > > > the > > > > > caller of the library is in many cases undesirable. Also, while > > > > > reconfiguring the firewall, it's not clear that resolving names > > > > > via > > > > > the network will work or makes sense. > > > > > > > > > > Add a new input flag NFT_CTX_INPUT_NO_DNS to opt-out from > > > > > getaddrinfo() > > > > > and only accept plain IP addresses. > > > > > > > > This sounds like user input validation via backend. Another way > > > > to > > > > solve > > > > the problem at hand is to not insert host names into the > > > > rules(et) > > > > fed > > > > into libnftables, right? > > > > > > Right. More generally, ensure not to pass any non-addresses in JSON > > > that would be resolved. > > > > Well, detecting if a string constitutes a valid IP address is rather > > trivial. In Python, there's even 'ipaddress' module for that job. > > firewalld messed it up, showing that it can happen. I'm just saying it's possible. From libnftables' PoV, hostname input is supported and works as expected. > > > Which requires that the user application is keenly aware, > > > understands > > > and validates the input data. For example, there couldn't be a > > > "expert > > > option" where the admin configures arbitrary JSON. > > > > Why is host resolution a problem in such scenario? The fact that > > using > > host names instead of IP addresses may result in significant delays > > due > > to the required DNS queries is pretty common knowledge among system > > administrators. > > > It seems prudent that libnftables provides a mode of operation so that > it doesn't block the calling application. Otherwise, it is a problem > for applications that care about that. Hmm. In that case, one might also have to take care of calls to getprotobyname() and maybe others (getaddrinfo()?). Depending on nsswitch.conf it may block, too, right? > > > And that the application doesn't make a mistake with that ([1]). > > > > > > [1] > > > https://github.com/firewalld/firewalld/commit/4db89e316f2d60f3cf856a7025a96a61e40b1e5a > > > > This is just a bug in firewall-cmd, missing to convert ranges into > > JSON > > format. I don't see the benefit for users which no longer may use > > host > > names in that spot. > > Which spot do you mean? /sbin/nft is not affected, unless it opts-in to > the new flag. firewalld never supported hostnames at that spot anyway > (or does it?). I'm pretty sure it does, albeit maybe not officially. Cheers, Phil
