On Wed, Mar 23, 2005 at 10:18:23AM -0500, Nathan Vidican wrote:
> Problem is apparently with locking issues, disabled oplocks in the [general]
> section, and the problem actually got worse...
> 
> Here's what happens:
> 
> User-A part of group1, opens Excel file off of share, saves, exits...
> User-B (or even User-A for that matter) tries to re-open same file, get
> error stating it's locked and can only open for read-only access...
> 
> Both users in the same group, and share definition looks like this:
> 
> [mysharename]
> path = /server/some/dir
> read only = no
> Valid users = @group1
> Write list = @group1
> Force group = group1
> 
> (general section defines create files as mode 0660, and directories as 0770
> (which works - apparently)
> 
> I have upgraded to 3.0.12, and the problem persists. We're running short on
> options, and I really don't know what to do from here... ANY suggestion
> would be greatly appreciated, if more information is required just let me
> know, figured before I spun wheels I'd make note of what's going on first.
> We're running two samba servers using ldap backend for domain/users/etc.

Ok, I have a working theory for this. It concerns ACLs and what happens
when excel wants to update the filetime on a file the user doesn't own.

Normally you just set the "dos filetime" parameter to allow this (this
causes a timestamp to be updated on a file if you can write to it - normally
POSIX only allows this if you're the owner). I've realised the codepath
here doesn't check ACL semantics. This is a bug we've had since we
introduced ACLs a long time ago but only now seems to have been triggered.

Here is a patch to the just released 3.0.13 that causes ACL entries to be
properly checked when "dos filetime= True" has been set.

Please try this on top of 3.0.13 and let me know if it fixes the issues.

Jeremy.
Index: smbd/posix_acls.c
===================================================================
--- smbd/posix_acls.c   (revision 6045)
+++ smbd/posix_acls.c   (working copy)
@@ -3758,23 +3758,27 @@
  Check for POSIX group ACLs. If none use stat entry.
 ****************************************************************************/
 
-static int check_posix_acl_group_write(connection_struct *conn, const char 
*dname, SMB_STRUCT_STAT *psbuf)
+static int check_posix_acl_group_write(connection_struct *conn, const char 
*fname, SMB_STRUCT_STAT *psbuf)
 {
        extern struct current_user current_user;
        SMB_ACL_T posix_acl = NULL;
        int entry_id = SMB_ACL_FIRST_ENTRY;
        SMB_ACL_ENTRY_T entry;
        int i;
+       BOOL seen_mask = False;
        int ret = -1;
 
-       if ((posix_acl = SMB_VFS_SYS_ACL_GET_FILE(conn, dname, 
SMB_ACL_TYPE_ACCESS)) == NULL) {
+       if ((posix_acl = SMB_VFS_SYS_ACL_GET_FILE(conn, fname, 
SMB_ACL_TYPE_ACCESS)) == NULL) {
                goto check_stat;
        }
 
        /* First ensure the group mask allows group read. */
+       /* Also check any user entries (these take preference over group). */
+
        while ( SMB_VFS_SYS_ACL_GET_ENTRY(conn, posix_acl, entry_id, &entry) == 
1) {
                SMB_ACL_TAG_T tagtype;
                SMB_ACL_PERMSET_T permset;
+               int have_write = -1;
 
                /* get_next... */
                if (entry_id == SMB_ACL_FIRST_ENTRY)
@@ -3788,20 +3792,51 @@
                        goto check_stat;
                }
 
+               have_write = SMB_VFS_SYS_ACL_GET_PERM(conn, permset, 
SMB_ACL_WRITE);
+               if (have_write == -1) {
+                       goto check_stat;
+               }
+
                switch(tagtype) {
                        case SMB_ACL_MASK:
-                               if (!SMB_VFS_SYS_ACL_GET_PERM(conn, permset, 
SMB_ACL_WRITE)) {
-                                       /* We don't have group write 
permission. */
+                               if (!have_write) {
+                                       /* We don't have any group or explicit 
user write permission. */
                                        ret = -1; /* Allow caller to check 
"other" permissions. */
+                                       DEBUG(10,("check_posix_acl_group_write: 
file %s \
+refusing write due to mask.\n", fname));
                                        goto done;
                                }
+                               seen_mask = True;
                                break;
+                       case SMB_ACL_USER:
+                       {
+                               /* Check against current_user.uid. */
+                               uid_t *puid = (uid_t 
*)SMB_VFS_SYS_ACL_GET_QUALIFIER(conn, entry);
+                               if (puid == NULL) {
+                                       goto check_stat;
+                               }
+                               if (current_user.uid == *puid) {
+                                       /* We have a uid match but we must 
ensure we have seen the acl mask. */
+                                       ret = have_write;
+                                       DEBUG(10,("check_posix_acl_group_write: 
file %s \
+match on user %u -> %s.\n", fname, (unsigned int)*puid, ret ? "can write" : 
"cannot write"));
+                                       if (seen_mask) {
+                                               goto done;
+                                       }
+                               }
+                               break;
+                       }
                        default:
                                continue;
                }
        }
 
-       /* Now check all group entries. */
+       /* If ret is anything other than -1 we matched on a user entry. */
+       if (ret != -1) {
+               goto done;
+       }
+
+       /* Next check all group entries. */
        entry_id = SMB_ACL_FIRST_ENTRY;
        while ( SMB_VFS_SYS_ACL_GET_ENTRY(conn, posix_acl, entry_id, &entry) == 
1) {
                SMB_ACL_TAG_T tagtype;
@@ -3826,35 +3861,23 @@
                }
 
                switch(tagtype) {
-                       case SMB_ACL_USER:
-                               {
-                                       /* Check against current_user.uid. */
-                                       uid_t *puid = (uid_t 
*)SMB_VFS_SYS_ACL_GET_QUALIFIER(conn, entry);
-                                       if (puid == NULL) {
-                                               goto check_stat;
-                                       }
-                                       if (current_user.uid == *puid) {
-                                               /* We're done now we have a uid 
match. */
+                       case SMB_ACL_GROUP:
+                       {
+                               gid_t *pgid = (gid_t 
*)SMB_VFS_SYS_ACL_GET_QUALIFIER(conn, entry);
+                               if (pgid == NULL) {
+                                       goto check_stat;
+                               }
+                               for (i = 0; i < current_user.ngroups; i++) {
+                                       if (current_user.groups[i] == *pgid) {
+                                               /* We're done now we have a gid 
match. */
                                                ret = have_write;
+                                               
DEBUG(10,("check_posix_acl_group_write: file %s \
+match on group %u -> %s.\n", fname, (unsigned int)*pgid, ret ? "can write" : 
"cannot write"));
                                                goto done;
                                        }
                                }
                                break;
-                       case SMB_ACL_GROUP:
-                               {
-                                       gid_t *pgid = (gid_t 
*)SMB_VFS_SYS_ACL_GET_QUALIFIER(conn, entry);
-                                       if (pgid == NULL) {
-                                               goto check_stat;
-                                       }
-                                       for (i = 0; i < current_user.ngroups; 
i++) {
-                                               if (current_user.groups[i] == 
*pgid) {
-                                                       /* We're done now we 
have a gid match. */
-                                                       ret = have_write;
-                                                       goto done;
-                                               }
-                                       }
-                               }
-                               break;
+                       }
                        default:
                                continue;
                }
@@ -3877,7 +3900,7 @@
 }
 
 /****************************************************************************
- Actually emulate the in-kernel access checking for write access. We need
+ Actually emulate the in-kernel access checking for delete access. We need
  this to successfully return ACCESS_DENIED on a file open for delete access.
 ****************************************************************************/
 
@@ -3888,6 +3911,11 @@
        pstring dname;
        int ret;
 
+       if (!CAN_WRITE(conn)) {
+               return False;
+       }
+
+       /* Get the parent directory permission mask and owners. */
        pstrcpy(dname, parent_dirname(fname));
        if(SMB_VFS_STAT(conn, dname, &sbuf) != 0) {
                return False;
@@ -3895,11 +3923,12 @@
        if (!S_ISDIR(sbuf.st_mode)) {
                return False;
        }
-       if (current_user.uid == 0) {
+       if (current_user.uid == 0 || conn->admin_user) {
                /* I'm sorry sir, I didn't know you were root... */
                return True;
        }
 
+       /* Check primary owner write access. */
        if (current_user.uid == sbuf.st_uid) {
                return (sbuf.st_mode & S_IWUSR) ? True : False;
        }
@@ -3918,11 +3947,52 @@
        }
 #endif
 
-       /* Check group ownership. */
+       /* Check group or explicit user acl entry write access. */
        ret = check_posix_acl_group_write(conn, dname, &sbuf);
        if (ret == 0 || ret == 1) {
                return ret ? True : False;
        }
 
+       /* Finally check other write access. */
        return (sbuf.st_mode & S_IWOTH) ? True : False;
 }
+
+/****************************************************************************
+ Actually emulate the in-kernel access checking for write access. We need
+ this to successfully check for ability to write for dos filetimes.
+****************************************************************************/
+
+BOOL can_write_to_file(connection_struct *conn, const char *fname)
+{
+       extern struct current_user current_user;
+       SMB_STRUCT_STAT sbuf;  
+       int ret;
+
+       if (!CAN_WRITE(conn)) {
+               return False;
+       }
+
+       if (current_user.uid == 0 || conn->admin_user) {
+               /* I'm sorry sir, I didn't know you were root... */
+               return True;
+       }
+
+       /* Get the file permission mask and owners. */
+       if(SMB_VFS_STAT(conn, fname, &sbuf) != 0) {
+               return False;
+       }
+
+       /* Check primary owner write access. */
+       if (current_user.uid == sbuf.st_uid) {
+               return (sbuf.st_mode & S_IWUSR) ? True : False;
+       }
+
+       /* Check group or explicit user acl entry write access. */
+       ret = check_posix_acl_group_write(conn, fname, &sbuf);
+       if (ret == 0 || ret == 1) {
+               return ret ? True : False;
+       }
+
+       /* Finally check other write access. */
+       return (sbuf.st_mode & S_IWOTH) ? True : False;
+}
Index: smbd/dosmode.c
===================================================================
--- smbd/dosmode.c      (revision 6045)
+++ smbd/dosmode.c      (working copy)
@@ -431,10 +431,8 @@
  than POSIX.
 *******************************************************************/
 
-int file_utime(connection_struct *conn, char *fname, struct utimbuf *times)
+int file_utime(connection_struct *conn, const char *fname, struct utimbuf 
*times)
 {
-       extern struct current_user current_user;
-       SMB_STRUCT_STAT sb;
        int ret = -1;
 
        errno = 0;
@@ -454,21 +452,12 @@
           (as DOS does).
         */
 
-       if(SMB_VFS_STAT(conn,fname,&sb) != 0)
-               return -1;
-
        /* Check if we have write access. */
-       if (CAN_WRITE(conn)) {
-               if (((sb.st_mode & S_IWOTH) || conn->admin_user ||
-                       ((sb.st_mode & S_IWUSR) && current_user.uid==sb.st_uid) 
||
-                       ((sb.st_mode & S_IWGRP) &&
-                               in_group(sb.st_gid,current_user.gid,
-                                       
current_user.ngroups,current_user.groups)))) {
-                       /* We are allowed to become root and change the 
filetime. */
-                       become_root();
-                       ret = SMB_VFS_UTIME(conn,fname, times);
-                       unbecome_root();
-               }
+       if (can_write_to_file(conn, fname)) {
+               /* We are allowed to become root and change the filetime. */
+               become_root();
+               ret = SMB_VFS_UTIME(conn,fname, times);
+               unbecome_root();
        }
 
        return ret;
@@ -478,7 +467,7 @@
  Change a filetime - possibly allowing DOS semantics.
 *******************************************************************/
 
-BOOL set_filetime(connection_struct *conn, char *fname, time_t mtime)
+BOOL set_filetime(connection_struct *conn, const char *fname, time_t mtime)
 {
        struct utimbuf times;
 
Index: lib/util.c
===================================================================
--- lib/util.c  (revision 6045)
+++ lib/util.c  (working copy)
@@ -274,24 +274,6 @@
 }
 
 /****************************************************************************
- Determine whether we are in the specified group.
-****************************************************************************/
-
-BOOL in_group(gid_t group, gid_t current_gid, int ngroups, const gid_t *groups)
-{
-       int i;
-
-       if (group == current_gid)
-               return(True);
-
-       for (i=0;i<ngroups;i++)
-               if (group == groups[i])
-                       return(True);
-
-       return(False);
-}
-
-/****************************************************************************
  Add a gid to an array of gids if it's not already there.
 ****************************************************************************/
 
-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/listinfo/samba

Reply via email to