linux-kernel - Re: [PATCH 0/3] SG_IO command filtering via sysfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181116174352.GH20617@thunk.org>
Date:   Fri, 16 Nov 2018 12:43:52 -0500
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Christoph Hellwig <hch@...radead.org>,
        linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org,
        Hannes Reinecke <hare@...e.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        James Bottomley <James.Bottomley@...senpartnership.com>
Subject: Re: [PATCH 0/3] SG_IO command filtering via sysfs

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