Bug 5202 - cannot change ACLs on writable file with "dos filemode=yes"
Summary: cannot change ACLs on writable file with "dos filemode=yes"
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.3
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P3 major
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks: 4956 5255
  Show dependency tree / graph
 
Reported: 2008-01-14 23:11 UTC by SATOH Fumiyasu
Modified: 2010-01-11 05:15 UTC (History)
2 users (show)

See Also:
gd: review+

Attachments
Proposed patch: Use can_write_to_file() in acl_group_override() (2.53 KB, patch)
2008-01-14 23:16 UTC, SATOH Fumiyasu 		no flags Details
Fix "dos filemode" and rebirth "acl group control" parameter (4.10 KB, patch)
2008-07-03 02:23 UTC, SATOH Fumiyasu 		no flags Details
git-am format patch for 3.5.0. (1.42 KB, patch)
2010-01-08 12:21 UTC, Jeremy Allison 		no flags Details
git-am fix for 3.4.5. (1.36 KB, patch)
2010-01-08 12:24 UTC, Jeremy Allison 		no flags Details
git-am format patch for 3.3.10 (1.35 KB, patch)
2010-01-08 12:26 UTC, Jeremy Allison 		no flags Details
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description SATOH Fumiyasu 2008-01-14 23:11:22 UTC 
The smb.conf(5) man page said:

dos filemode (S)
  ...
  Enabling this  parameter  allows  a  user  who has write access to
  the file (by whatever means) to modify the permissions (including ACL) on it.
  ...

But a user who is not owner of a file and don't have write privilege on
the file, cannot change permissions and ACLs on the file.
Comment 1 SATOH Fumiyasu 2008-01-14 23:16:07 UTC 
Created attachment 3098 [details]
Proposed patch: Use can_write_to_file() in acl_group_override()

Patch for Samba 3.0.28.
Sorry, I'm newbie in git usage...
Comment 2 SATOH Fumiyasu 2008-01-14 23:20:00 UTC 
s/permissions and ACLs/ACLs/ on summary.
Comment 3 SATOH Fumiyasu 2008-07-03 02:23:12 UTC 
Created attachment 3380 [details]
Fix "dos filemode" and rebirth "acl group control" parameter

Please give me comments.
This patch can be applied to Samba 3.0.30 too.
Comment 4 Karolin Seeger 2008-07-03 06:30:26 UTC 
Patch works for me.
I had issues with setting ACEs if the user/group had write access only by ACEs, not by group or owner rights. This is solved with the patch, too.

Would anybody please review and check in?

Thank you very much! :-)
Comment 5 Jeremy Allison 2008-07-03 12:14:55 UTC 
Looks good to me, I'll take care of this for 3.0.x and 3.2.1.

Jeremy.
Comment 6 Jeremy Allison 2008-07-03 12:29:26 UTC 
Fixed for the above named releases.
Jeremy.
Comment 7 Jeremy Allison 2010-01-08 11:46:22 UTC 
This bug re-occurred for 3.3.x and above, so re-opening.

The reason is that to change a NT ACL we now have to open the file requesting WRITE_DAC and WRITE_OWNER access. The mapping from POSIX "w" to NT permissions in posix_acls doesn't add these bits when "dos filemode = yes", so even though the permission or owner change would be allowed by the POSIX ACL code, the NTCreateX call fails with ACCESS_DENIED now we always check NT permissions first.

Patches to follow for 3.3.x, 3.4.x, 3.5.x.

Jeremy.
Comment 8 Jeremy Allison 2010-01-08 12:21:46 UTC 
Created attachment 5150 [details]
git-am format patch for 3.5.0.
Comment 9 Jeremy Allison 2010-01-08 12:24:37 UTC 
Created attachment 5151 [details]
git-am fix for 3.4.5.
Comment 10 Jeremy Allison 2010-01-08 12:26:13 UTC 
Created attachment 5152 [details]
git-am format patch for 3.3.10
Comment 11 Jeremy Allison 2010-01-08 12:27:06 UTC 
Guenther, please review and test and then re-assign to Karolin for inclusion in the given releases.

Thanks !

Jeremy.
Comment 12 Jeremy Allison 2010-01-08 12:28:23 UTC 
This was also logged as Red Hat bugzilla:

https://bugzilla.redhat.com/show_bug.cgi?id=537165

Jeremy.
Comment 13 Guenther Deschner 2010-01-08 13:16:34 UTC 
tested the v3-3-test variant, looks fine. Thanks!
Comment 14 Guenther Deschner 2010-01-08 13:18:20 UTC 
reassigning to karo for inclusion.
Comment 15 Karolin Seeger 2010-01-11 05:15:00 UTC 
Pushed to v3-5-test, v3-4-test and v3-3-test.
Closing out bug report.

Thanks!