Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Fri, Dec 14, 2012 at 01:19:18PM -0600, Steve French wrote: On Fri, Dec 14, 2012 at 9:30 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: We can make this feature (passing O_DENY* flags received from clients to filesystem) can be turned on/off on Samba/NFS server to let this particular use case work. In general, I think we really need to be sure that nobody has a read access for files that a Windows process opened with O_DENYREAD (because there can be important reasons for the Windows process to do so). It should only affect windows emulated tasks, nothing else yes, but not just wine - there is probably a case for Samba server and NFSv4 to optionally request such behafvior). Agreed, but: Also we are likely to see more cases where users want to run Samba over an NFS mount and vice versa. I don't personally see the interest in this case. (And in fact I'd rather we removed the nfs export code for cifs; I seem to recall from the last discussion that filehandle lookups get ESTALE for inodes that have gone out of cache, and that that wasn't really fixable.) --b.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
2012/12/12 David Laight da...@l8s.co.uk: On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote: The problem is the possibility of denial-of-service attacks here. We can try to prevent them by: FWIW I already see a DoS 'attack'. I have some filestore shared using NFS (to Linux and Solaris) and using samba (to Windows). I use it for release builds of a product to ensure the versions built for the different operating systems match, and because some files have to be built on an 'alien' system (eg gcc targetted at embedded card). I can't run the windows build at the same time as the others because the windows C compiler manages to obtain exclusive access to the source files - stopping the other systems from reading them. We can make this feature (passing O_DENY* flags received from clients to filesystem) can be turned on/off on Samba/NFS server to let this particular use case work. In general, I think we really need to be sure that nobody has a read access for files that a Windows process opened with O_DENYREAD (because there can be important reasons for the Windows process to do so). -- Best regards, Pavel Shilovsky.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
We can make this feature (passing O_DENY* flags received from clients to filesystem) can be turned on/off on Samba/NFS server to let this particular use case work. In general, I think we really need to be sure that nobody has a read access for files that a Windows process opened with O_DENYREAD (because there can be important reasons for the Windows process to do so). It should only affect windows emulated tasks, nothing else. If you want to do locking friendly behaviour you could also take lockf locks so that Linux domain code that *wishes* to be nice in this area behaves the way you want. Without that you break so much stuff its a horrible idea (eg backups, ubuntu one sync etc). Alan
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Fri, Dec 14, 2012 at 9:30 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: We can make this feature (passing O_DENY* flags received from clients to filesystem) can be turned on/off on Samba/NFS server to let this particular use case work. In general, I think we really need to be sure that nobody has a read access for files that a Windows process opened with O_DENYREAD (because there can be important reasons for the Windows process to do so). It should only affect windows emulated tasks, nothing else yes, but not just wine - there is probably a case for Samba server and NFSv4 to optionally request such behafvior). Also we are likely to see more cases where users want to run Samba over an NFS mount and vice versa. -- Thanks, Steve
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote: The problem is the possibility of denial-of-service attacks here. We can try to prevent them by: FWIW I already see a DoS 'attack'. I have some filestore shared using NFS (to Linux and Solaris) and using samba (to Windows). I use it for release builds of a product to ensure the versions built for the different operating systems match, and because some files have to be built on an 'alien' system (eg gcc targetted at embedded card). I can't run the windows build at the same time as the others because the windows C compiler manages to obtain exclusive access to the source files - stopping the other systems from reading them. David -- David Laight: da...@l8s.co.uk
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Mon, 10 Dec 2012 11:41:16 -0500
J. Bruce Fields bfie...@fieldses.org wrote:
On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
The problem is the possibility of denial-of-service attacks here. We
can try to prevent them by:
1) specifying an extra security bit on the file that indicates that
share flags are accepted (like we have for mandatory locks now) and
setting it for neccessary files only, or
2) adding a special mount option (but it it probably makes sense if
we decided to add this support for CIFS and NFS only).
In the case of knfsd and samba exporting a common filesystem, you'd also
want to be able to enforce it on the exported filesystem.
Sorry for chiming in late on this, but I've been looking at this
problem from the other end as well, from the POV of a fileserver. For
there, you absolutely do want to have some mechanism for enforcing this
on local filesystems.
Currently, file servers generally enforce share reservations
internally. The upshot is that they're not aware when other tasks
outside the server modify a file. This is also problematic too in many
common fileserving situations -- when exporting files via both NFS and
SMB, for instance.
One thing that's important to note is that there is already some
support for this in the kernel. The LOCK_MAND flag and its associates
are intended for just this purpose. Samba even already calls into the
kernel to set LOCK_MAND locks, but the kernel just passes them through
to the fs. Rumor has it that GPFS does something with these flags, but
I can't confirm that.
Of course, LOCK_MAND is racy since you can't set it on open(), but it
might be nice to use that as a starting point for trying to add this
support.
At the very least, if we're going to do this, we need to consider what
to do with the LOCK_MAND interfaces. As a starting point for
discussion, here's a patch that I was playing with a few months ago. I
haven't had much time to really spend on this project, but it may be
worthwhile to consider. It works, but I'm not sure about the
semantics...
-[snip]
locks: add enforcement of LOCK_MAND locks
The LOCK_MAND lock mechanism is currently a no-op for any in-tree
filesystem. The flags are passed to f_ops-flock, but the standard
flock routines basically ignore them.
Change this by adding enforcement against other LOCK_MAND locks. Also,
assume that LOCK_MAND also implies LOCK_NB.
Signed-off-by: Jeff Layton jlay...@redhat.com
---
fs/locks.c | 45 ++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 814c51d..736e38b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -625,6 +625,43 @@ static int posix_locks_conflict(struct file_lock
*caller_fl, struct file_lock *s
return (locks_conflict(caller_fl, sys_fl));
}
+/*
+ * locks_mand_conflict - Determine if there's a share reservation conflict
+ * @caller_fl: lock we're attempting to acquire
+ * @sys_fl: lock already present on system that we're checking against
+ *
+ * Check to see if there's a share_reservation conflict. LOCK_READ/LOCK_WRITE
+ * tell us whether the reservation allows other readers and writers.
+ *
+ * We only check against other LOCK_MAND locks, so applications that want to
+ * use share mode locking will only conflict against one another. normal
+ * applications that open files won't be affected by and won't themselves
+ * affect the share reservations.
+ */
+static int locks_mand_conflict(struct file_lock *caller_fl,
+ struct file_lock *sys_fl)
+{
+ unsigned char caller_type = caller_fl-fl_type;
+ unsigned char sys_type = sys_fl-fl_type;
+ fmode_t caller_fmode = caller_fl-fl_file-f_mode;
+ fmode_t sys_fmode = sys_fl-fl_file-f_mode;
+
+ /* they can only conflict if they're both LOCK_MAND */
+ if (!(caller_type LOCK_MAND) || !(sys_type LOCK_MAND))
+ return 0;
+
+ if (!(caller_type LOCK_READ) (sys_fmode FMODE_READ))
+ return 1;
+ if (!(caller_type LOCK_WRITE) (sys_fmode FMODE_WRITE))
+ return 1;
+ if (!(sys_type LOCK_READ) (caller_fmode FMODE_READ))
+ return 1;
+ if (!(sys_type LOCK_WRITE) (caller_fmode FMODE_WRITE))
+ return 1;
+
+ return 0;
+}
+
/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
* checking before calling the locks_conflict().
*/
@@ -633,9 +670,11 @@ static int flock_locks_conflict(struct file_lock
*caller_fl, struct file_lock *s
/* FLOCK locks referring to the same filp do not conflict with
* each other.
*/
- if (!IS_FLOCK(sys_fl) || (caller_fl-fl_file == sys_fl-fl_file))
- return (0);
+ if (!IS_FLOCK(sys_fl))
+ return 0;
if ((caller_fl-fl_type LOCK_MAND) || (sys_fl-fl_type LOCK_MAND))
+ return
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Thu, Dec 6, 2012 at 1:57 PM, Jeremy Allison j...@samba.org wrote: On Thu, Dec 06, 2012 at 07:49:49PM +, Alan Cox wrote: On Thu, 6 Dec 2012 22:26:28 +0400 Pavel Shilovsky pias...@etersoft.ru wrote: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). If I have my root fs on NFS then the same applies does it not. Your patches fail to describe the security semantics and what file rights I must have to apply each option. How do I track down a lock user, what tools are provided ? How do the new options interact with the security layer? I don't have a problem with the idea, but it needs a lot more clear description of how it works so the model can be checked and if need be things tweaked (eg needing write to denywrite etc) And this is where things get really ugly of course :-). For the CIFSFS client they're expecting to be able to just ship them to a Windows server, where they'll get the (insane) Windows semantics. These semantics are not what would be wanted on a local filesystem. So unless we just say these things have Windows semantics (where openers of files can lock out others I suspect that WINE would have the same need to ship them to an NFS server as to a Windows server, and the NFS4 protocol specification also defines these, although I could not find the same level of detail that MS-FSA provides (e.g. see section 2.14.10 for the detailed description of how lock conflicts are checked) but the semantics are probably the same. -- Thanks, Steve
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Fri, Dec 7, 2012 at 8:29 AM, Steve French smfre...@gmail.com wrote: although I could not find the same level of detail that MS-FSA provides (e.g. see section 2.14.10 for the detailed Typo It is section 2.1.4.10 -- Thanks, Steve
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Fri, Dec 07, 2012 at 01:08:46PM +0400, Pavel Shilovsky wrote: 2012/12/6 Pavel Shilovsky pias...@etersoft.ru: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0. Pavel Shilovsky (3): fcntl: Introduce new O_DENY* open flags for network filesystems CIFS: Add O_DENY* open flags support CIFS: Use NT_CREATE_ANDX command for forcemand mounts fs/cifs/cifsacl.c| 10 fs/cifs/cifsglob.h | 11 - fs/cifs/cifsproto.h |9 fs/cifs/cifssmb.c| 47 -- fs/cifs/dir.c| 14 fs/cifs/file.c | 18 ++- fs/cifs/inode.c | 11 + fs/cifs/link.c | 10 fs/cifs/readdir.c|2 +- fs/cifs/smb1ops.c| 15 ++-- fs/cifs/smb2file.c | 10 fs/cifs/smb2inode.c |4 ++-- fs/cifs/smb2ops.c| 10 fs/cifs/smb2pdu.c|6 ++--- fs/cifs/smb2proto.h | 14 +++- fs/fcntl.c |5 ++-- include/uapi/asm-generic/fcntl.h | 11 + 17 files changed, 125 insertions(+), 82 deletions(-) -- 1.7.10.4 First of all, sorry for being unclear at this proposal. I am not from Wine team but I am working on things related to Wine+CIFS-client+Samba. We (at Etersoft) need to organize the work of Wine applications through cifs-client share mounted to Samba (or Windows server). They are related to accounting (mostly Russian ones - e.g. http://www.1c.ru/eng/title.htm). So, the problem is that such applications use share flags to control the parallel access to the same files of several clients on a remote share. Also, there can be a situation where Windows (native) clients and Wine clients are working together in the same time. That's why we need such flags in the kernel (patch #1). With these flags Wine can pass them to every open and they will be used by CIFS (and probably NFS) file systems to pass to the Samba server. In the same time if the file is on local filesystem - these flags will be simply ignored (not implemented). NFS supports the deny-read and deny-write bits but not deny-delete. If we could do such opens in-kernel on local and clustered filesystems, that could also be useful for multi-protocol (Samba and NFS) and clustered exports. Currently knfsd tries to enforce deny bits in the nfsd code, which is a bit ugly. And knfsd currently requires write permissions for deny-read. My understanding is that Windows considers that wrong, but I'd be curious to know whether that breaks Windows applications in practice. --b.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Fri, 2012-12-07 at 09:52 -0500, J. Bruce Fields wrote: On Fri, Dec 07, 2012 at 01:08:46PM +0400, Pavel Shilovsky wrote: 2012/12/6 Pavel Shilovsky pias...@etersoft.ru: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0. Pavel Shilovsky (3): fcntl: Introduce new O_DENY* open flags for network filesystems CIFS: Add O_DENY* open flags support CIFS: Use NT_CREATE_ANDX command for forcemand mounts fs/cifs/cifsacl.c| 10 fs/cifs/cifsglob.h | 11 - fs/cifs/cifsproto.h |9 fs/cifs/cifssmb.c| 47 -- fs/cifs/dir.c| 14 fs/cifs/file.c | 18 ++- fs/cifs/inode.c | 11 + fs/cifs/link.c | 10 fs/cifs/readdir.c|2 +- fs/cifs/smb1ops.c| 15 ++-- fs/cifs/smb2file.c | 10 fs/cifs/smb2inode.c |4 ++-- fs/cifs/smb2ops.c| 10 fs/cifs/smb2pdu.c|6 ++--- fs/cifs/smb2proto.h | 14 +++- fs/fcntl.c |5 ++-- include/uapi/asm-generic/fcntl.h | 11 + 17 files changed, 125 insertions(+), 82 deletions(-) -- 1.7.10.4 First of all, sorry for being unclear at this proposal. I am not from Wine team but I am working on things related to Wine+CIFS-client+Samba. We (at Etersoft) need to organize the work of Wine applications through cifs-client share mounted to Samba (or Windows server). They are related to accounting (mostly Russian ones - e.g. http://www.1c.ru/eng/title.htm). So, the problem is that such applications use share flags to control the parallel access to the same files of several clients on a remote share. Also, there can be a situation where Windows (native) clients and Wine clients are working together in the same time. That's why we need such flags in the kernel (patch #1). With these flags Wine can pass them to every open and they will be used by CIFS (and probably NFS) file systems to pass to the Samba server. In the same time if the file is on local filesystem - these flags will be simply ignored (not implemented). NFS supports the deny-read and deny-write bits but not deny-delete. If we could do such opens in-kernel on local and clustered filesystems, that could also be useful for multi-protocol (Samba and NFS) and clustered exports. Currently knfsd tries to enforce deny bits in the nfsd code, which is a bit ugly. And knfsd currently requires write permissions for deny-read. My understanding is that Windows considers that wrong, but I'd be curious to know whether that breaks Windows applications in practice. It probably does if you look hard enough. IIRC Deny-reads are very loosely like read locks, and you can take read locks if you have read-only access. Why does knfsd restrict deny-read to read-write access ? Simo. -- Simo Sorce Samba Team GPL Compliance Officer s...@samba.org Principal Software Engineer at Red Hat, Inc. s...@redhat.com
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Fri, Dec 07, 2012 at 10:37:45AM -0500, simo wrote: On Fri, 2012-12-07 at 09:52 -0500, J. Bruce Fields wrote: On Fri, Dec 07, 2012 at 01:08:46PM +0400, Pavel Shilovsky wrote: 2012/12/6 Pavel Shilovsky pias...@etersoft.ru: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0. Pavel Shilovsky (3): fcntl: Introduce new O_DENY* open flags for network filesystems CIFS: Add O_DENY* open flags support CIFS: Use NT_CREATE_ANDX command for forcemand mounts fs/cifs/cifsacl.c| 10 fs/cifs/cifsglob.h | 11 - fs/cifs/cifsproto.h |9 fs/cifs/cifssmb.c| 47 -- fs/cifs/dir.c| 14 fs/cifs/file.c | 18 ++- fs/cifs/inode.c | 11 + fs/cifs/link.c | 10 fs/cifs/readdir.c|2 +- fs/cifs/smb1ops.c| 15 ++-- fs/cifs/smb2file.c | 10 fs/cifs/smb2inode.c |4 ++-- fs/cifs/smb2ops.c| 10 fs/cifs/smb2pdu.c|6 ++--- fs/cifs/smb2proto.h | 14 +++- fs/fcntl.c |5 ++-- include/uapi/asm-generic/fcntl.h | 11 + 17 files changed, 125 insertions(+), 82 deletions(-) -- 1.7.10.4 First of all, sorry for being unclear at this proposal. I am not from Wine team but I am working on things related to Wine+CIFS-client+Samba. We (at Etersoft) need to organize the work of Wine applications through cifs-client share mounted to Samba (or Windows server). They are related to accounting (mostly Russian ones - e.g. http://www.1c.ru/eng/title.htm). So, the problem is that such applications use share flags to control the parallel access to the same files of several clients on a remote share. Also, there can be a situation where Windows (native) clients and Wine clients are working together in the same time. That's why we need such flags in the kernel (patch #1). With these flags Wine can pass them to every open and they will be used by CIFS (and probably NFS) file systems to pass to the Samba server. In the same time if the file is on local filesystem - these flags will be simply ignored (not implemented). NFS supports the deny-read and deny-write bits but not deny-delete. If we could do such opens in-kernel on local and clustered filesystems, that could also be useful for multi-protocol (Samba and NFS) and clustered exports. Currently knfsd tries to enforce deny bits in the nfsd code, which is a bit ugly. And knfsd currently requires write permissions for deny-read. My understanding is that Windows considers that wrong, but I'd be curious to know whether that breaks Windows applications in practice. It probably does if you look hard enough. IIRC Deny-reads are very loosely like read locks, and you can take read locks if you have read-only access. I had the impression they didn't care about read or write permissions at all, but I don't know. Why does knfsd restrict deny-read to read-write access ? Because I think it's normal to want files that everyone's able to read without giving everyone the right to DOS readers indefinitely. And because I'm having a hard time thinking why you'd care to keep everyone from reading a file that you don't intend to modify. --b.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Thu, Dec 06, 2012 at 10:26:28PM +0400, Pavel Shilovsky wrote: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0. I don't think introducing user visible flags that are only supported on a single network filesystem is a good idea. I'm not even sure adding these flags does make a lot of sense, but assuming we'd actually want this (and I'd like some more detailed explanation) I think we'd at least need to make sure that: a) opening files with the new modes gives a proper error message if not supported b) there needs to be local support for them as well c) we need to think really hard when they should be supported, and need a good rational for it. I can't see how we could do it unconditionally for all users as that would introduce easy denial of services attacks the way I understand the semantics (correct me if I'm wrong). So a mount option like you currently do probably is the least bad even if don't fell overly happy about that version. What is the reason your special wine use case can't simply use a userspace cifs client? Given that wine uses windows filesystem semantics and cifs does as well tunnelling it through a Posix-like API inbetween is never going to be perfect.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
I suspect that WINE would have the same need Tricky - Wine needs to enforce this behaviour solely between Wine and the file server, Trying to muck up non emulated local behaviour would be a bad mistake. One way perhaps to look at this is you want some tasks to be able to *opt in* to this behaviour. First however I think we need a much better description of the setup that uses them and why it matters.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
Christoph Hellwig писал 07.12.2012 20:16: On Thu, Dec 06, 2012 at 10:26:28PM +0400, Pavel Shilovsky wrote: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0. I don't think introducing user visible flags that are only supported on a single network filesystem is a good idea. It can bring benefits for both CIFS and NFS filesystems - so, at least two ones. I'm not even sure adding these flags does make a lot of sense, but assuming we'd actually want this (and I'd like some more detailed explanation) I think we'd at least need to make sure that: a) opening files with the new modes gives a proper error message if not supported It makes us add such checks for all other filesystems, if I understand right, - not a problem, I think. b) there needs to be local support for them as well c) we need to think really hard when they should be supported, and need a good rational for it. I can't see how we could do it unconditionally for all users as that would introduce easy denial of services attacks the way I understand the semantics (correct me if I'm wrong). So a mount option like you currently do probably is the least bad even if don't fell overly happy about that version. What is the reason your special wine use case can't simply use a userspace cifs client? Given that wine uses windows filesystem semantics and cifs does as well tunnelling it through a Posix-like API inbetween is never going to be perfect. Ideally we should not make any difference between underlying filesystems in Wine: an application requests an open of the file and we issue this open with flags it passed. Since Wineserver can process share flags locally itself (for one linux user), we only need to add this support for CIFS (that is actively used by Wine applications because of it's Windows nature). Bringing these flags for local filesystems can benefit Wine too: it will help in cases when Wine applications of different users on the same machine use the same file and can make all those things easier, of course. The problem is the possibility of denial-of-service attacks here. We can try to prevent them by: 1) specifying an extra security bit on the file that indicates that share flags are accepted (like we have for mandatory locks now) and setting it for neccessary files only, or 2) adding a special mount option (but it it probably makes sense if we decided to add this support for CIFS and NFS only). -- Best regards, Pavel Shilovsky.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
The problem is the possibility of denial-of-service attacks here. We can try to prevent them by: 1) specifying an extra security bit on the file that indicates that share flags are accepted (like we have for mandatory locks now) and setting it for neccessary files only, or 2) adding a special mount option (but it it probably makes sense if we decided to add this support for CIFS and NFS only). 3) making it a property that the process opts into - the same fs can have Linux and wine users at once. But for such a big set of changes and with the kind of potential fallout you need to demonstrate a good practical use case IMHO, not just a neat idea I had. Alan
RE: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
-Original Message- From: linux-nfs-ow...@vger.kernel.org [mailto:linux-nfs- ow...@vger.kernel.org] On Behalf Of Pavel Shilovsky Sent: Friday, December 07, 2012 9:43 PM To: Christoph Hellwig Cc: linux-c...@vger.kernel.org; linux-ker...@vger.kernel.org; linux- fsde...@vger.kernel.org; wine-devel@winehq.org; linux- n...@vger.kernel.org Subject: Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs Christoph Hellwig писал 07.12.2012 20:16: On Thu, Dec 06, 2012 at 10:26:28PM +0400, Pavel Shilovsky wrote: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0. I don't think introducing user visible flags that are only supported on a single network filesystem is a good idea. It can bring benefits for both CIFS and NFS filesystems - so, at least two ones. I'm not even sure adding these flags does make a lot of sense, but assuming we'd actually want this (and I'd like some more detailed explanation) I think we'd at least need to make sure that: a) opening files with the new modes gives a proper error message if not supported It makes us add such checks for all other filesystems, if I understand right, - not a problem, I think. b) there needs to be local support for them as well c) we need to think really hard when they should be supported, and need a good rational for it. I can't see how we could do it unconditionally for all users as that would introduce easy denial of services attacks the way I understand the semantics (correct me if I'm wrong). So a mount option like you currently do probably is the least bad even if don't fell overly happy about that version. What is the reason your special wine use case can't simply use a userspace cifs client? Given that wine uses windows filesystem semantics and cifs does as well tunnelling it through a Posix-like API inbetween is never going to be perfect. Ideally we should not make any difference between underlying filesystems in Wine: an application requests an open of the file and we issue this open with flags it passed. Since Wineserver can process share flags locally itself (for one linux user), we only need to add this support for CIFS (that is actively used by Wine applications because of it's Windows nature). Bringing these flags for local filesystems can benefit Wine too: it will help in cases when Wine applications of different users on the same machine use the same file and can make all those things easier, of course. The problem is the possibility of denial-of-service attacks here. We can try to prevent them by: 1) specifying an extra security bit on the file that indicates that share flags are accepted (like we have for mandatory locks now) and setting it for neccessary files only, or 2) adding a special mount option (but it it probably makes sense if we decided to add this support for CIFS and NFS only). Why not just put it under the control of LSM? It seems to me that this doesn't so much want to be a per-mount switch but rather deserves to be a per-process MAC (i.e. is this running in a Wine sandbox or not)... Cheers Trond
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote: The problem is the possibility of denial-of-service attacks here. We can try to prevent them by: 1) specifying an extra security bit on the file that indicates that share flags are accepted (like we have for mandatory locks now) and setting it for neccessary files only, or 2) adding a special mount option (but it it probably makes sense if we decided to add this support for CIFS and NFS only). In the case of knfsd and samba exporting a common filesystem, you'd also want to be able to enforce it on the exported filesystem. --b.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
2012/12/6 Pavel Shilovsky pias...@etersoft.ru: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0. Pavel Shilovsky (3): fcntl: Introduce new O_DENY* open flags for network filesystems CIFS: Add O_DENY* open flags support CIFS: Use NT_CREATE_ANDX command for forcemand mounts fs/cifs/cifsacl.c| 10 fs/cifs/cifsglob.h | 11 - fs/cifs/cifsproto.h |9 fs/cifs/cifssmb.c| 47 -- fs/cifs/dir.c| 14 fs/cifs/file.c | 18 ++- fs/cifs/inode.c | 11 + fs/cifs/link.c | 10 fs/cifs/readdir.c|2 +- fs/cifs/smb1ops.c| 15 ++-- fs/cifs/smb2file.c | 10 fs/cifs/smb2inode.c |4 ++-- fs/cifs/smb2ops.c| 10 fs/cifs/smb2pdu.c|6 ++--- fs/cifs/smb2proto.h | 14 +++- fs/fcntl.c |5 ++-- include/uapi/asm-generic/fcntl.h | 11 + 17 files changed, 125 insertions(+), 82 deletions(-) -- 1.7.10.4 First of all, sorry for being unclear at this proposal. I am not from Wine team but I am working on things related to Wine+CIFS-client+Samba. We (at Etersoft) need to organize the work of Wine applications through cifs-client share mounted to Samba (or Windows server). They are related to accounting (mostly Russian ones - e.g. http://www.1c.ru/eng/title.htm). So, the problem is that such applications use share flags to control the parallel access to the same files of several clients on a remote share. Also, there can be a situation where Windows (native) clients and Wine clients are working together in the same time. That's why we need such flags in the kernel (patch #1). With these flags Wine can pass them to every open and they will be used by CIFS (and probably NFS) file systems to pass to the Samba server. In the same time if the file is on local filesystem - these flags will be simply ignored (not implemented). Now we have to make our own builds of cifs module for every kernel that use these flags passed by our build of Wine - this scheme is working but requires merging every time new kernel is released. Getting things into mainline let Wine supports more applications. As for the security layer, I don't think that we need any extra bits to switch these flags on or off - it will be switched on/off by an underlying filesystem. Since, this change is targeted for a network purpose only, a tool, that shows us what process/user locks a file, doesn't help a lot because the file can be locked remotely. As was said above, this change let us give Wine application share flags possibility for both Samba and Windows servers. Patch #2 enables share flags support for cifs mounts of Windows servers or Samba servers with nounix mount option. But we already have forcemand mount option in cifs module that switches on mandatory byte-range locking semantic for Samba without nounix. Since share flags capability is a kind of 'mandatory' locking scheme, I suggest that this option should switch on share flags for Samba without nounix too (by using NTCreateAndX command for open rather than transaction2) - that is what patch #3 does. -- Best regards, Pavel Shilovsky.
[PATCH 0/3] Add O_DENY* flags to fcntl and cifs
Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0. Pavel Shilovsky (3): fcntl: Introduce new O_DENY* open flags for network filesystems CIFS: Add O_DENY* open flags support CIFS: Use NT_CREATE_ANDX command for forcemand mounts fs/cifs/cifsacl.c| 10 fs/cifs/cifsglob.h | 11 - fs/cifs/cifsproto.h |9 fs/cifs/cifssmb.c| 47 -- fs/cifs/dir.c| 14 fs/cifs/file.c | 18 ++- fs/cifs/inode.c | 11 + fs/cifs/link.c | 10 fs/cifs/readdir.c|2 +- fs/cifs/smb1ops.c| 15 ++-- fs/cifs/smb2file.c | 10 fs/cifs/smb2inode.c |4 ++-- fs/cifs/smb2ops.c| 10 fs/cifs/smb2pdu.c|6 ++--- fs/cifs/smb2proto.h | 14 +++- fs/fcntl.c |5 ++-- include/uapi/asm-generic/fcntl.h | 11 + 17 files changed, 125 insertions(+), 82 deletions(-) -- 1.7.10.4
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Thu, 6 Dec 2012 22:26:28 +0400 Pavel Shilovsky pias...@etersoft.ru wrote: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). If I have my root fs on NFS then the same applies does it not. Your patches fail to describe the security semantics and what file rights I must have to apply each option. How do I track down a lock user, what tools are provided ? How do the new options interact with the security layer? I don't have a problem with the idea, but it needs a lot more clear description of how it works so the model can be checked and if need be things tweaked (eg needing write to denywrite etc) Alan
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Thu, Dec 06, 2012 at 07:49:49PM +, Alan Cox wrote: On Thu, 6 Dec 2012 22:26:28 +0400 Pavel Shilovsky pias...@etersoft.ru wrote: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). If I have my root fs on NFS then the same applies does it not. Your patches fail to describe the security semantics and what file rights I must have to apply each option. How do I track down a lock user, what tools are provided ? How do the new options interact with the security layer? I don't have a problem with the idea, but it needs a lot more clear description of how it works so the model can be checked and if need be things tweaked (eg needing write to denywrite etc) And this is where things get really ugly of course :-). For the CIFSFS client they're expecting to be able to just ship them to a Windows server, where they'll get the (insane) Windows semantics. These semantics are not what would be wanted on a local filesystem. So unless we just say these things have Windows semantics (where openers of files can lock out others under dubious circumstances) there'll be this horrible difference between (I'm assuming) the sane semantics that are defined for local filesystems and the insane ones that you get when you're connecting remotely. I don't know a good way to fix that, but I'm pretty sure you don't want the Windows semantics defined locally :-). Jeremy.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Thu, Dec 06, 2012 at 11:57:52AM -0800, Jeremy Allison wrote: On Thu, Dec 06, 2012 at 07:49:49PM +, Alan Cox wrote: On Thu, 6 Dec 2012 22:26:28 +0400 Pavel Shilovsky pias...@etersoft.ru wrote: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). If I have my root fs on NFS then the same applies does it not. Your patches fail to describe the security semantics and what file rights I must have to apply each option. How do I track down a lock user, what tools are provided ? How do the new options interact with the security layer? I don't have a problem with the idea, but it needs a lot more clear description of how it works so the model can be checked and if need be things tweaked (eg needing write to denywrite etc) And this is where things get really ugly of course :-). For the CIFSFS client they're expecting to be able to just ship them to a Windows server, where they'll get the (insane) Windows semantics. These semantics are not what would be wanted on a local filesystem. So unless we just say these things have Windows semantics (where openers of files can lock out others under dubious circumstances) there'll be this horrible difference between (I'm assuming) the sane semantics that are defined for local filesystems and the insane ones that you get when you're connecting remotely. I don't know a good way to fix that, but I'm pretty sure you don't want the Windows semantics defined locally :-). You could just flags these as ignored on local filesystems of course, exact semantics defined by the remote filesystem. That's really what applications will get anyway. But it's not condusive to writing documentation on what these things do :-). Jeremy.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Thu, Dec 06, 2012 at 11:57:52AM -0800, Jeremy Allison wrote: And this is where things get really ugly of course :-). For the CIFSFS client they're expecting to be able to just ship them to a Windows server, where they'll get the (insane) Windows semantics. These semantics are not what would be wanted on a local filesystem. I'm confused; why would a userspace application need to be able to request this behavior? I can understand why an SMB client might depend on this so it can use Windows' insane cache coherency scheme. Are you trying to let Samba act as a middle man, where a remote file system is mounted on Linux, and then Samba will try to act as a SMB server, so you want to be able to pass through these semantics, i.e.: Windows SMB Server --- Linux cifs remote file system --- Linux Samba server --- Windows SMB client Is this somewhat contrivuewd example the intended use case? Or something else? - Ted
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Thu, Dec 06, 2012 at 04:31:33PM -0500, Theodore Ts'o wrote: On Thu, Dec 06, 2012 at 11:57:52AM -0800, Jeremy Allison wrote: And this is where things get really ugly of course :-). For the CIFSFS client they're expecting to be able to just ship them to a Windows server, where they'll get the (insane) Windows semantics. These semantics are not what would be wanted on a local filesystem. I'm confused; why would a userspace application need to be able to request this behavior? This isn't my proposal Ted, I'm just commenting on it :-). Jeremy.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Thu, Dec 06, 2012 at 01:33:29PM -0800, Jeremy Allison wrote: I'm confused; why would a userspace application need to be able to request this behavior? This isn't my proposal Ted, I'm just commenting on it :-). Ah, sorry, I thought was coming from the Samba team. :-) Hmm... I see wine-devel is cc'ed; is this coming from the Wine team, wanting to do SMB paravirtualization? It would be useful if the commit description described how these flags are intended to be used - Ted
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Thu, Dec 06, 2012 at 04:37:27PM -0500, Theodore Ts'o wrote: On Thu, Dec 06, 2012 at 01:33:29PM -0800, Jeremy Allison wrote: I'm confused; why would a userspace application need to be able to request this behavior? This isn't my proposal Ted, I'm just commenting on it :-). Ah, sorry, I thought was coming from the Samba team. :-) Well it sort of is, as the people working on cifsfs are also Samba Team members, but this isn't an official Samba thing, as I can't see exactly what apps would want this either (other than Samba smbd running on top of smbd, re-exporting a cifsfs share, pointing to a Samba server, running on a... ERROR STACK OVERFLOW :-). Hmm... I see wine-devel is cc'ed; is this coming from the Wine team, wanting to do SMB paravirtualization? It would be useful if the commit description described how these flags are intended to be used Indeed :-). Jeremy.
Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
2012/11/30 Pavel Shilovsky pias...@etersoft.ru: Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file). Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0. Pavel Shilovsky (3): fcntl: Introduce new O_DENY* open flags for network filesystems CIFS: Add O_DENY* open flags support CIFS: Use NT_CREATE_ANDX command for forcemand mounts fs/cifs/cifsacl.c| 10 fs/cifs/cifsglob.h | 11 - fs/cifs/cifsproto.h |9 fs/cifs/cifssmb.c| 47 -- fs/cifs/dir.c| 14 fs/cifs/file.c | 18 ++- fs/cifs/inode.c | 11 + fs/cifs/link.c | 10 fs/cifs/readdir.c|2 +- fs/cifs/smb1ops.c| 15 ++-- fs/cifs/smb2file.c | 10 fs/cifs/smb2inode.c |4 ++-- fs/cifs/smb2ops.c| 10 fs/cifs/smb2pdu.c|6 ++--- fs/cifs/smb2proto.h | 14 +++- fs/fcntl.c |5 ++-- include/uapi/asm-generic/fcntl.h | 11 + 17 files changed, 125 insertions(+), 82 deletions(-) -- 1.7.10.4 CC'ing wine-devel@. -- Best regards, Pavel Shilovsky.
