[PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[Paolo Bonzini]

Persistent reservations commands cannot be issued right now without
giving CAP_SYS_RAWIO to the process who wishes to send them.  This
is a bit heavy-handed, allow these two commands.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        Ok for 3.5 as well?

 block/scsi_ioctl.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 260fa80..5d6c9c1 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -137,6 +137,7 @@ static void blk_set_cmd_filter_defaults(struct blk_cmd_filter *filter)
 	__set_bit(SERVICE_ACTION_IN, filter->read_ok);
 	__set_bit(RECEIVE_DIAGNOSTIC, filter->read_ok);
 	__set_bit(MAINTENANCE_IN, filter->read_ok);
+	__set_bit(PERSISTENT_RESERVE_IN, filter->read_ok);
 	__set_bit(GPCMD_READ_BUFFER_CAPACITY, filter->read_ok);
 
 	/* Audio CD commands */
@@ -178,6 +179,7 @@ static void blk_set_cmd_filter_defaults(struct blk_cmd_filter *filter)
 	__set_bit(GPCMD_MODE_SELECT_10, filter->write_ok);
 	__set_bit(MODE_SELECT, filter->write_ok);
 	__set_bit(LOG_SELECT, filter->write_ok);
+	__set_bit(PERSISTENT_RESERVE_OUT, filter->write_ok);
 	__set_bit(GPCMD_BLANK, filter->write_ok);
 	__set_bit(GPCMD_CLOSE_TRACK, filter->write_ok);
 	__set_bit(GPCMD_FLUSH_CACHE, filter->write_ok);
-- 
1.7.1

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[James Bottomley]

On Tue, 2012-06-12 at 18:08 +0200, Paolo Bonzini wrote:
> Persistent reservations commands cannot be issued right now without
> giving CAP_SYS_RAWIO to the process who wishes to send them.  This
> is a bit heavy-handed, allow these two commands.

Why is this heavy handed?  If you remove CAP_SYS_RAWIO, any userspace
process can send these, which would allow any user to completely disrupt
a SAN by injecting spurious reservations ... that doesn't look to be
terribly safe for an operating system running in a data centre.

James
 

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[Paolo Bonzini]

Il 12/06/2012 18:21, James Bottomley ha scritto:
>> > Persistent reservations commands cannot be issued right now without
>> > giving CAP_SYS_RAWIO to the process who wishes to send them.  This
>> > is a bit heavy-handed, allow these two commands.
>
> Why is this heavy handed?  If you remove CAP_SYS_RAWIO, any userspace
> process can send these, which would allow any user to completely disrupt
> a SAN by injecting spurious reservations ... that doesn't look to be
> terribly safe for an operating system running in a data centre.

It is heavy-handed because:

1) there are still other protections such as DAC (both Unix permissions
and ACLs) and SELinux; CAP_SYS_RAWIO is effectively the same as root.

2) if any user could disrupt the SAN by injecting spurious reservations
just by having his laptop's root password, that data centre wouldn't be
terribly safe to begin with.

Paolo

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[Paolo Bonzini]

Il 12/06/2012 18:24, Paolo Bonzini ha scritto:
> Il 12/06/2012 18:21, James Bottomley ha scritto:
>>>> Persistent reservations commands cannot be issued right now without
>>>> giving CAP_SYS_RAWIO to the process who wishes to send them.  This
>>>> is a bit heavy-handed, allow these two commands.
>>
>> Why is this heavy handed?  If you remove CAP_SYS_RAWIO, any userspace
>> process can send these, which would allow any user to completely disrupt
>> a SAN by injecting spurious reservations ... that doesn't look to be
>> terribly safe for an operating system running in a data centre.
> 
> It is heavy-handed because:
> 
> 1) there are still other protections such as DAC (both Unix permissions
> and ACLs) and SELinux; CAP_SYS_RAWIO is effectively the same as root.
> 
> 2) if any user could disrupt the SAN by injecting spurious reservations
> just by having his laptop's root password, that data centre wouldn't be
> terribly safe to begin with.

3) assume that with this patch user X could disrupt the SAN by injecting
spurious reservations, e.g. forbidding another user from writing some
data.  Then they could also destroy those same data even without this
patch, which is just as disrupting.

This is because you still need write permission to the device to issue
reservations.  Read permission will only let you use PERSISTENT RESREVE IN.

Paolo

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[James Bottomley]

