Re: [PATCH] iscsi_ibft,iscsi_boot: remove CAP_SYS_ADMIN restriction for reading entries

[Christian Seiler]

On 10/05/2016 08:28 PM, Dan Williams wrote:
> On Tue, 2016-10-04 at 19:23 -0400, Konrad Rzeszutek Wilk wrote:
>> On Oct 4, 2016 12:11 PM, "Dan Williams"  wrote:
>>>
>>>
>>> On Tue, 2016-10-04 at 12:08 -0400, Peter Jones wrote:

 On Tue, Oct 04, 2016 at 11:03:05AM -0500, Dan Williams wrote:
>
>
> All the iSCSI boot entries are read-only anyway; it's unclear
> why
> the
> CAP_SYS_ADMIN restriction is in place since this information
> isn't
> particularly sensitive and cannot be changed.  Userspace
> applications
> may want to read this without requiring CAP_SYS_ADMIN for their
> entire process just for iBFT info.
>
> Signed-off-by: Dan Williams 

 Uh, because there are login credentials to the target in there.
>>>
>>> Fair enough.  So can we just check CAP_SYS_ADMIN for the login
>>> credentials, and not check it for all the IP details and such?
>>
>> The only consumer is iscsiadm - which runs as root. So why expose
>> this
>> information to non root ?
> 
> This is more about root processes dropping unnecessary privileges after
> starting.  But at least for the network stuff, there doesn't seem to be
> a good reason to restrict stuff to root either; like 'ifconfig' doesn't
> require root access to display info.
> 
> While NetworkManager currently spawns iscsiadm to read this stuff, NM
> only cares about the iBFT network settings and we'd rather just read
> sysfs directly instead of spawing and parsing iscsiadm output.  So I
> wouldn't expect the only consumer to be iscsiadm in the near future.
> 
> Once we do this, NM would also like to drop CAP_SYS_ADMIN after staring
> since we don't actually need it for anything except reading iBFT.

Maybe CAP_SYS_ADMIN is simply the wrong capability here? If this is
supposed to remain restricted, maybe using CAP_NET_ADMIN for the
network configuration data (NetworkManager has that, right?) and
CAP_SYS_ADMIN for the rest? (Target name, portal, auth data.)
That would still be rather restrictive, but better suited for the
use case in mind?

Regards,
Christian

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] iscsi_ibft,iscsi_boot: remove CAP_SYS_ADMIN restriction for reading entries

[Dan Williams]

On Tue, 2016-10-04 at 19:23 -0400, Konrad Rzeszutek Wilk wrote:
> On Oct 4, 2016 12:11 PM, "Dan Williams"  wrote:
> > 
> > 
> > On Tue, 2016-10-04 at 12:08 -0400, Peter Jones wrote:
> > > 
> > > On Tue, Oct 04, 2016 at 11:03:05AM -0500, Dan Williams wrote:
> > > > 
> > > > 
> > > > All the iSCSI boot entries are read-only anyway; it's unclear
> > > > why
> > > > the
> > > > CAP_SYS_ADMIN restriction is in place since this information
> > > > isn't
> > > > particularly sensitive and cannot be changed.  Userspace
> > > > applications
> > > > may want to read this without requiring CAP_SYS_ADMIN for their
> > > > entire process just for iBFT info.
> > > > 
> > > > Signed-off-by: Dan Williams 
> > > 
> > > Uh, because there are login credentials to the target in there.
> > 
> > Fair enough.  So can we just check CAP_SYS_ADMIN for the login
> > credentials, and not check it for all the IP details and such?
> 
> The only consumer is iscsiadm - which runs as root. So why expose
> this
> information to non root ?

This is more about root processes dropping unnecessary privileges after
starting.  But at least for the network stuff, there doesn't seem to be
a good reason to restrict stuff to root either; like 'ifconfig' doesn't
require root access to display info.

While NetworkManager currently spawns iscsiadm to read this stuff, NM
only cares about the iBFT network settings and we'd rather just read
sysfs directly instead of spawing and parsing iscsiadm output.  So I
wouldn't expect the only consumer to be iscsiadm in the near future.

Once we do this, NM would also like to drop CAP_SYS_ADMIN after staring
since we don't actually need it for anything except reading iBFT.

Dan

> > 
> > 
> > Dan
> > 
> > > 
> > > > 
> > > > 
> > > > ---
> > > >  drivers/scsi/iscsi_boot_sysfs.c | 3 ---
> > > >  1 file changed, 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/scsi/iscsi_boot_sysfs.c
> > > > b/drivers/scsi/iscsi_boot_sysfs.c
> > > > index d453667..4e9c324 100644
> > > > --- a/drivers/scsi/iscsi_boot_sysfs.c
> > > > +++ b/drivers/scsi/iscsi_boot_sysfs.c
> > > > @@ -47,9 +47,6 @@ static ssize_t
> > > > iscsi_boot_show_attribute(struct
> > > > kobject *kobj,
> > > > ssize_t ret = -EIO;
> > > > char *str = buf;
> > > > 
> > > > -   if (!capable(CAP_SYS_ADMIN))
> > > > -   return -EACCES;
> > > > -
> > > > if (boot_kobj->show)
> > > > ret = boot_kobj->show(boot_kobj->data, boot_attr-
> > > > > 
> > > > > type, str);
> > > > return ret;
> > > > --
> > > > 2.7.4
> > > 

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] iscsi_ibft,iscsi_boot: remove CAP_SYS_ADMIN restriction for reading entries

