All of lore.kernel.org
 help / color / mirror / Atom feed
* inode security state and cluster file systems
@ 2011-02-18  3:08 Yuri L Volobuev
  2011-02-18 13:28 ` Stephen Smalley
  0 siblings, 1 reply; 16+ messages in thread
From: Yuri L Volobuev @ 2011-02-18  3:08 UTC (permalink / raw)
  To: SELinux

[-- Attachment #1: Type: text/plain, Size: 2646 bytes --]



Hi,

I'm a developer on the IBM General Parallel File System (GPFS) team.  We
are currently looking into implementing SELinux inode labeling support.  In
the process, we've run into some complications that have to do with the
semantics of the Linux LSM API.  We've discussed the issue briefly with
Eric Paris from RedHat, who has recommended that I bring up this topic on a
public list, since it concerns a larger issue of LSM interaction with
cluster file systems.  The idea here is not just to make a change for the
benefit of an out-of-tree file system, but rather improve the LSM API to be
friendlier to cluster file systems in general.

The issue has to do with the semantics of multi-node xattr updates.  The
desirable behavior is simple: a change in an inode security label (stored
as an xattr) made on nodeA should be visible on all other nodes on the next
access.  As far as I can tell, the current SELinux code would initialize
the inode security state on the first access (e.g. via
security_d_instantiate/inode_doinit_with_dentry), and from that point on
the cached security state is considered valid, until the inode is destroyed
or reused.  Any subsequent inode_doinit_with_dentry call would be a no-op,
since isec->initialized is true.  There's no way to clear 'initialized', as
far as I can see.  This works when all changes to the inode are local, and
a local setxattr call would update the inode security state.  However, if
the security label has been changed on another node, some mechanism is
needed to update the cached security state.  One could achieve this by
using security_inode_notifysecctx if the value of the security context is
known.  However, in the general case retrieving the context value would
require some knowledge about the implementation details of LSM (like the
name of the security label xattr), and such knowledge is currently kept
within LSM code, and arguably should remain so.  In other words, one would
have to resort to hacking.

To remedy this situation, a new API is proposed, courtesy of Eric Paris:

void security_inode_refresh_security(struct dentry *dentry);

The semantics would be similar to what SELinux inode_doinit provides: for
the SECURITY_FS_USE_XATTR case, inode security state would be set based on
the value of the security label fetched via getxattr -- even if the
security state is already initialized.  For other labeling behaviors, the
call could be a no-op if security is already initialized, and an equivalent
of inode_doinit otherwise.

Does this API look useful, in particular to other cluster file systems?

Thanks,

yuri

[-- Attachment #2: Type: text/html, Size: 2709 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18  3:08 inode security state and cluster file systems Yuri L Volobuev
@ 2011-02-18 13:28 ` Stephen Smalley
  2011-02-18 13:35   ` Stephen Smalley
  2011-02-18 16:15   ` Yuri L Volobuev
  0 siblings, 2 replies; 16+ messages in thread
From: Stephen Smalley @ 2011-02-18 13:28 UTC (permalink / raw)
  To: Yuri L Volobuev; +Cc: SELinux, Eric Paris, selinux

On Thu, 2011-02-17 at 21:08 -0600, Yuri L Volobuev wrote:
> Hi,
> 
> I'm a developer on the IBM General Parallel File System (GPFS) team.
> We are currently looking into implementing SELinux inode labeling
> support. In the process, we've run into some complications that have
> to do with the semantics of the Linux LSM API. We've discussed the
> issue briefly with Eric Paris from RedHat, who has recommended that I
> bring up this topic on a public list, since it concerns a larger issue
> of LSM interaction with cluster file systems. The idea here is not
> just to make a change for the benefit of an out-of-tree file system,
> but rather improve the LSM API to be friendlier to cluster file
> systems in general.
> 
> The issue has to do with the semantics of multi-node xattr updates.
> The desirable behavior is simple: a change in an inode security label
> (stored as an xattr) made on nodeA should be visible on all other
> nodes on the next access. As far as I can tell, the current SELinux
> code would initialize the inode security state on the first access
> (e.g. via security_d_instantiate/inode_doinit_with_dentry), and from
> that point on the cached security state is considered valid, until the
> inode is destroyed or reused. Any subsequent inode_doinit_with_dentry
> call would be a no-op, since isec->initialized is true. There's no way
> to clear 'initialized', as far as I can see. This works when all
> changes to the inode are local, and a local setxattr call would update
> the inode security state. However, if the security label has been
> changed on another node, some mechanism is needed to update the cached
> security state. One could achieve this by using
> security_inode_notifysecctx if the value of the security context is
> known. However, in the general case retrieving the context value would
> require some knowledge about the implementation details of LSM (like
> the name of the security label xattr), and such knowledge is currently
> kept within LSM code, and arguably should remain so. In other words,
> one would have to resort to hacking.

Isn't this what inode_getsecctx() is for?  So that on the node where the
setxattr() occurs, you can fetch the security context (without needing
to know the attribute name or whether it is even implemented via zero,
one, or many attributes), and then ship that context over the wire using
whatever protocol you like to the other nodes.  Then on the other nodes,
you can invoke inode_notifysecctx() as you said to update the context.
I think that is how it works for the labeled NFS support (not yet in
mainline).  Admittedly that is a simpler client/server model and not a
distributed cluster model.

> To remedy this situation, a new API is proposed, courtesy of Eric
> Paris:
> 
> void security_inode_refresh_security(struct dentry *dentry);
> 
> The semantics would be similar to what SELinux inode_doinit provides:
> for the SECURITY_FS_USE_XATTR case, inode security state would be set
> based on the value of the security label fetched via getxattr -- even
> if the security state is already initialized. For other labeling
> behaviors, the call could be a no-op if security is already
> initialized, and an equivalent of inode_doinit otherwise.
> 
> Does this API look useful, in particular to other cluster file
> systems?

How do you know when to call this interface?  And if you know to call
it, why don't you know what the new context is already?

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18 13:28 ` Stephen Smalley
@ 2011-02-18 13:35   ` Stephen Smalley
  2011-02-18 15:18     ` Eric Paris
  2011-02-18 16:15   ` Yuri L Volobuev
  1 sibling, 1 reply; 16+ messages in thread