On Tue, 2012-06-12 at 18:54 +0200, Paolo Bonzini wrote:
> Il 12/06/2012 18:24, Paolo Bonzini ha scritto:
> > Il 12/06/2012 18:21, James Bottomley ha scritto:
> >>>> Persistent reservations commands cannot be issued right now without
> >>>> giving CAP_SYS_RAWIO to the process who wishes to send them.  This
> >>>> is a bit heavy-handed, allow these two commands.
> >>
> >> Why is this heavy handed?  If you remove CAP_SYS_RAWIO, any userspace
> >> process can send these, which would allow any user to completely disrupt
> >> a SAN by injecting spurious reservations ... that doesn't look to be
> >> terribly safe for an operating system running in a data centre.
> > 
> > It is heavy-handed because:
> > 
> > 1) there are still other protections such as DAC (both Unix permissions
> > and ACLs) and SELinux; CAP_SYS_RAWIO is effectively the same as root.
> > 
> > 2) if any user could disrupt the SAN by injecting spurious reservations
> > just by having his laptop's root password, that data centre wouldn't be
> > terribly safe to begin with.
> 
> 3) assume that with this patch user X could disrupt the SAN by injecting
> spurious reservations, e.g. forbidding another user from writing some
> data.  Then they could also destroy those same data even without this
> patch, which is just as disrupting.
> 
> This is because you still need write permission to the device to issue
> reservations.  Read permission will only let you use PERSISTENT RESREVE IN.

I don't think you understand how persistent reservations work.

The first thing I'll say is I agree with Alan.  Unless you can justify
why you want to relax permissions I'm not going to do it.

But secondly, the reason we're so up in arms about SCSI-3 PR is that
there's a feature called reservation by transport ID.  This is used to
reserve multipath devices when one of the paths is down.  Effectively it
allows a PR-OUT command to set a reservation on any LUN with access only
to one of them.  It's definitely a hack in the SCSI standard, but it's
not one that can be controlled by a unix like permission model.  Write
access to *any* LUN allows you to reserve *all* luns.

James

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[Paolo Bonzini]

Il 12/06/2012 19:20, James Bottomley ha scritto:
> I don't think you understand how persistent reservations work.
> 
> The first thing I'll say is I agree with Alan.  Unless you can justify
> why you want to relax permissions I'm not going to do it.

See my answer to John.  The reason is that I want to let VMs use
persistent reservations without running them as root.

> But secondly, the reason we're so up in arms about SCSI-3 PR is that
> there's a feature called reservation by transport ID.  This is used to
> reserve multipath devices when one of the paths is down.  Effectively it
> allows a PR-OUT command to set a reservation on any LUN with access only
> to one of them.  It's definitely a hack in the SCSI standard, but it's
> not one that can be controlled by a unix like permission model.  Write
> access to *any* LUN allows you to reserve *all* luns.

Thanks for taking the time to explain---I knew about this, but I thought
it could (perhaps should) be disabled on the SAN.  Anybody could already
use reservation by transport ID if they had root access on the local
machine, no?

Paolo

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[James Bottomley]

On Tue, 2012-06-12 at 19:25 +0200, Paolo Bonzini wrote:
> Il 12/06/2012 19:20, James Bottomley ha scritto:
> > But secondly, the reason we're so up in arms about SCSI-3 PR is that
> > there's a feature called reservation by transport ID.  This is used to
> > reserve multipath devices when one of the paths is down.  Effectively it
> > allows a PR-OUT command to set a reservation on any LUN with access only
> > to one of them.  It's definitely a hack in the SCSI standard, but it's
> > not one that can be controlled by a unix like permission model.  Write
> > access to *any* LUN allows you to reserve *all* luns.
> 
> Thanks for taking the time to explain---I knew about this, but I thought
> it could (perhaps should) be disabled on the SAN.  Anybody could already
> use reservation by transport ID if they had root access on the local
> machine, no?

No ... it's required for multipath to work correctly and multipath is a
usual enterprise feature.

The only way around this is either to trust your users or not to give
out root ... and most data centres choose the latter.  It causes real
pain from NPIV and SR-IOV ... you need hardware solutions like some type
of SAN volume controller to fix it.

James

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[Paolo Bonzini]

Il 12/06/2012 20:02, James Bottomley ha scritto:
>> > Thanks for taking the time to explain---I knew about this, but I thought
>> > it could (perhaps should) be disabled on the SAN.  Anybody could already
>> > use reservation by transport ID if they had root access on the local
>> > machine, no?
> No ... it's required for multipath to work correctly and multipath is a
> usual enterprise feature.
> 
> The only way around this is either to trust your users or not to give
> out root ... and most data centres choose the latter.  It causes real
> pain from NPIV and SR-IOV ...