[Konrad Rzeszutek Wilk]

On Oct 4, 2016 12:11 PM, "Dan Williams"  wrote:
>
> On Tue, 2016-10-04 at 12:08 -0400, Peter Jones wrote:
> > On Tue, Oct 04, 2016 at 11:03:05AM -0500, Dan Williams wrote:
> > >
> > > All the iSCSI boot entries are read-only anyway; it's unclear why
> > > the
> > > CAP_SYS_ADMIN restriction is in place since this information isn't
> > > particularly sensitive and cannot be changed.  Userspace
> > > applications
> > > may want to read this without requiring CAP_SYS_ADMIN for their
> > > entire process just for iBFT info.
> > >
> > > Signed-off-by: Dan Williams 
> >
> > Uh, because there are login credentials to the target in there.
>
> Fair enough.  So can we just check CAP_SYS_ADMIN for the login
> credentials, and not check it for all the IP details and such?

The only consumer is iscsiadm - which runs as root. So why expose this
information to non root ?

>
> Dan
>
> > >
> > > ---
> > >  drivers/scsi/iscsi_boot_sysfs.c | 3 ---
> > >  1 file changed, 3 deletions(-)
> > >
> > > diff --git a/drivers/scsi/iscsi_boot_sysfs.c
> > > b/drivers/scsi/iscsi_boot_sysfs.c
> > > index d453667..4e9c324 100644
> > > --- a/drivers/scsi/iscsi_boot_sysfs.c
> > > +++ b/drivers/scsi/iscsi_boot_sysfs.c
> > > @@ -47,9 +47,6 @@ static ssize_t iscsi_boot_show_attribute(struct
> > > kobject *kobj,
> > > ssize_t ret = -EIO;
> > > char *str = buf;
> > >
> > > -   if (!capable(CAP_SYS_ADMIN))
> > > -   return -EACCES;
> > > -
> > > if (boot_kobj->show)
> > > ret = boot_kobj->show(boot_kobj->data, boot_attr-
> > > >type, str);
> > > return ret;
> > > --
> > > 2.7.4
> >

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Antw: Re: [PATCH] iscsi_ibft,iscsi_boot: remove CAP_SYS_ADMIN restriction for reading entries

[Ulrich Windl]

>>> Dan Williams  schrieb am 04.10.2016 um 18:11 in Nachricht
<1475597465.21760.3.ca...@redhat.com>:
> On Tue, 2016-10-04 at 12:08 -0400, Peter Jones wrote:
>> On Tue, Oct 04, 2016 at 11:03:05AM -0500, Dan Williams wrote:
>> > 
>> > All the iSCSI boot entries are read-only anyway; it's unclear why
>> > the
>> > CAP_SYS_ADMIN restriction is in place since this information isn't
>> > particularly sensitive and cannot be changed.  Userspace
>> > applications
>> > may want to read this without requiring CAP_SYS_ADMIN for their
>> > entire process just for iBFT info.
>> > 
>> > Signed-off-by: Dan Williams 
>> 
>> Uh, because there are login credentials to the target in there.
> 
> Fair enough.  So can we just check CAP_SYS_ADMIN for the login
> credentials, and not check it for all the IP details and such?

The "need to know?" principle: Who needs to know that information?

> 
> Dan
> 
>> > 
>> > ---
>> >  drivers/scsi/iscsi_boot_sysfs.c | 3 ---
>> >  1 file changed, 3 deletions(-)
>> > 
>> > diff --git a/drivers/scsi/iscsi_boot_sysfs.c
>> > b/drivers/scsi/iscsi_boot_sysfs.c
>> > index d453667..4e9c324 100644
>> > --- a/drivers/scsi/iscsi_boot_sysfs.c
>> > +++ b/drivers/scsi/iscsi_boot_sysfs.c
>> > @@ -47,9 +47,6 @@ static ssize_t iscsi_boot_show_attribute(struct
>> > kobject *kobj,
>> >ssize_t ret = -EIO;
>> >char *str = buf;
>> >  
>> > -  if (!capable(CAP_SYS_ADMIN))
>> > -  return -EACCES;
>> > -
>> >if (boot_kobj->show)
>> >ret = boot_kobj->show(boot_kobj->data, boot_attr-
>> > >type, str);
>> >return ret;
>> > -- 
>> > 2.7.4
>> 
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "open-iscsi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to open-iscsi+unsubscr...@googlegroups.com.
> To post to this group, send email to open-iscsi@googlegroups.com.
> Visit this group at https://groups.google.com/group/open-iscsi.
> For more options, visit https://groups.google.com/d/optout.




-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.