From: Stephen Smalley @ 2011-02-18 13:35 UTC (permalink / raw)
  To: Yuri L Volobuev; +Cc: SELinux, Eric Paris, selinux

On Fri, 2011-02-18 at 08:28 -0500, Stephen Smalley wrote:
> On Thu, 2011-02-17 at 21:08 -0600, Yuri L Volobuev wrote:
> > Hi,
> > 
> > I'm a developer on the IBM General Parallel File System (GPFS) team.
> > We are currently looking into implementing SELinux inode labeling
> > support. In the process, we've run into some complications that have
> > to do with the semantics of the Linux LSM API. We've discussed the
> > issue briefly with Eric Paris from RedHat, who has recommended that I
> > bring up this topic on a public list, since it concerns a larger issue
> > of LSM interaction with cluster file systems. The idea here is not
> > just to make a change for the benefit of an out-of-tree file system,
> > but rather improve the LSM API to be friendlier to cluster file
> > systems in general.
> > 
> > The issue has to do with the semantics of multi-node xattr updates.
> > The desirable behavior is simple: a change in an inode security label
> > (stored as an xattr) made on nodeA should be visible on all other
> > nodes on the next access. As far as I can tell, the current SELinux
> > code would initialize the inode security state on the first access
> > (e.g. via security_d_instantiate/inode_doinit_with_dentry), and from
> > that point on the cached security state is considered valid, until the
> > inode is destroyed or reused. Any subsequent inode_doinit_with_dentry
> > call would be a no-op, since isec->initialized is true. There's no way
> > to clear 'initialized', as far as I can see. This works when all
> > changes to the inode are local, and a local setxattr call would update
> > the inode security state. However, if the security label has been
> > changed on another node, some mechanism is needed to update the cached
> > security state. One could achieve this by using
> > security_inode_notifysecctx if the value of the security context is
> > known. However, in the general case retrieving the context value would
> > require some knowledge about the implementation details of LSM (like
> > the name of the security label xattr), and such knowledge is currently
> > kept within LSM code, and arguably should remain so. In other words,
> > one would have to resort to hacking.
> 
> Isn't this what inode_getsecctx() is for?  So that on the node where the
> setxattr() occurs, you can fetch the security context (without needing
> to know the attribute name or whether it is even implemented via zero,
> one, or many attributes), and then ship that context over the wire using
> whatever protocol you like to the other nodes.  Then on the other nodes,
> you can invoke inode_notifysecctx() as you said to update the context.
> I think that is how it works for the labeled NFS support (not yet in
> mainline).  Admittedly that is a simpler client/server model and not a
> distributed cluster model.
> 
> > To remedy this situation, a new API is proposed, courtesy of Eric
> > Paris:
> > 
> > void security_inode_refresh_security(struct dentry *dentry);
> > 
> > The semantics would be similar to what SELinux inode_doinit provides:
> > for the SECURITY_FS_USE_XATTR case, inode security state would be set
> > based on the value of the security label fetched via getxattr -- even
> > if the security state is already initialized. For other labeling
> > behaviors, the call could be a no-op if security is already
> > initialized, and an equivalent of inode_doinit otherwise.
> > 
> > Does this API look useful, in particular to other cluster file
> > systems?
> 
> How do you know when to call this interface?  And if you know to call
> it, why don't you know what the new context is already?

It would also be useful to know how you handle uid/gid/mode/ACL updates.
Ideally we would follow a similar model for the security contexts.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18 13:35   ` Stephen Smalley
@ 2011-02-18 15:18     ` Eric Paris
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Paris @ 2011-02-18 15:18 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Yuri L Volobuev, SELinux, selinux, swhiteho

Adding GFS2 maintainer to thread.

On Fri, Feb 18, 2011 at 8:35 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Fri, 2011-02-18 at 08:28 -0500, Stephen Smalley wrote:
>> On Thu, 2011-02-17 at 21:08 -0600, Yuri L Volobuev wrote:
>> > Hi,
>> >
>> > I'm a developer on the IBM General Parallel File System (GPFS) team.
>> > We are currently looking into implementing SELinux inode labeling
>> > support. In the process, we've run into some complications that have
>> > to do with the semantics of the Linux LSM API. We've discussed the
>> > issue briefly with Eric Paris from RedHat, who has recommended that I
>> > bring up this topic on a public list, since it concerns a larger issue
>> > of LSM interaction with cluster file systems. The idea here is not
>> > just to make a change for the benefit of an out-of-tree file system,
>> > but rather improve the LSM API to be friendlier to cluster file
>> > systems in general.
>> >
>> > The issue has to do with the semantics of multi-node xattr updates.
>> > The desirable behavior is simple: a change in an inode security label
>> > (stored as an xattr) made on nodeA should be visible on all other
>> > nodes on the next access. As far as I can tell, the current SELinux
>> > code would initialize the inode security state on the first access
>> > (e.g. via security_d_instantiate/inode_doinit_with_dentry), and from
>> > that point on the cached security state is considered valid, until the
>> > inode is destroyed or reused. Any subsequent inode_doinit_with_dentry
>> > call would be a no-op, since isec->initialized is true. There's no way
>> > to clear 'initialized', as far as I can see. This works when all
>> > changes to the inode are local, and a local setxattr call would update
>> > the inode security state. However, if the security label has been
>> > changed on another node, some mechanism is needed to update the cached
>> > security state. One could achieve this by using
>> > security_inode_notifysecctx if the value of the security context is
>> > known. However, in the general case retrieving the context value would
>> > require some knowledge about the implementation details of LSM (like
>> > the name of the security label xattr), and such knowledge is currently
>> > kept within LSM code, and arguably should remain so. In other words,
>> > one would have to resort to hacking.
>>
>> Isn't this what inode_getsecctx() is for?  So that on the node where the
>> setxattr() occurs, you can fetch the security context (without needing
>> to know the attribute name or whether it is even implemented via zero,
>> one, or many attributes), and then ship that context over the wire using
>> whatever protocol you like to the other nodes.  Then on the other nodes,
>> you can invoke inode_notifysecctx() as you said to update the context.
>> I think that is how it works for the labeled NFS support (not yet in
>> mainline).  Admittedly that is a simpler client/server model and not a
>> distributed cluster model.
>>
>> > To remedy this situation, a new API is proposed, courtesy of Eric
>> > Paris:
>> >
>> > void security_inode_refresh_security(struct dentry *dentry);
>> >
>> > The semantics would be similar to what SELinux inode_doinit provides:
>> > for the SECURITY_FS_USE_XATTR case, inode security state would be set
>> > based on the value of the security label fetched via getxattr -- even
>> > if the security state is already initialized. For other labeling
>> > behaviors, the call could be a no-op if security is already
>> > initialized, and an equivalent of inode_doinit otherwise.
>> >
>> > Does this API look useful, in particular to other cluster file
>> > systems?
>>
>> How do you know when to call this interface?  And if you know to call
>> it, why don't you know what the new context is already?
>
> It would also be useful to know how you handle uid/gid/mode/ACL updates.
> Ideally we would follow a similar model for the security contexts.

