| Message ID | 1243205117-3351-3-git-send-email-jlayton@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, May 24, 2009 at 5:45 PM, Jeff Layton <jlayton@redhat.com> wrote: > The current default file mode is 02767 and dir mode is 0777. This is > extremely "loose". Given that CIFS is a single-user protocol, these > permissions allow anyone to use the mount -- in effect, giving anyone on > the machine access to the credentials used to mount the share. Yes they are probably too loose, although Multiuser mount do work in some use cases, most use single user mounts. Will be useful to know when cifsacl code is ready to turn on by default, and whether that would help. Perhaps the right answer is a default (to Windows) of something like 0744, but seems like we need more user feedback from a variety of different users (desktop, enterprise, home etc.) to decide what the ideal default is. > Note that this patch also removes the mandatory locking flags from the > default file_mode. After having looked at how these flags are used by > the kernel, I don't think that keeping them as the default offers any > real benefit. That flag combination makes it so that the kernel enforces > mandatory locking. This was initially done as much to report to the program that we use mandatory locking on these files (when mounted to Windows). Linux doesn't handle "mandatory locking" in the vfs on the client (which would be redundant with what the server is doing). We can't do much to turn off Windows byte range locking behavior but at least this alerts apps that they are getting mandatory locking on these inodes. If no app ever checks this bit, and the bit is obsolete, we could remove it.
On Sun, 24 May 2009 20:45:39 -0500 Steve French <smfrench@gmail.com> wrote: > On Sun, May 24, 2009 at 5:45 PM, Jeff Layton <jlayton@redhat.com> wrote: > > The current default file mode is 02767 and dir mode is 0777. This is > > extremely "loose". Given that CIFS is a single-user protocol, these > > permissions allow anyone to use the mount -- in effect, giving anyone on > > the machine access to the credentials used to mount the share. > > Yes they are probably too loose, although Multiuser mount do work > in some use cases, most use single user mounts. Will be useful to > know when cifsacl code is ready to turn on by default, and whether > that would help. Perhaps the right answer is a default (to Windows) > of something like 0744, but seems like we need more user feedback > from a variety of different users (desktop, enterprise, home etc.) > to decide what the ideal default is. > This change really isn't so much about multiuser mounts I think (unless I'm missing the connection). I think we need to keep this in perspective -- this value is only a default. The right thing to do is to make that default a restrictive, owner-only mode (i.e. 0700). The admin can *always* override this if they think it's too restrictive, but at that point it's a conscious decision. > > Note that this patch also removes the mandatory locking flags from the > > default file_mode. After having looked at how these flags are used by > > the kernel, I don't think that keeping them as the default offers any > > real benefit. That flag combination makes it so that the kernel enforces > > mandatory locking. > > This was initially done as much to report to the program that we use > mandatory locking on these files (when mounted to Windows). > Linux doesn't handle "mandatory locking" in the vfs on the client > (which would be redundant with what the server is doing). > We can't do much to turn off Windows byte range locking behavior > but at least this alerts apps that they are getting mandatory > locking on these inodes. If no app ever checks this bit, and the bit > is obsolete, we could remove it. > > I don't think programs ever look at this as a hint, for them to do that would mean that they have to also pay attention to the "mand" mount option, and that's almost certainly not portable. The kernel on the other hand definitely pays attention to this. When the share is mounted with "mand", the group execute bit is cleared, and setgid bit is set, the kernel will enforce mandatory locking (see Documentation/filesystems/mandatory-locking.txt in the kernel sources). I regularly have to change the default file_mode when testing because of this. I think what this really comes down to is this question: "When mounting a server w/o unix extensions, do we want the client kernel to default to enforcing mandatory locking between processes on the same machine?" My feeling on this is "no". Mandatory locking under unix is sufficiently rare that almost no programs are actually coded for it. If the server is enforcing mandatory locking, there's nothing we can do about that, but there's nothing to be gained by making the client's kernel default to enforcing the same semantics.
> "When mounting a server w/o unix extensions, do we want the client > kernel to default to enforcing mandatory locking between processes on > the same machine?" Yes - I agree we don't want to do enforce mandatory locking on the client, the server already does this as long as we get the file handle and pid right. It is likely that the kernel's mandatory locking behavior (on the client kernel) wouldn't match windows semantics (on the server) anyway
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 4f5a03c..c8f4cc2 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -827,9 +827,9 @@ cifs_parse_mount_options(char *options, const char *devname, vol->target_rfc1001_name[0] = 0; vol->linux_uid = current_uid(); /* use current_euid() instead? */ vol->linux_gid = current_gid(); - vol->dir_mode = S_IRWXUGO; - /* 2767 perms indicate mandatory locking support */ - vol->file_mode = (S_IRWXUGO | S_ISGID) & (~S_IXGRP); + + /* default to only allowing access to owner of the mount */ + vol->dir_mode = vol->file_mode = S_IRWXU; /* vol->retry default is 0 (i.e. "soft" limited retry not hard retry) */ vol->rw = true;
The current default file mode is 02767 and dir mode is 0777. This is extremely "loose". Given that CIFS is a single-user protocol, these permissions allow anyone to use the mount -- in effect, giving anyone on the machine access to the credentials used to mount the share. Change this by making the default permissions restrict access to only the uid of the share. Note that this patch also removes the mandatory locking flags from the default file_mode. After having looked at how these flags are used by the kernel, I don't think that keeping them as the default offers any real benefit. That flag combination makes it so that the kernel enforces mandatory locking. Since the server is going to do that for us anyway, I don't think we want the client to enforce this by default on applications that just want advisory locks. Anyone that does want this behavior can always enable it by setting the file_mode appropriately. Signed-off-by: Jeff Layton <jlayton@redhat.com> --- fs/cifs/connect.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)