Re: [RFC 0/3] watchdog: do not allow reboot without CAP_SYS_BOOT set

[Tony Zelenoff <antonz@parallels.com>]

8/06/12 6:28 PM, Hans de Goede пишет:
> Hi,
>
> On 06/08/2012 03:09 PM, Tony Zelenoff wrote:
>> The CAP_SYS_BOOT capability required to reboot hardware node. But watchdog
>> writers are not checked for this capability. So, the process may reboot
>> hardware node even if it has no any capabilities to do it.
>
> Hmm, I can imagine people explicitly doing a chown on /dev/watchdog, to allow
> some non root running, critical from a service availability pov, process to
> open it and ping it.
>
> The suggest change would mean for most standard linux distributions, that
> a process opening /dev/watchdog now must run as root, even if the rights
> of /dev/watchdog allow a process to open it.
Hmm. I've missed it ) The patches may be modified to skip capabilities check
when watchdog opened from non root user.


> Also since you add the check on open, not on specific syscalls you are
> adding extra security checks to the open path. Now users are trained when
> open() fails with -EPERM to check
> 1: Standard unix file rights
> 2: For selinux denials
>
> Adding a third way to make open() fail with -EPERM is not going to make
> sysadmins very happy, esp. since this will not have any special logging
> to make the cause clear (unlike selinux).
Add log message is not problem too. The EPERM error got from other places,
where this capability checked. May be you can suggest better error code?

> Moreover, since you add the check to open, what does it buy us over
> normal file-permissions? We already have a perfectly fine way to limit
> access to the watchdog device, namely standard unix file permissions,
> needing to fiddle with both file permissions and capabilities to allow
> a non root process to open /dev/watchdog is not making things easier,
> while at the same time not adding any value, since no extra granularity
> wrt security is gained.
Hm, so for what capabilities were created if standard permissions are 
good enough? Reason of this patchset is to guard one more way to reboot 
hardware node in same manner as it does in other places, because now 
root process without this capability set can write something to watchdog 
device and after some timeout the hardware reboots. May be my way is 
wrong, but this looks like a small security hole when non authorized 
process do things that it should not be able to do.


> Last, but not least, this will break userspace ABI compatibility, which is
> a very strong "thy shall never do that" scenario.
> So all in all, a strong nack from me.


--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html