All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
	Hannes Reinecke <hare@suse.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	James Bottomley <James.Bottomley@hansenpartnership.com>
Subject: Re: [PATCH 0/3] SG_IO command filtering via sysfs
Date: Fri, 16 Nov 2018 12:43:52 -0500	[thread overview]
Message-ID: <20181116174352.GH20617@thunk.org> (raw)
In-Reply-To: <fa3ed175-13a3-bedf-ebf9-ddf46a2decd5@redhat.com>

On Fri, Nov 16, 2018 at 10:45:11AM +0100, Paolo Bonzini wrote:
> On 16/11/18 10:32, Christoph Hellwig wrote:
> > On Mon, Nov 12, 2018 at 11:17:29AM +0100, Paolo Bonzini wrote:
> >>> Well, that's what we have the security_file_ioctl() LSM hook for so that
> >>> your security model can arbitrate access to ioctls.
> >>
> >> Doesn't that have TOC-TOU races by design?
> > 
> > If you want to look at the command - yes.  If you just want to filter
> > read vs write vs ioctl, no.
> 
> Yeah, but looking at the command is what Ted wants.  The thing that we
> did in RHEL was a single sysfs bool that allows unfiltered access,
> because it was sort of enough and made the delta very small.  But for
> upstream I want to do it right, even if that means learning all that
> new-fangled BPF stuff. :)

I'd argue that a purpose-built eBPF access control facility is
superior to the security_file_ioctl() LSM hook because it can make
available to the authorization function access to the cached results
of the SCSI INQUIRY command, and it avoids needing to duplicate
knowledge of how to parse the parameters of the SG_IO ioctl in the LSM
module as well as in the SCSI stack.

Just because you *could* implement anything in terms of a turing
machine tape doesn't mean that it is good idea.  Similarly, just
because you *can* implement something as an LSM hook doesn't mean that
it's the best design.

> >> Also, what about SG_IO giving write access to files that are only opened
> >> read-only (and only have read permissions)?
> > 
> > Allowing SG_IO on read-only permissions sounds like a reall bad idea,
> > filtering or not.
> 
> I would even agree, however it's allowed right now and I would be
> surprised if no one was relying on it in good faith ("I'm just doing an
> INQUIRY, why do I need to open O_RDWR").  And indeed:
> 
> $ sudo chmod a+r /dev/sda
> $ strace -e openat sg_inq /dev/sda
> openat(AT_FDCWD, "/dev/sda", O_RDONLY|O_NONBLOCK) = 3
>                              ^^^^^^^^
> 
> So it would be a regression.

Ugh, that's... unfortunate.  I suppose we could try to figure out all
of the SCSI commands that would have to be white-listed to be allowed
using O_RDONLY from historical usage, but that would be a huge job,
and it's highly likely we would miss some anyway.  OTOH, this could be
called a security botch that should be fixed, and if we make a best
effort to white list all of the innocuous cases such as SCSI INQUIRY,
maybe that would be acceptible.

					- Ted

  parent reply	other threads:[~2018-11-16 17:44 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-10 16:35 [PATCH 0/3] SG_IO command filtering via sysfs Paolo Bonzini
2018-11-10 16:35 ` [PATCH 1/3] block: add back queue-private command filter Paolo Bonzini
2018-11-10 16:35 ` [PATCH 2/3] scsi: create an all-one filter for scanners Paolo Bonzini
2018-11-10 16:35 ` [PATCH 3/3] block: add back command filter modification via sysfs Paolo Bonzini
2018-11-16  5:46   ` Bart Van Assche
2018-11-16  7:00     ` Paolo Bonzini
2018-11-16 14:42       ` Bart Van Assche
2018-11-10 19:05 ` [PATCH 0/3] SG_IO command filtering " Theodore Y. Ts'o
2018-11-11 13:26   ` Paolo Bonzini
2018-11-11 14:14     ` Theodore Y. Ts'o
2018-11-16  0:26       ` Paolo Bonzini
2018-11-16  0:37         ` Bart Van Assche
2018-11-16  7:01           ` Paolo Bonzini
2018-11-16 17:35             ` Theodore Y. Ts'o
2018-11-11 13:14 ` Christoph Hellwig
2018-11-11 13:42   ` Theodore Y. Ts'o
2018-11-12  8:20     ` Christoph Hellwig
2018-11-12 10:17       ` Paolo Bonzini
2018-11-16  9:32         ` Christoph Hellwig
2018-11-16  9:45           ` Paolo Bonzini
2018-11-16  9:48             ` Christoph Hellwig
2018-11-16 17:43             ` Theodore Y. Ts'o [this message]
2018-11-16 18:17               ` Bart Van Assche
2018-11-16 21:08                 ` Paolo Bonzini

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=20181116174352.GH20617@thunk.org \
    --to=tytso@mit.edu \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=hare@suse.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=pbonzini@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.