Re: [PATCH 0/2] Shared flags

Re: [PATCH 0/2] Shared flags

[Christoph Hellwig]

On Mon, Nov 10, 2008 at 04:58:17PM +0300, Pavel Shilovsky wrote:
> Hello!
> We have situation during our work with Wine and Cifs-client  then we  
> have to use sharing access policy to files and we used 3 free bits from  
> 21 to 23 of attribute flag(it allows to do NT-semantic for opening files  
> in open() and it is necessary for correct implementation of function  
> NTCreateFile in Wine). We think that it'll be useful not only for us and  
> we proffer to add into the file kernel/include/asm-generic/fcntl.h  
> following flags:
>
> #define O_DENYREAD      004000000 /* Do not permit read access */
> #define O_DENYWRITE     010000000 /* Do not permit write access */
> #define O_DENYDELETE  020000000 /* Do not permit delete or rename  
> operations*/

I'm not completely opposed to these, but the patches you postd are
rather incomplete:

 (1) you need to add it to fcntl.h for those architetures not using the
     generic fcntl.h or those who would have overlapping bits with the
     generic defintions.
 (2) you also need to enforce these semantics in the VFS for local
     filesystems

Now if (2) doesn't cause too much overhead I would say it's fine, if not
I would rather avoid it.

Also please Cc the linux-api list for userspace API updates.

--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Shared flags

[Jamie Lokier]

Christoph Hellwig wrote:
> On Mon, Nov 10, 2008 at 04:58:17PM +0300, Pavel Shilovsky wrote:
> > #define O_DENYREAD      004000000 /* Do not permit read access */
> > #define O_DENYWRITE     010000000 /* Do not permit write access */
> > #define O_DENYDELETE  020000000 /* Do not permit delete or rename operations*/

>  (2) you also need to enforce these semantics in the VFS for local
>      filesystems
> 
> Now if (2) doesn't cause too much overhead I would say it's fine, if not
> I would rather avoid it.

On the face of it, they look like they have similar denial-of-service
potential as mandatory locks.  For example, a root process cleaning
out /tmp gets stuck because a user process has O_DENYDELETE set.

-- Jamie

Re: [PATCH 0/2] Shared flags

[Christoph Hellwig]

On Tue, Nov 11, 2008 at 10:09:40AM +0000, Jamie Lokier wrote:
> Christoph Hellwig wrote:
> > On Mon, Nov 10, 2008 at 04:58:17PM +0300, Pavel Shilovsky wrote:
> > > #define O_DENYREAD      004000000 /* Do not permit read access */
> > > #define O_DENYWRITE     010000000 /* Do not permit write access */
> > > #define O_DENYDELETE  020000000 /* Do not permit delete or rename operations*/
> 
> >  (2) you also need to enforce these semantics in the VFS for local
> >      filesystems
> > 
> > Now if (2) doesn't cause too much overhead I would say it's fine, if not
> > I would rather avoid it.
> 
> On the face of it, they look like they have similar denial-of-service
> potential as mandatory locks.  For example, a root process cleaning
> out /tmp gets stuck because a user process has O_DENYDELETE set.

Oh, that's the part I forgot to mention in the previous mail, all of
these option of course can only be root only, everything else would
be - as you say - a complete security nightmare.

Re: [PATCH 0/2] Shared flags

[Pavel Shilovsky]

Christoph Hellwig wrote:
> On Tue, Nov 11, 2008 at 10:09:40AM +0000, Jamie Lokier wrote:
>   
>> Christoph Hellwig wrote:
>>     
>>> On Mon, Nov 10, 2008 at 04:58:17PM +0300, Pavel Shilovsky wrote:
>>>       
>>>> #define O_DENYREAD      004000000 /* Do not permit read access */
>>>> #define O_DENYWRITE     010000000 /* Do not permit write access */
>>>> #define O_DENYDELETE  020000000 /* Do not permit delete or rename operations*/
>>>>         
>>>  (2) you also need to enforce these semantics in the VFS for local
>>>      filesystems
>>>
>>> Now if (2) doesn't cause too much overhead I would say it's fine, if not
>>> I would rather avoid it.
>>>       
>> On the face of it, they look like they have similar denial-of-service
>> potential as mandatory locks.  For example, a root process cleaning
>> out /tmp gets stuck because a user process has O_DENYDELETE set.
>>     
>
> Oh, that's the part I forgot to mention in the previous mail, all of
> these option of course can only be root only, everything else would
> be - as you say - a complete security nightmare.
>
>   
We suggest to switch on this flags with special options during mounting. 
By default, it'll be switch off.
This solution deletes possibility of such situations, like in example.