It sounds to me from reading the GFS2 bugzilla like the GFS2
->getxattr() call is cluster coherent.  They explictly have a call to
flush cached ACLs when one changes somewhere and plan to use that same
explicit mechanism to flush the 'cached' sid.  I don't know how they
handle uid/gid/etc changes....

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18 13:28 ` Stephen Smalley
  2011-02-18 13:35   ` Stephen Smalley
@ 2011-02-18 16:15   ` Yuri L Volobuev
  2011-02-18 16:25     ` Stephen Smalley
  1 sibling, 1 reply; 16+ messages in thread
From: Yuri L Volobuev @ 2011-02-18 16:15 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux, SELinux, swhiteho

[-- Attachment #1: Type: text/plain, Size: 5413 bytes --]


> > The issue has to do with the semantics of multi-node xattr updates.
> > The desirable behavior is simple: a change in an inode security label
> > (stored as an xattr) made on nodeA should be visible on all other
> > nodes on the next access. As far as I can tell, the current SELinux
> > code would initialize the inode security state on the first access
> > (e.g. via security_d_instantiate/inode_doinit_with_dentry), and from
> > that point on the cached security state is considered valid, until the
> > inode is destroyed or reused. Any subsequent inode_doinit_with_dentry
> > call would be a no-op, since isec->initialized is true. There's no way
> > to clear 'initialized', as far as I can see. This works when all
> > changes to the inode are local, and a local setxattr call would update
> > the inode security state. However, if the security label has been
> > changed on another node, some mechanism is needed to update the cached
> > security state. One could achieve this by using
> > security_inode_notifysecctx if the value of the security context is
> > known. However, in the general case retrieving the context value would
> > require some knowledge about the implementation details of LSM (like
> > the name of the security label xattr), and such knowledge is currently
> > kept within LSM code, and arguably should remain so. In other words,
> > one would have to resort to hacking.
>
> Isn't this what inode_getsecctx() is for?  So that on the node where the
> setxattr() occurs, you can fetch the security context (without needing
> to know the attribute name or whether it is even implemented via zero,
> one, or many attributes), and then ship that context over the wire using
> whatever protocol you like to the other nodes.  Then on the other nodes,
> you can invoke inode_notifysecctx() as you said to update the context.
> I think that is how it works for the labeled NFS support (not yet in
> mainline).  Admittedly that is a simpler client/server model and not a
> distributed cluster model.

In principle, it's possible to use the "push mode": the node that changes
the xattr would notify others, and since it has all of the information
about the xattr at hand, on the receiving side inode_notifysecctx() could
be used.  The "push mode" has inherent problematic properties though.  The
sender could die before notifying all nodes that have legitimate interest
in seeing the update, which would make it tricky to commit the xattr change
to disk and notify all interested nodes without the risk of an
inconsistency.  Doing this would require a synchronous RPC multicast,
possibly to a large number of nodes, and that's not a good thing for
performance and scalability.  For a client-server model, this makes sense,
for a cluster file system, not necessarily.

Another approach to managing metadata consistency is the "pull mode": the
node doing the update would acquire a distributed lock of an appropriate
strength, which would make other nodes relinquish any conflicting locks
they may have had and invalidate metadata protected by those locks.  The
writer would then make a (logged) metadata update.  If another node needs
to access the metadata object in question again, it would acquire the
appropriate distributed lock (which would make the writer
relinquish/downgrade its lock and flush corresponding dirty objects to
disk), and read the metadata object from disk.  This is the mode that GPFS
uses for metadata updates, e.g. inode attribute (mtime, uid, etc) changes.
On Linux, struct inode is transparent, so the file system code could mark
its content as invalid when relinquishing the lock that protects it (and
mark any corresponding dentry as needing revalidation), and then update it
with the up-to-date content when a new stat or d_revalidate arrives.  One
can't do the same with the inode security state, at present.

> > To remedy this situation, a new API is proposed, courtesy of Eric
> > Paris:
> >
> > void security_inode_refresh_security(struct dentry *dentry);
> >
> > The semantics would be similar to what SELinux inode_doinit provides:
> > for the SECURITY_FS_USE_XATTR case, inode security state would be set
> > based on the value of the security label fetched via getxattr -- even
> > if the security state is already initialized. For other labeling
> > behaviors, the call could be a no-op if security is already
> > initialized, and an equivalent of inode_doinit otherwise.
> >
> > Does this API look useful, in particular to other cluster file
> > systems?
>
> How do you know when to call this interface?  And if you know to call
> it, why don't you know what the new context is already?

The logical place to make this call would be at d_revalidate time.  The new
value of the security context is not readily available at that time, as
described above.  Technically speaking, the file system code could know the
new context -- it can do a getxattr, which would return the up-to-date
label content.  However, this would require knowing the security label
xattr name, which is not readily available in RHEL6.  (I actually had the
code working with correct semantics on RHEL5, which has
security_inode_xattr_getsuffix(), but it would be fair to say that it was
hacking around the API rather than leveraging it as intended).

Hope it makes things clearer.

yuri

[-- Attachment #2: Type: text/html, Size: 6077 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18 16:15   ` Yuri L Volobuev
@ 2011-02-18 16:25     ` Stephen Smalley
  2011-02-18 19:07       ` Casey Schaufler
  2011-02-18 20:42       ` Yuri L Volobuev
  0 siblings, 2 replies; 16+ messages in thread
From: Stephen Smalley @ 2011-02-18 16:25 UTC (permalink / raw)
  To: Yuri L Volobuev; +Cc: selinux, SELinux, swhiteho, Eric Paris

On Fri, 2011-02-18 at 10:15 -0600, Yuri L Volobuev wrote:
> The logical place to make this call would be at d_revalidate time.
>  The new value of the security context is not readily available at
> that time, as described above.  Technically speaking, the file system
> code could know the new context -- it can do a getxattr, which would
> return the up-to-date label content.  However, this would require
> knowing the security label xattr name, which is not readily available
> in RHEL6.  (I actually had the code working with correct semantics on
> RHEL5, which has security_inode_xattr_getsuffix(), but it would be
> fair to say that it was hacking around the API rather than leveraging
> it as intended).
> 
> Hope it makes things clearer.

Yes, thanks.  One lingering concern then is that we revalidate
permissions on read/write via security_file_permission ->
selinux_file_permission.  Within selinux_file_permission, we check
whether the task SID, inode SID, or policy has changed since the file
was opened, and if so, we recheck permissions.  If you only make the
call at d_revalidate time, then we'll only update the inode sid on next
lookup and thus ongoing reads/writes on an already open file will
continue to succeed even if they should be denied by policy until some
process on that node attempts another lookup.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18 16:25     ` Stephen Smalley
@ 2011-02-18 19:07       ` Casey Schaufler
  2011-02-18 20:42       ` Yuri L Volobuev
  1 sibling, 0 replies; 16+ messages in thread
From: Casey Schaufler @ 2011-02-18 19:07 UTC (permalink / raw)
  To: Yuri L Volobuev; +Cc: Stephen Smalley, selinux, SELinux, swhiteho, Eric Paris

On 2/18/2011 8:25 AM, Stephen Smalley wrote:
> On Fri, 2011-02-18 at 10:15 -0600, Yuri L Volobuev wrote:
>> The logical place to make this call would be at d_revalidate time.
>>  The new value of the security context is not readily available at
>> that time, as described above.  Technically speaking, the file system
>> code could know the new context -- it can do a getxattr, which would
>> return the up-to-date label content.  However, this would require
>> knowing the security label xattr name, which is not readily available
>> in RHEL6.  (I actually had the code working with correct semantics on
>> RHEL5, which has security_inode_xattr_getsuffix(), but it would be
>> fair to say that it was hacking around the API rather than leveraging
>> it as intended).
>>
>> Hope it makes things clearer.
> Yes, thanks.  One lingering concern then is that we revalidate
> permissions on read/write via security_file_permission ->
> selinux_file_permission.  Within selinux_file_permission, we check
> whether the task SID, inode SID, or policy has changed since the file
> was opened, and if so, we recheck permissions.  If you only make the
> call at d_revalidate time, then we'll only update the inode sid on next
> lookup and thus ongoing reads/writes on an already open file will
> continue to succeed even if they should be denied by policy until some
> process on that node attempts another lookup.

Would you be so kind as to move this discussion to
linux-security-module@vger.kernel.org so as to include
the LSM community as a whole?

Thank you.


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18 16:25     ` Stephen Smalley
  2011-02-18 19:07       ` Casey Schaufler
@ 2011-02-18 20:42       ` Yuri L Volobuev
  2011-02-18 22:39         ` Eric Paris
  1 sibling, 1 reply; 16+ messages in thread
From: Yuri L Volobuev @ 2011-02-18 20:42 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Eric Paris, SELinux, selinux, swhiteho

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]


> > The logical place to make this call would be at d_revalidate time.
> >  The new value of the security context is not readily available at
> > that time, as described above.  Technically speaking, the file system
> > code could know the new context -- it can do a getxattr, which would
> > return the up-to-date label content.  However, this would require
> > knowing the security label xattr name, which is not readily available
> > in RHEL6.  (I actually had the code working with correct semantics on
> > RHEL5, which has security_inode_xattr_getsuffix(), but it would be
> > fair to say that it was hacking around the API rather than leveraging
> > it as intended).
> >
> > Hope it makes things clearer.
>
> Yes, thanks.  One lingering concern then is that we revalidate
> permissions on read/write via security_file_permission ->
> selinux_file_permission.  Within selinux_file_permission, we check
> whether the task SID, inode SID, or policy has changed since the file
> was opened, and if so, we recheck permissions.  If you only make the
> call at d_revalidate time, then we'll only update the inode sid on next
> lookup and thus ongoing reads/writes on an already open file will
> continue to succeed even if they should be denied by policy until some
> process on that node attempts another lookup.

Interesting.  With plain Unix semantics, a permission check would be done
at file open time, and wouldn't be repeated at read/write time.  So there's
no attempt by the kernel proper to revalidate the associated dentry/inode
prior to calling read/write fop.  So, if the file owner were to change (for
example), the local inode would not necessarily have the correct owner
prior to a write, which is probably important.  So this is something that
transcends the issue of inode security state per se.

Another possible approach here, as noted by GFS folks, is to invalidate
inode security state when a lock protecting it is lost.  GPFS could
certainly take that approach as well (it's done already for "regular" inode
state).  In this case, it sounds like kernel/LSM code would be responsible
to refreshing the inode security state when it's needed (although fs code
could also drive this explicitly at d_revalidate time).  I'd be fine with
that approach.

