Bug 6050 - No write permission if POSIX bits 0 on ZFS written by M$ Office - dos_mode returning r
Summary: No write permission if POSIX bits 0 on ZFS written by M$ Office - dos_mode re...
Status: NEW
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: File Services (show other bugs)
Version: 3.0.32
Hardware: All Solaris
: P3 normal
Target Milestone: none
Assignee: Samba Bugzilla Account
QA Contact: Samba QA Contact
URL: http://lists.samba.org/archive/samba/...
Depends on:
Reported: 2009-01-18 13:47 UTC by Nils Goroll
Modified: 2013-02-18 13:30 UTC (History)
2 users (show)

See Also:

Patch - will discuss it in bug comments (3.24 KB, patch)
2009-01-18 13:49 UTC, Nils Goroll
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nils Goroll 2009-01-18 13:47:20 UTC
Description copied from posting:

Hi all,

I'd appreciate any pointers or advise regarding the following issue with files 
written by M$ Office on Samba 3.0.32 on snv_98 (OpenSolaris) on a ZFS filesystem:

samba share:

         read only = No
         browseable = yes
         writeable = yes

         directory mask = 0770
         create mask = 0770
         delete readonly = Yes
         acl check permissions = False
         vfs objects = zfsacl
         nfs4:mode = special

ZFS aclmode and aclinherit are both passthrough

If userA creates a M$ word file on a directory with these ACLs (note: 
inheritance) ...

drwxrwx---+  2 userA    group1        10 Nov 24 17:25 .

... the file gets properly created with these permissions (because the user's 
primary group is group3)

-rwxrwx---+  1 userA    group3     79258 Nov 24 17:26 f.xlsx

now userB can overwrite the file and we get the following ACL:

----rwx---+  1 userB    group3     35067 Nov 24 17:55 f.xlsx

userA and userB are both members of group1, 2 and 3, group3 being
their primary group.

Now *NO-ONE* can overwrite this file anymore.

The workaround is to either reset permissions or to explicitly add owner 

   chmod A+owner@:rwxpdDaARWc--s:-------:allow <file>

It doesn't seem to matter who the owner is, as long as the owner has non-NULL 
permissions and the ACLs allow permissions for the user in question, the access 
is granted, while it is not if the owner acl does not exist.

When I reproduced the issue with debugging enabled, I noticed this section from 
the log which seemed to be relevant:

(trying to write to test/t.rtf which has the same ACL as above,
  but would allow my user write access by group):

[2008/11/24 17:04:45, 8] smbd/dosmode.c:(371)
   dos_mode: test/t.rtf
[2008/11/24 17:04:45, 8] smbd/dosmode.c:(188)
   dos_mode_from_sbuf returning r
[2008/11/24 17:04:45, 8] smbd/dosmode.c:(409)
   dos_mode returning r
[2008/11/24 17:04:45, 8] lib/util.c:(1844)
   is_in_path: test/t.rtf
[2008/11/24 17:04:45, 8] lib/util.c:(1868)
   is_in_path: match not found
[2008/11/24 17:04:45, 10] smbd/open.c:(852)
   open_match_attributes: file test/t.rtf old_dos_attr = 0x1, existing_unx_mode 
= 0100070, new_dos_attr = 0x0 returned_unx_mode = 00
[2008/11/24 17:04:45, 10] smbd/open.c:(1347)
   open_file_ntcreate: fname=test/t.rtf, after mapping access_mask=0x2019f
[2008/11/24 17:04:45, 5] smbd/open.c:(1399)
   open_file_ntcreate: write access requested for file test/t.rtf on read only file
[2008/11/24 17:04:45, 3] smbd/error.c:(106)
   error packet at smbd/nttrans.c(805) cmd=162 (SMBntcreateX) 

 From the debug output, it looks like the issue was releated to dos_mode returning r

This issue resembles the old "Other user can't overwrite files written with M$ 
Office", but unfortunately the workaround

         force create mode = 0770
         force directory mode = 0770

seems *not* to work with ZFS ACLs any more.

I have googled quite intensively, but could not come up with any pointers to 
this issue on "real ACLs" (ZFS) - only for POSIX semantics.

Again, I'd very much appreciate any pointers or hints.

Comment 1 Nils Goroll 2009-01-18 13:48:32 UTC

* The primary issue here is that dos_mode_from_sbuf basically yields R/O if
  the config parameter "map read only" is Yes and the owner does not have
  write permission (irrespective of who the owner is, but that's something
  for the major refactoring work)

  So the first thing to do is to set

    map read only = Permissions

  to actually check the ACLs.

* The second issue is that various places call file_set_dosmode which in
  turn calls umask(). This will most likely destroy your acl if you
  don't use aclmode = passthrough.

  Some of these cases can be avoided by setting

    map archive = no

* The third and fourth issue are umask based access checks in
  can_delete_file_in_directory() and can_access_file() 
Comment 2 Nils Goroll 2009-01-18 13:49:32 UTC
Created attachment 3881 [details]
Patch - will discuss it in bug comments
Comment 3 Nils Goroll 2009-01-18 14:00:03 UTC
Discussion of the patch:

Jerry commented on the posting:

> Why not use extended attributes to store the read only bit, by
> setting :
>         store dos attributes = yes
>         map system = no
>         map archive = no
>         map readonly = no

He's right - besides map readonly = no, I should also have set
the other bits he is suggesting.

I still think the changes I proposed in the patch are necessary in this
way or another. Here's why:

* Removing primary owner write access check in can_delete_file_in_directory:

  This function is called from reply_ntcreate_and_X in two places,
  which both resemble each other:

        if (lp_acl_check_permissions(SNUM(conn))
            && (create_disposition != FILE_CREATE)
            && (share_access & FILE_SHARE_DELETE)
            && (access_mask & DELETE_ACCESS)) {
                if ((dos_mode(conn, fname, &sbuf) & FILE_ATTRIBUTE_READONLY) ||
                                !can_delete_file_in_directory(conn, fname)) {
                                /* DO SOMETHING */

  If I get this right, none of the attributes Jerry suggested to
  remove special meanings of POSIX mask bits do help here.

  IMHO, removing the owner owner write access check in 
  can_delete_file_in_directory is necessary, as it represents a shortcut
  which will avoid the ACL check. But if we have ACLs, we don't want to
  give any special meaning to the file owner.

* A similar argumentation holds for the can_access_file change I proposed.

  What I don't quite like about my suggestion is that it ties the shortcut
  to MAP_READONLY_YES. I think A cleaner solution would be to completely
  remove the "primary owner shortcut", or to introduce a config variable
  whether we want such shortcuts (which could also be used for the case

At any rate, I'd be glad if someone from the core team would consider these
points again.

Cheers, Nils
Comment 4 Nils Goroll 2009-04-22 05:21:52 UTC
I would appreciate if someone would review the suggested patch.
If I am wrong, I would like to understand, why.
Comment 5 Nicolas Dorfsman (mail address dead) 2009-04-22 11:19:15 UTC
Posted some comments on