linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Tony Zelenoff <antonz@parallels.com>
Cc: linux-watchdog@vger.kernel.org, wim@iguana.be
Subject: Re: [RFC 0/3] watchdog: do not allow reboot without CAP_SYS_BOOT set
Date: Fri, 08 Jun 2012 16:28:04 +0200	[thread overview]
Message-ID: <4FD20BF4.7010607@redhat.com> (raw)
In-Reply-To: <1339160997-160581-1-git-send-email-antonz@parallels.com>

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.

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).

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.

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.

Regards,

Hans

  parent reply	other threads:[~2012-06-08 14:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-08 13:09 [RFC 0/3] watchdog: do not allow reboot without CAP_SYS_BOOT set Tony Zelenoff
2012-06-08 13:09 ` [RFC 1/3] watchdog: check CAP_SYS_BOOT at watchdog open Tony Zelenoff
2012-06-08 13:09 ` [RFC 2/3] watchdog: move err initialization to place it used Tony Zelenoff
2012-06-08 13:09 ` [RFC 3/3] watchdog: connect watchdog_may_open to legacy code Tony Zelenoff
2012-06-08 14:28 ` Hans de Goede [this message]
2012-06-08 15:12   ` [RFC 0/3] watchdog: do not allow reboot without CAP_SYS_BOOT set Tony Zelenoff
2012-06-08 20:42     ` Hans de Goede
2012-06-09 15:28       ` Tony Zelenoff

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FD20BF4.7010607@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=antonz@parallels.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).