yuri

[-- Attachment #2: Type: text/html, Size: 2661 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18 20:42       ` Yuri L Volobuev
@ 2011-02-18 22:39         ` Eric Paris
  2011-02-18 22:40           ` Eric Paris
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Paris @ 2011-02-18 22:39 UTC (permalink / raw)
  To: Yuri L Volobuev
  Cc: Stephen Smalley, SELinux, selinux, swhiteho, Mark Fasheh, Joel Becker

On Fri, Feb 18, 2011 at 3:42 PM, Yuri L Volobuev <volobuev@us.ibm.com> wrote:
>> > The logical place to make this call would be at d_revalidate time.
>> >  The new value of the security context is not readily available at
>> > that time, as described above.  Technically speaking, the file system
>> > code could know the new context -- it can do a getxattr, which would
>> > return the up-to-date label content.  However, this would require
>> > knowing the security label xattr name, which is not readily available
>> > in RHEL6.  (I actually had the code working with correct semantics on
>> > RHEL5, which has security_inode_xattr_getsuffix(), but it would be
>> > fair to say that it was hacking around the API rather than leveraging
>> > it as intended).
>> >
>> > Hope it makes things clearer.
>>
>> Yes, thanks.  One lingering concern then is that we revalidate
>> permissions on read/write via security_file_permission ->
>> selinux_file_permission.  Within selinux_file_permission, we check
>> whether the task SID, inode SID, or policy has changed since the file
>> was opened, and if so, we recheck permissions.  If you only make the
>> call at d_revalidate time, then we'll only update the inode sid on next
>> lookup and thus ongoing reads/writes on an already open file will
>> continue to succeed even if they should be denied by policy until some
>> process on that node attempts another lookup.
>
> Interesting.  With plain Unix semantics, a permission check would be done at
> file open time, and wouldn't be repeated at read/write time.  So there's no
> attempt by the kernel proper to revalidate the associated dentry/inode prior
> to calling read/write fop.  So, if the file owner were to change (for
> example), the local inode would not necessarily have the correct owner prior
> to a write, which is probably important.  So this is something that
> transcends the issue of inode security state per se.
>
> Another possible approach here, as noted by GFS folks, is to invalidate
> inode security state when a lock protecting it is lost.  GPFS could
> certainly take that approach as well (it's done already for "regular" inode
> state).  In this case, it sounds like kernel/LSM code would be responsible
> to refreshing the inode security state when it's needed (although fs code
> could also drive this explicitly at d_revalidate time).  I'd be fine with
> that approach.