I can imagine...  my impression was that it would only affect whatever
LUNs the zoning allowed access to (NPIV is pretty much required to use
persistent reservations on guests, or guests will all share the same WWN).

Would it be acceptable to restrict access to PR OUT with ALL_TG_PT set,
and allow it freely without the flag?

Paolo

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[Alan Cox]

On Tue, 12 Jun 2012 18:08:32 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Persistent reservations commands cannot be issued right now without
> giving CAP_SYS_RAWIO to the process who wishes to send them.  This
> is a bit heavy-handed, allow these two commands.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         Ok for 3.5 as well?

NAK.

Persistent reservations are exactly the kind of command that should have
a security model attached to them.

Red Hat seems to be an ever growing source of "mummy its hard, lets
disable all the security" type fixes. Please stop it.

There is a sensible debate to be had about whether a lesser privilege
ought to be allowed. The real fix to this as with half of the other
crazy attempts to break all the security models that seem to keep spewing
forth is for someone who cares about it (that seems to me Red Hat) add
support for pushing a BPF filter onto a block device command queue. 

All the supporting code is there and used for other stuff, we can even
jit the things, not that it's a fast path here.

Alan

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[Paolo Bonzini]

Il 12/06/2012 18:55, Alan Cox ha scritto:
> On Tue, 12 Jun 2012 18:08:32 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Persistent reservations commands cannot be issued right now without
>> giving CAP_SYS_RAWIO to the process who wishes to send them.  This
>> is a bit heavy-handed, allow these two commands.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>         Ok for 3.5 as well?
> 
> NAK.
> 
> Persistent reservations are exactly the kind of command that should have
> a security model attached to them.

There is.  It's called "chmod"; you don't give write access to LUNs to
random users.  and SCM_RIGHTS is what lets you override it securely.

> Red Hat seems to be an ever growing source of "mummy its hard, lets
> disable all the security" type fixes. Please stop it.

Last time you were complaining that I was turning things *off* (SG_IO to
partitions for root).  Now you complain that I'm turning things *on*.
It's difficult to say they are the same thing.  Though perhaps you were
talking about someone else.

> There is a sensible debate to be had about whether a lesser privilege
> ought to be allowed. The real fix to this as with half of the other
> crazy attempts to break all the security models that seem to keep spewing
> forth is for someone who cares about it (that seems to me Red Hat) add
> support for pushing a BPF filter onto a block device command queue. 

Sure; however, doing so requires access to some member of "struct file"
from SG_IO.  Thus, ioctl would need to take a "struct file" rather than
just an fmode_t.

The switch to fmode_t was done in 2007 by Al Viro.  I would like to
understand the reasons for the switch; it seems to me that it was part
of the big kernel lock removal.  If it's acceptable to undo it, I would
very much would like to add generic BPF filtering to SG_IO.

Paolo

Can we pass a file handle down to the block ioctls to implement per file filters on scsi SG_IO ?

[Alan Cox]

> > Persistent reservations are exactly the kind of command that should have
> > a security model attached to them.
> 
> There is.  It's called "chmod"; you don't give write access to LUNs to
> random users.  and SCM_RIGHTS is what lets you override it securely.

Only you do - for virtual machines for example.

> > Red Hat seems to be an ever growing source of "mummy its hard, lets
> > disable all the security" type fixes. Please stop it.
> 
> Last time you were complaining that I was turning things *off* (SG_IO to
> partitions for root).  Now you complain that I'm turning things *on*.
> It's difficult to say they are the same thing.  Though perhaps you were
> talking about someone else.

Last time you were trying to break stuff that worked and stop the
privileged user doing it. This time you are tring to take away security
features that users might be relying on.

> > There is a sensible debate to be had about whether a lesser privilege
> > ought to be allowed. The real fix to this as with half of the other
> > crazy attempts to break all the security models that seem to keep spewing
> > forth is for someone who cares about it (that seems to me Red Hat) add
> > support for pushing a BPF filter onto a block device command queue. 
> 
> Sure; however, doing so requires access to some member of "struct file"
> from SG_IO.  Thus, ioctl would need to take a "struct file" rather than
> just an fmode_t.
> 
> The switch to fmode_t was done in 2007 by Al Viro.  I would like to
> understand the reasons for the switch; it seems to me that it was part
> of the big kernel lock removal.  If it's acceptable to undo it, I would
> very much would like to add generic BPF filtering to SG_IO

