[Dnsmasq-discuss] Minimal capabilities for options

[Geert Stappers]

On Sun, Jan 20, 2019 at 02:38:46AM -0800, Mathieu Hofman wrote:
> Running dnsmasq in docker currently requires explicitly granting the
> NET_ADMIN capability for the container, or dnsmasq fails to start if
> configured to drop root.
> 
> The failure is due to a capset() call that includes NET_ADMIN when dnsmasq
> attempts to keep capabilities before dropping root. If the capability is
> not already available, the call fails. If dnsmasq doesn't drop root, the
> checks are skipped and it starts successfully without the capability, but
> potentially fails later depending on the configured options.
> 
> From what I gather, the NET_ADMIN capability is only needed to inject a
> neighbor / ARP entry after receiving a DHCP request from a client so that
> the response can be sent using unicast. The capability is not required if
> the DHCP server is disabled or if the dhcp-broadcast option is used.
> 
> The NET_RAW capability is similarly needed to send an ICMP ping before
> allocating an address for a client, but pings are not used if the DHCP
> server is disabled or if the no-ping option is specified.
> 
> I would like to suggest 2 improvements to the way capabilities are handled
> in dnsmasq:
> 
> 1) Only try to keep capabilities which are actually needed according to the
> configured options. If the DHCP server is disabled, do not keep (or request
> if not available) the NET_ADMIN and NET_RAW capabilities. If the
> dhcp-broadcast option is specified, do not include NET_ADMIN. If no-ping is
> specified, do not include NET_RAW.
> Currently the NET_BIND_SERVICE capability is kept only if DAD or dynamic
> binding are required by the config. This suggestion would use similar logic
> for the NET_ADMIN and NET_RAW capabilities.
> 
> 2) Check that the capabilities required for the configuration are available
> to the process when starting, and fail early if they are not. Currently
> capabilities are not checked. It's only a side effect of the capset() call
> when dropping root that dnsmasq will fail to start if a capability is
> missing. If dnsmasq is configured to not drop privileges, such as starting
> as a non-root user, or staying root without changing user, dnsmasq will
> only fail later when attempting to use a feature requiring the capability.
> 
> Optionally, dnsmasq could try to automatically disable any configured
> feature that relies on the missing capability, and probably warn such
> action was taken in the logs. For the NET_RAW capability, it's probably not
> possible to disable pings if the DHCP server is not authoritative as that
> might be too risky. For the NET_ADMIN capability hopefully it's safe to
> automatically switch to dhcp-broadcast.
> 
> For now, I'm working around the current capabilities handling by manually
> dropping root and granting the required capabilities to dnsmasq before
> executing it. Dnsmasq seem to run fine from what I can tell, but I've only
> tested it in my environment.
> My docker config for this is here:
> https://gist.github.com/mhofman/cdd85a6baa4b9206830b254d0ab9bb89
> 
> To summarize the new suggested capabilities logic would be:
> - Figure out the set of capabilities required for the configured options
> (regardless of user config).
> - Check if the process has the required capabilities. Fail if not, or
> optionally gracefully degrade features and warn.
> - If configured to drop root, call capset() only with required capabilities.
> 
> The current alternative is not acceptable in my opinion: keep running as
> root (or worse, in debug mode).
> 
> This change would also help pi-hole which recently added code to check for
> available capabilities before invoking dnsmasq. See
> https://github.com/pi-hole/FTL/issues/432
> 
> A similar request was made in 2013:
> http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2013q2/007188.html
> 

Some how I did read that as
  "Before I spend time on improving dnsmasq source code, I would
   like to known how if changes against capability will be excepted?"


So now writing that explicied.


Cheers
Geert Stappers
DevOps Engineer at Hendrikx-ITC