I'm thinking about the GFS suggested approach of just 'invalidating'
the inode security label.  It presents us with 2 problems I can see up
front.  It really seems like filesystems would be upset when the next
read/write calls starts running down the ->getxattr() code.  Even if
they can handle the locking and such of having getxattr called at
random times, most of those calls into the LSM only come with an
inode.  But we need a dentry to make said ->getxattr calls.   (I
believe this is a broken VFS problem, but the VFS requires a dentry)

If we made the invalidation point an explicit refresh of the label at
least the filesystems would be able to control both of those
problems.....

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18 22:39         ` Eric Paris
@ 2011-02-18 22:40           ` Eric Paris
  2011-02-20 20:16             ` Casey Schaufler
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Paris @ 2011-02-18 22:40 UTC (permalink / raw)
  To: Yuri L Volobuev
  Cc: Stephen Smalley, SELinux, selinux, swhiteho, Mark Fasheh,
	Joel Becker, LSM List

Resending adding linux-security-module to the thread.  Sorry if you
feel lost, you can try to look through the SELinux list history, but
some of this conversation didn't appear their since people are not
members.....

I can try to write a review if anyone needs/wants it....

On Fri, Feb 18, 2011 at 5:39 PM, Eric Paris <eparis@parisplace.org> wrote:
> On Fri, Feb 18, 2011 at 3:42 PM, Yuri L Volobuev <volobuev@us.ibm.com> wrote:
>>> > The logical place to make this call would be at d_revalidate time.
>>> >  The new value of the security context is not readily available at
>>> > that time, as described above.  Technically speaking, the file system
>>> > code could know the new context -- it can do a getxattr, which would
>>> > return the up-to-date label content.  However, this would require
>>> > knowing the security label xattr name, which is not readily available
>>> > in RHEL6.  (I actually had the code working with correct semantics on
>>> > RHEL5, which has security_inode_xattr_getsuffix(), but it would be
>>> > fair to say that it was hacking around the API rather than leveraging
>>> > it as intended).
>>> >
>>> > Hope it makes things clearer.
>>>
>>> Yes, thanks.  One lingering concern then is that we revalidate
>>> permissions on read/write via security_file_permission ->
>>> selinux_file_permission.  Within selinux_file_permission, we check
>>> whether the task SID, inode SID, or policy has changed since the file
>>> was opened, and if so, we recheck permissions.  If you only make the
>>> call at d_revalidate time, then we'll only update the inode sid on next
>>> lookup and thus ongoing reads/writes on an already open file will
>>> continue to succeed even if they should be denied by policy until some
>>> process on that node attempts another lookup.
>>
>> Interesting.  With plain Unix semantics, a permission check would be done at
>> file open time, and wouldn't be repeated at read/write time.  So there's no
>> attempt by the kernel proper to revalidate the associated dentry/inode prior
>> to calling read/write fop.  So, if the file owner were to change (for
>> example), the local inode would not necessarily have the correct owner prior
>> to a write, which is probably important.  So this is something that
>> transcends the issue of inode security state per se.
>>
>> Another possible approach here, as noted by GFS folks, is to invalidate
>> inode security state when a lock protecting it is lost.  GPFS could
>> certainly take that approach as well (it's done already for "regular" inode
>> state).  In this case, it sounds like kernel/LSM code would be responsible
>> to refreshing the inode security state when it's needed (although fs code
>> could also drive this explicitly at d_revalidate time).  I'd be fine with
>> that approach.
>
> I'm thinking about the GFS suggested approach of just 'invalidating'
> the inode security label.  It presents us with 2 problems I can see up
> front.  It really seems like filesystems would be upset when the next
> read/write calls starts running down the ->getxattr() code.  Even if
> they can handle the locking and such of having getxattr called at
> random times, most of those calls into the LSM only come with an
> inode.  But we need a dentry to make said ->getxattr calls.   (I
> believe this is a broken VFS problem, but the VFS requires a dentry)
>
> If we made the invalidation point an explicit refresh of the label at
> least the filesystems would be able to control both of those
> problems.....
>
> -Eric
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-18 22:40           ` Eric Paris
@ 2011-02-20 20:16             ` Casey Schaufler
  2011-02-21 19:56               ` Eric Paris
  0 siblings, 1 reply; 16+ messages in thread
From: Casey Schaufler @ 2011-02-20 20:16 UTC (permalink / raw)
  To: Eric Paris
  Cc: Yuri L Volobuev, Stephen Smalley, SELinux, selinux, swhiteho,
	Mark Fasheh, Joel Becker, LSM List, Casey Schaufler