That would solve all sorts of problems in this area, so lets ping the man
himself -  Al ?

Alan

Re: Can we pass a file handle down to the block ioctls to implement per file filters on scsi SG_IO ?

[Paolo Bonzini]

Il 12/06/2012 20:52, Alan Cox ha scritto:
> > Sure; however, doing so requires access to some member of "struct file"
> > from SG_IO.  Thus, ioctl would need to take a "struct file" rather than
> > just an fmode_t.
> > 
> > The switch to fmode_t was done in 2007 by Al Viro.  I would like to
> > understand the reasons for the switch; it seems to me that it was part
> > of the big kernel lock removal.  If it's acceptable to undo it, I would
> > very much would like to add generic BPF filtering to SG_IO
> 
> That would solve all sorts of problems in this area, so lets ping the man
> himself -  Al ?

(Even if he says no, we could probably use cgroups instead).

Paolo

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[John Stoffel]

Paolo> Persistent reservations commands cannot be issued right now
Paolo> without giving CAP_SYS_RAWIO to the process who wishes to send
Paolo> them.  This is a bit heavy-handed, allow these two commands.

This seems like a bad idea, now anyone can just put in a SCSI
reservation on a system and then you have to hunt around trying to
figure it out.  

What's the motivation here?  What's the use case this solves?  

John



Paolo> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo> ---
Paolo>         Ok for 3.5 as well?

Paolo>  block/scsi_ioctl.c |    2 ++
Paolo>  1 files changed, 2 insertions(+), 0 deletions(-)

Paolo> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
Paolo> index 260fa80..5d6c9c1 100644
Paolo> --- a/block/scsi_ioctl.c
Paolo> +++ b/block/scsi_ioctl.c
Paolo> @@ -137,6 +137,7 @@ static void blk_set_cmd_filter_defaults(struct blk_cmd_filter *filter)
Paolo>  	__set_bit(SERVICE_ACTION_IN, filter->read_ok);
Paolo>  	__set_bit(RECEIVE_DIAGNOSTIC, filter->read_ok);
Paolo>  	__set_bit(MAINTENANCE_IN, filter->read_ok);
Paolo> +	__set_bit(PERSISTENT_RESERVE_IN, filter->read_ok);
Paolo>  	__set_bit(GPCMD_READ_BUFFER_CAPACITY, filter->read_ok);
 
Paolo>  	/* Audio CD commands */
Paolo> @@ -178,6 +179,7 @@ static void blk_set_cmd_filter_defaults(struct blk_cmd_filter *filter)
Paolo>  	__set_bit(GPCMD_MODE_SELECT_10, filter->write_ok);
Paolo>  	__set_bit(MODE_SELECT, filter->write_ok);
Paolo>  	__set_bit(LOG_SELECT, filter->write_ok);
Paolo> +	__set_bit(PERSISTENT_RESERVE_OUT, filter->write_ok);
Paolo>  	__set_bit(GPCMD_BLANK, filter->write_ok);
Paolo>  	__set_bit(GPCMD_CLOSE_TRACK, filter->write_ok);
Paolo>  	__set_bit(GPCMD_FLUSH_CACHE, filter->write_ok);
Paolo> -- 
Paolo> 1.7.1

Paolo> --
Paolo> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Paolo> the body of a message to majordomo@vger.kernel.org
Paolo> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] scsi: allow persistent reservations without CAP_SYS_RAWIO

[Paolo Bonzini]

Il 12/06/2012 19:08, John Stoffel ha scritto:
> Paolo> Persistent reservations commands cannot be issued right now
> Paolo> without giving CAP_SYS_RAWIO to the process who wishes to send
> Paolo> them.  This is a bit heavy-handed, allow these two commands.
> 
> This seems like a bad idea, now anyone can just put in a SCSI
> reservation on a system and then you have to hunt around trying to
> figure it out.  

What's the difference from anyone destroying data on a disk?  You still
need write access to the block device node.  Also, you could already do
the same if you have root permissions on your _local_ machine.

(BTW, please reply to these objections where I already stated them, in
the answer to James Bottomley).

> What's the motivation here?  What's the use case this solves?  

I would like to give access to persistent reservations to VMs, without
having to run qemu as root.  One alternative is to run a userspace iSCSI
initiator, but of course that would only work with iSCSI.

Paolo