Re: [Dnsmasq-discuss] Minimal capabilities for options
http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=305ffb5ef0ba5ab1df32ef80f266a4c9e395ca13 is a first pass on this. I have a nasty feeling that there are configurations which need some of the capabilities and have had a free pass because they are always there, which I've missed (I only just noticed that the IPset code needs NET_ADMIN) No doubt they will turn up eventually. Please test! Cheers, Simon. On 20/01/2019 10:38, 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 > > Mathieu > > ___ > Dnsmasq-discuss mailing list > Dnsmasq-discuss@lists.thekelleys.org.uk > http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss > ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] Minimal capabilities for options
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 ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
[Dnsmasq-discuss] Minimal capabilities for options
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 Mathieu ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