On 2/18/2011 2:40 PM, Eric Paris wrote:
> Resending adding linux-security-module to the thread.  Sorry if you
> feel lost, you can try to look through the SELinux list history, but
> some of this conversation didn't appear their since people are not
> members.....
>
> I can try to write a review if anyone needs/wants it....
>
> On Fri, Feb 18, 2011 at 5:39 PM, Eric Paris <eparis@parisplace.org> wrote:
>> On Fri, Feb 18, 2011 at 3:42 PM, Yuri L Volobuev <volobuev@us.ibm.com> wrote:
>>>>> The logical place to make this call would be at d_revalidate time.
>>>>>  The new value of the security context is not readily available at
>>>>> that time, as described above.  Technically speaking, the file system
>>>>> code could know the new context -- it can do a getxattr, which would
>>>>> return the up-to-date label content.  However, this would require
>>>>> knowing the security label xattr name, which is not readily available
>>>>> in RHEL6.  (I actually had the code working with correct semantics on
>>>>> RHEL5, which has security_inode_xattr_getsuffix(), but it would be
>>>>> fair to say that it was hacking around the API rather than leveraging
>>>>> it as intended).
>>>>>
>>>>> Hope it makes things clearer.
>>>> Yes, thanks.  One lingering concern then is that we revalidate
>>>> permissions on read/write via security_file_permission ->
>>>> selinux_file_permission.  Within selinux_file_permission, we check
>>>> whether the task SID, inode SID, or policy has changed since the file
>>>> was opened, and if so, we recheck permissions.  If you only make the
>>>> call at d_revalidate time, then we'll only update the inode sid on next
>>>> lookup and thus ongoing reads/writes on an already open file will
>>>> continue to succeed even if they should be denied by policy until some
>>>> process on that node attempts another lookup.
>>> Interesting.  With plain Unix semantics, a permission check would be done at
>>> file open time, and wouldn't be repeated at read/write time.  So there's no
>>> attempt by the kernel proper to revalidate the associated dentry/inode prior
>>> to calling read/write fop.  So, if the file owner were to change (for
>>> example), the local inode would not necessarily have the correct owner prior
>>> to a write, which is probably important.  So this is something that
>>> transcends the issue of inode security state per se.
>>>
>>> Another possible approach here, as noted by GFS folks, is to invalidate
>>> inode security state when a lock protecting it is lost.  GPFS could
>>> certainly take that approach as well (it's done already for "regular" inode
>>> state).  In this case, it sounds like kernel/LSM code would be responsible
>>> to refreshing the inode security state when it's needed (although fs code
>>> could also drive this explicitly at d_revalidate time).  I'd be fine with
>>> that approach.
>> I'm thinking about the GFS suggested approach of just 'invalidating'
>> the inode security label.  It presents us with 2 problems I can see up
>> front.  It really seems like filesystems would be upset when the next
>> read/write calls starts running down the ->getxattr() code.  Even if
>> they can handle the locking and such of having getxattr called at
>> random times, most of those calls into the LSM only come with an
>> inode.  But we need a dentry to make said ->getxattr calls.   (I
>> believe this is a broken VFS problem, but the VFS requires a dentry)
>>
>> If we made the invalidation point an explicit refresh of the label at
>> least the filesystems would be able to control both of those
>> problems.....

So far the only case that has been discussed is one in which there
is one security attribute on the file. While we don't have it yet there
are a couple of efforts underway to support multiple concurrent LSMs.
It is also plausible for an LSM to use more than one security attribute
on a given file. An approach based on a secid interface may not work
in either scenario. If you want a comparative example, look at directory
default ACLs, which are important security information, but are nor used
to make access decisions on the object to which they are attached.


>> -Eric
>>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
>
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-20 20:16             ` Casey Schaufler
@ 2011-02-21 19:56               ` Eric Paris
  2011-02-22  3:57                 ` Casey Schaufler
  2011-02-22 15:25                 ` Yuri L Volobuev
  0 siblings, 2 replies; 16+ messages in thread
From: Eric Paris @ 2011-02-21 19:56 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Yuri L Volobuev, Stephen Smalley, SELinux, selinux, swhiteho,
	Mark Fasheh, Joel Becker, LSM List

On Sun, Feb 20, 2011 at 3:16 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 2/18/2011 2:40 PM, Eric Paris wrote:

>>> If we made the invalidation point an explicit refresh of the label at
>>> least the filesystems would be able to control both of those
>>> problems.....
>
> So far the only case that has been discussed is one in which there
> is one security attribute on the file. While we don't have it yet there
> are a couple of efforts underway to support multiple concurrent LSMs.
> It is also plausible for an LSM to use more than one security attribute
> on a given file. An approach based on a secid interface may not work
> in either scenario. If you want a comparative example, look at directory
> default ACLs, which are important security information, but are nor used
> to make access decisions on the object to which they are attached.

I'm still leaning towards a solution in which the filesystem needs to
tell the LSM that it's labels are invalid (I suggest that be done on
any change in the security.* namespace) and that same call needs to
cause the LSM to refresh its labels.  I seem to think that such a call
would work just fine with any LSM stacker or even
multi-attribute/multi-label LSM (which uses the security.* namespace).

The biggest problem is if the 3 cluster filesystems in question can
handle an interface that requires a dentry and if they can handle the
fact that we do permissions checks on read/write and would like those
operations to start failing immediately if another node changed the
label.  I really don't see any other way to solve the problem....