Pavel Shilovsky.

Re: [PATCH 0/2] Shared flags

[Christoph Hellwig]

On Thu, Nov 13, 2008 at 01:08:58PM +0300, Pavel Shilovsky wrote:
> We suggest to switch on this flags with special options during mounting.  
> By default, it'll be switch off.
> This solution deletes possibility of such situations, like in example.

Sorry, having completely weird open modes only for one filesystem, and
only depending on mount options is not very nice to the user.

And options or not allowing the above functionality for regular users
is a security issue.

--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Shared flags

[Jamie Lokier]

Christoph Hellwig wrote:
> On Thu, Nov 13, 2008 at 01:08:58PM +0300, Pavel Shilovsky wrote:
> > We suggest to switch on this flags with special options during mounting.  
> > By default, it'll be switch off.
> > This solution deletes possibility of such situations, like in example.
> 
> Sorry, having completely weird open modes only for one filesystem, and
> only depending on mount options is not very nice to the user.
> 
> And options or not allowing the above functionality for regular users
> is a security issue.

A generic mount option is currently used for mandatory locking - this
is very similar.

The only different I see, for security, is with mandatory locking a
process which doesn't want to get stuck can check the permission bits
before opening a file.  But I'm not aware of anything actually doing this.

-- Jamie

Re: [PATCH 0/2] Shared flags

[Christoph Hellwig]

On Fri, Nov 14, 2008 at 03:37:12AM +0000, Jamie Lokier wrote:
> A generic mount option is currently used for mandatory locking - this
> is very similar.
> 
> The only different I see, for security, is with mandatory locking a
> process which doesn't want to get stuck can check the permission bits
> before opening a file.  But I'm not aware of anything actually doing this.

More importantly the admin can do it.  And mandatory locking doesn't
mean you can't remove something, with would be a complete nightmare.  It
also doesn't apply to directories, which from my reading of the patch
this one would do.  But yeah, doing the read/write part as as a special
case of mandlock on open might make some sense, but I'm still not too
convinced.

That more I think about these option the less I like the idea, it's just
going to cause a lot of problems to help with some wine issues that
hasn't even been explained yet.

Re: [PATCH 0/2] Shared flags

[Pavel Shilovsky]

I think that it'll be interesting for others filesystems. Also I should
say that solution with mounting options let us increase usual
possibilities to organize access policy. If you need it, you switch it
on, if don't (like in example) you simply don't use it! In conclusion,
we have increased functionality without any troubles.

Also, I should say that share flags is necessary for working with wine,
because NTCreateFile use it... 

On Saturday 15 November 2008 14:39:38 you wrote:
> On Fri, Nov 14, 2008 at 03:37:12AM +0000, Jamie Lokier wrote:
> > A generic mount option is currently used for mandatory locking - this
> > is very similar.
> >
> > The only different I see, for security, is with mandatory locking a
> > process which doesn't want to get stuck can check the permission bits
> > before opening a file.  But I'm not aware of anything actually doing
> > this.
>
> More importantly the admin can do it.  And mandatory locking doesn't
> mean you can't remove something, with would be a complete nightmare.  It
> also doesn't apply to directories, which from my reading of the patch
> this one would do.  But yeah, doing the read/write part as as a special
> case of mandlock on open might make some sense, but I'm still not too
> convinced.
>
> That more I think about these option the less I like the idea, it's just
> going to cause a lot of problems to help with some wine issues that
> hasn't even been explained yet.


--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Shared flags

[Benny Halevy]

Christoph Hellwig wrote:
> On Thu, Nov 13, 2008 at 01:08:58PM +0300, Pavel Shilovsky wrote:
>> We suggest to switch on this flags with special options during mounting.  
>> By default, it'll be switch off.
>> This solution deletes possibility of such situations, like in example.
> 
> Sorry, having completely weird open modes only for one filesystem, and

FWIW, I could see something like this being used for nfsv4 OPENs

Benny

> only depending on mount options is not very nice to the user.
> 
> And options or not allowing the above functionality for regular users
> is a security issue.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html