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.
-------------- next part --------------
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.
****************************************************************************/