-Eric

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-21 19:56               ` Eric Paris
@ 2011-02-22  3:57                 ` Casey Schaufler
  2011-02-22 15:25                 ` Yuri L Volobuev
  1 sibling, 0 replies; 16+ messages in thread
From: Casey Schaufler @ 2011-02-22  3:57 UTC (permalink / raw)
  To: Eric Paris
  Cc: Yuri L Volobuev, Stephen Smalley, SELinux, selinux, swhiteho,
	Mark Fasheh, Joel Becker, LSM List, Casey Schaufler

On 2/21/2011 11:56 AM, Eric Paris wrote:
> On Sun, Feb 20, 2011 at 3:16 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 2/18/2011 2:40 PM, Eric Paris wrote:
>>>> If we made the invalidation point an explicit refresh of the label at
>>>> least the filesystems would be able to control both of those
>>>> problems.....
>> So far the only case that has been discussed is one in which there
>> is one security attribute on the file. While we don't have it yet there
>> are a couple of efforts underway to support multiple concurrent LSMs.
>> It is also plausible for an LSM to use more than one security attribute
>> on a given file. An approach based on a secid interface may not work
>> in either scenario. If you want a comparative example, look at directory
>> default ACLs, which are important security information, but are nor used
>> to make access decisions on the object to which they are attached.
> I'm still leaning towards a solution in which the filesystem needs to
> tell the LSM that it's labels are invalid (I suggest that be done on
> any change in the security.* namespace) and that same call needs to
> cause the LSM to refresh its labels.  I seem to think that such a call
> would work just fine with any LSM stacker or even
> multi-attribute/multi-label LSM (which uses the security.* namespace).

Yes, this seems completely reasonable and equally annoying for all
concerned. I think that it would be a good idea to pass the name of
the attribute changed (e.g. "SMACK64") so that the LSM(s) can decide
if it(they) need take action.


> The biggest problem is if the 3 cluster filesystems in question can
> handle an interface that requires a dentry

If they can't I think that the LSM can get the dentry from an inode,
although it is not convenient. Hmm. A brief look at code indicates
that this may not be easy for the LSM. So it looks as if the filesystem
might have to pass dentrys.

>  and if they can handle the
> fact that we do permissions checks on read/write and would like those
> operations to start failing immediately if another node changed the
> label.  I really don't see any other way to solve the problem....

Distributed systems are tricky. That's one reason I work in security,
it is so much simpler.

> -Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-21 19:56               ` Eric Paris
  2011-02-22  3:57                 ` Casey Schaufler
@ 2011-02-22 15:25                 ` Yuri L Volobuev
  2011-02-22 16:05                   ` Eric Paris
  1 sibling, 1 reply; 16+ messages in thread
From: Yuri L Volobuev @ 2011-02-22 15:25 UTC (permalink / raw)
  To: Eric Paris
  Cc: Casey Schaufler, Joel Becker, LSM List, Mark Fasheh,
	Stephen Smalley, selinux, SELinux, swhiteho

[-- Attachment #1: Type: text/plain, Size: 2812 bytes --]


> >>> If we made the invalidation point an explicit refresh of the label at
> >>> least the filesystems would be able to control both of those
> >>> problems.....
> >
> > So far the only case that has been discussed is one in which there
> > is one security attribute on the file. While we don't have it yet there
> > are a couple of efforts underway to support multiple concurrent LSMs.
> > It is also plausible for an LSM to use more than one security attribute
> > on a given file. An approach based on a secid interface may not work
> > in either scenario. If you want a comparative example, look at
directory
> > default ACLs, which are important security information, but are nor
used
> > to make access decisions on the object to which they are attached.
>
> I'm still leaning towards a solution in which the filesystem needs to
> tell the LSM that it's labels are invalid (I suggest that be done on
> any change in the security.* namespace) and that same call needs to
> cause the LSM to refresh its labels.  I seem to think that such a call
> would work just fine with any LSM stacker or even
> multi-attribute/multi-label LSM (which uses the security.* namespace).

In the "pull" model that GFS2, OCFS2, and GPFS apparently follow, on a
given node the file system code doesn't know that an xattr in a security.*
namespace got modified on another node.  All that is known is that the lock
protecting inode xattrs has been lost, so now everything protected by this
lock is invalid.  The next time this information is needed, it has to be
re-initialized (e.g. by reading data from disk).  The security attribute
label may or may not have changed, that is not known at that point, but the
inode security state has to be re-initialized in case it did.  So it sounds
like all distributed file systems would benefit from an LSM API call that
invalidates inode security state.

I think the tricky part here is how/when to drive inode security state
refresh, once it has been invalidated.  File system code could do this
easily at dentry revalidation time, possibly even using existing API, like
security_d_instantiate (provided it does a full re-init, using getxattr, if
it sees that the inode state has been invalidated).  A dentry is
conveniently available at this juncture.
Permission checking prior to a write on an already open file is a somewhat
different problem.  In the current code, the permission check is made
before file system code gets involved, so the call to re-init inode
security state has to be made in VFS or LSM code, before the permission
check.  It would probably be easier/more logical to do this in
security_file_permission.  Since the latter is passed struct file *file, a
dentry could be easily obtained as file->f_path.dentry.

yuri

[-- Attachment #2: Type: text/html, Size: 3173 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-22 15:25                 ` Yuri L Volobuev
@ 2011-02-22 16:05                   ` Eric Paris
  2011-02-22 22:17                     ` Yuri L Volobuev
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Paris @ 2011-02-22 16:05 UTC (permalink / raw)
  To: Yuri L Volobuev
  Cc: Eric Paris, Casey Schaufler, Joel Becker, LSM List, Mark Fasheh,
	Stephen Smalley, selinux, SELinux, swhiteho

On Tue, 2011-02-22 at 09:25 -0600, Yuri L Volobuev wrote:
> > >>> If we made the invalidation point an explicit refresh of the
> label at
> > >>> least the filesystems would be able to control both of those
> > >>> problems.....
> > >
> > > So far the only case that has been discussed is one in which there
> > > is one security attribute on the file. While we don't have it yet
> there
> > > are a couple of efforts underway to support multiple concurrent
> LSMs.
> > > It is also plausible for an LSM to use more than one security
> attribute
> > > on a given file. An approach based on a secid interface may not
> work
> > > in either scenario. If you want a comparative example, look at
> directory
> > > default ACLs, which are important security information, but are
> nor used
> > > to make access decisions on the object to which they are attached.
> > 
> > I'm still leaning towards a solution in which the filesystem needs
> to
> > tell the LSM that it's labels are invalid (I suggest that be done on
> > any change in the security.* namespace) and that same call needs to
> > cause the LSM to refresh its labels.  I seem to think that such a
> call
> > would work just fine with any LSM stacker or even
> > multi-attribute/multi-label LSM (which uses the security.*
> namespace).
> 
> In the "pull" model that GFS2, OCFS2, and GPFS apparently follow, on a
> given node the file system code doesn't know that an xattr in a
> security.* namespace got modified on another node.  All that is known
> is that the lock protecting inode xattrs has been lost, so now
> everything protected by this lock is invalid.  The next time this
> information is needed, it has to be re-initialized (e.g. by reading
> data from disk).  The security attribute label may or may not have
> changed, that is not known at that point, but the inode security state
> has to be re-initialized in case it did.  So it sounds like all
> distributed file systems would benefit from an LSM API call that
> invalidates inode security state.
> 
> I think the tricky part here is how/when to drive inode security state
> refresh, once it has been invalidated.  File system code could do this
> easily at dentry revalidation time, possibly even using existing API,
> like security_d_instantiate (provided it does a full re-init, using
> getxattr, if it sees that the inode state has been invalidated).  A
> dentry is conveniently available at this juncture.  
> Permission checking prior to a write on an already open file is a
> somewhat different problem.  In the current code, the permission check
> is made before file system code gets involved, so the call to re-init
> inode security state has to be made in VFS or LSM code, before the
> permission check.  It would probably be easier/more logical to do this
> in security_file_permission.  Since the latter is passed struct file
> *file, a dentry could be easily obtained as file->f_path.dentry.

The two problems I see with doing the refresh 'when needed' is that FS
code is almost certainly not designed to have a ->getxattr call during a
read/write and not all permissions checks have a dentry.  Namely any
calls to fs/namei.c::inode_permission() are going to end up in the LSM
without a dentry and with an 'invalid' security struct.  I feel like the
invalidate and refresh need to be done at the same time....

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: inode security state and cluster file systems
  2011-02-22 16:05                   ` Eric Paris
@ 2011-02-22 22:17                     ` Yuri L Volobuev
  0 siblings, 0 replies; 16+ messages in thread
From: Yuri L Volobuev @ 2011-02-22 22:17 UTC (permalink / raw)
  To: Eric Paris
  Cc: Casey Schaufler, Eric Paris, Joel Becker, LSM List, Mark Fasheh,
	Stephen Smalley, SELinux, selinux, swhiteho

[-- Attachment #1: Type: text/plain, Size: 4414 bytes --]


> > In the "pull" model that GFS2, OCFS2, and GPFS apparently follow, on a
> > given node the file system code doesn't know that an xattr in a
> > security.* namespace got modified on another node.  All that is known
> > is that the lock protecting inode xattrs has been lost, so now
> > everything protected by this lock is invalid.  The next time this
> > information is needed, it has to be re-initialized (e.g. by reading
> > data from disk).  The security attribute label may or may not have
> > changed, that is not known at that point, but the inode security state
> > has to be re-initialized in case it did.  So it sounds like all
> > distributed file systems would benefit from an LSM API call that
> > invalidates inode security state.
> >
> > I think the tricky part here is how/when to drive inode security state
> > refresh, once it has been invalidated.  File system code could do this
> > easily at dentry revalidation time, possibly even using existing API,
> > like security_d_instantiate (provided it does a full re-init, using
> > getxattr, if it sees that the inode state has been invalidated).  A
> > dentry is conveniently available at this juncture.
> > Permission checking prior to a write on an already open file is a
> > somewhat different problem.  In the current code, the permission check
> > is made before file system code gets involved, so the call to re-init
> > inode security state has to be made in VFS or LSM code, before the
> > permission check.  It would probably be easier/more logical to do this
> > in security_file_permission.  Since the latter is passed struct file
> > *file, a dentry could be easily obtained as file->f_path.dentry.
>
> The two problems I see with doing the refresh 'when needed' is that FS
> code is almost certainly not designed to have a ->getxattr call during a
> read/write and not all permissions checks have a dentry.  Namely any
> calls to fs/namei.c::inode_permission() are going to end up in the LSM
> without a dentry and with an 'invalid' security struct.  I feel like the
> invalidate and refresh need to be done at the same time....

The invalidate and refresh can't really be done at the same time.  The
invalidate would happen at a point where a node has to relinquish a
distributed lock.  If in the process of relinquishing a lock one tried to
synchronously run an operation that requires the same lock (getxattr), that
would deadlock.  So the refresh has to come at a later point.

I think the scope of the "when to revalidate" problem here is not as large
as it may seem.  Cluster file systems already have the same
invalidation/revalidation problem with the struct inode content.  In most
code paths, a valid dentry is needed to get things going, and if the dentry
has been marked as needing revalidation, it'll be revalidated before the
first use.  The specific case of security_file_permission() being called
from rw_verify_area() would be an exception, since on vanilla Unix no
permission checking would be done prior to a read/write on an already open
file.  I don't think a getxattr() call from inside rw_verify_area() would
be a problem for file system code, since the file system-specific code
hasn't been entered yet.  It would be somewhat difficult to verify that all
special cases of this nature are covered, however.

It is not that hard to look up _a_ dentry given an inode pointer, one just
has to look at the inode's i_dentry list.  The problem is that multiple
dentry's could be pointing to the same inode (e.g. for a regular file with
multiple hardlinks).  I don't know whether it matters which of the aliases
is given as an argument to getxattr.  GPFS getxattr iop uses the dentry
argument to get to an inode pointer, and doesn't use it for anything else,
so any alias dentry would work as good as any other.  If this is also the
case for other cluster file systems, perhaps doing an inode security state
refresh with nothing by inode pointer is not a problem.  From the GPFS
point of view, as long as the getxattr call is done outside of file system
call proper, it shouldn't be a problem.  Calling it from inside of another
iop/fop could be problematic, depending on what locks are held at this
point.  There's already precedent for this (security_d_instantiate called
from d_splice_alias et al), so with care this can be made to work.

yuri

[-- Attachment #2: Type: text/html, Size: 4988 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2011-02-22 22:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-18  3:08 inode security state and cluster file systems Yuri L Volobuev
2011-02-18 13:28 ` Stephen Smalley
2011-02-18 13:35   ` Stephen Smalley
2011-02-18 15:18     ` Eric Paris
2011-02-18 16:15   ` Yuri L Volobuev
2011-02-18 16:25     ` Stephen Smalley
2011-02-18 19:07       ` Casey Schaufler
2011-02-18 20:42       ` Yuri L Volobuev
2011-02-18 22:39         ` Eric Paris
2011-02-18 22:40           ` Eric Paris
2011-02-20 20:16             ` Casey Schaufler
2011-02-21 19:56               ` Eric Paris
2011-02-22  3:57                 ` Casey Schaufler
2011-02-22 15:25                 ` Yuri L Volobuev
2011-02-22 16:05                   ` Eric Paris
2011-02-22 22:17                     ` Yuri L Volobuev

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.