Even though setting "inherit permissions = yes" correctly sets the owner of a newly created file, the code in modules/vfs_acl_common will overwrite it with the owner sid from the currently connected user token.
Jeremy.

It turns out that the vfs_acl_common code doesn't actually have to handle the CHOWN or FCHOWN calls, as these are currently made only from codepaths triggered by "inherit owner = yes", which we will handle inside the inherit_new_acl() code inside modules/vfs_acl_common.c.
I will post a small patch for 3.5.next here, and post a larger patch set for metze to review for master/v3-6-test on samba-technical.
Jeremy.

Created attachment 6402[details]
git-am fix for 3.5.next
Metze - please review. If you're ok with this I'll create a similar fix for master/v3-6-test. For master v-3-6-test I'm working on adding more secure versions of the chown_fsp call, to reduce the risk of symlink races on pathnames when root, but this is a separate change to this fix.
"If "inherit owner = yes", pass in the directory owner and group
owner as the target for CREATOR_OWNER and CREATOR_GROUP substitutions,
and also as the owner and primary group of the new security descriptor
being applied to the object."
Jeremy.

Jeremy, thank you for the patch!
I applied it to my test env and did the tests again.
I've a test directory, owned by root:root and the ace which allows my user bbaumba to write to the dir.
I can create files and sub directories in the directory and the ownership is inherited to root:root without any errors on the Windows client side, great!
But if I create a file (not a directory), there are NT_STATUS_ACCESS_DENIED messages on first open_acl_common() call in Sambas log file. After playing with the NT acls of the parent on windows side (added inheritance to sub dirs/files and removed the rule to delete and change the ownership) I get files with -r--rwx---+ root root permissions and ACCESS_DENIED messages if I read the NT ACLS with Windows. But I can see bbaumba's permissions and get no errors on windows side.

I did some additional tests.
I created a directory and set the ownership to
root:bbaumba
With Windows I added the rules for the group, that allows users to create, write, read, ... and NOT to delete files or change the permissions/ownership (and inherit them).
getfacl tells me now
default:group:bbaumba:rwx
which is correct.
I can now create files in the directory, can NOT remove them (fine), but unfortunately not write to them. If I edit a text file and try to safe my changes Samba says:
[2011/04/13 16:39:34.419047, 5] smbd/open.c:1769(open_file_ntcreate)
open_file_ntcreate: write access requested for file testdir3/Neu Textdokument (5).txt on read only file
[2011/04/13 16:39:34.419069, 5] smbd/files.c:497(file_free)
freed files structure 18316 (1 used)
[2011/04/13 16:39:34.419085, 10] smbd/open.c:3209(create_file_unixpath)
create_file_unixpath: NT_STATUS_ACCESS_DENIED
[2011/04/13 16:39:34.419100, 10] smbd/open.c:3462(create_file_default)
create_file: NT_STATUS_ACCESS_DENIED
But getfacl says it should be allowed:
# owner: root
# group: bbaumba
user::r-x
group::rwx
other::r-x
Same with smbcacl and Windows:
OWNER:BBASDF\root
GROUP:Unix Group\bbaumba
ACL:Unix Group\bbaumba:ALLOWED/0x0/RWX

This error:
"open_file_ntcreate: write access requested for file testdir3/Neu Textdokument (5).txt on read only file"
is a check on DOS attributes, not on the permissions of a file. Using a GUI text editor may be trying to do other things than simply open the file for write. You might want to make sure you also have WRITE_ATTRIBUTES set on the inherited ACL, as well as WRITE_DATA.
Don't use a GUI test, try just doing:
copy file.txt \\server\dropbox_share\dir
and then:
echo >>\\server\dropbox_share\dir\file.txt
to test this. That should match more what a dropbox can realistically do.
Remember, trying to use complex apps. on this will fail, as things like MS-Office do:
create new file with tmp name
write data
rename tmp name -> orig name
when editing a file. The "rename" here will fail, as it requires DELETE access to the file (which the dropbox denies).
Dropboxes are limited. I still think this patch is good :-).
Jeremy.

Created attachment 6417[details]
git-am fix for 3.5.next
Karolin - please try this version (should apply to v3-5-test cleanly). It should be identical to the version that won't apply (don't know why that failed).
Jeremy.

Very strange. It fixes it perfectly for me (plus it also occurred on files).
What settings do you have in the share stanza for your smb.conf ? I have:
path = /tmp/foo
vfs objects = acl_xattr
inherit permissions = yes
inherit acls = yes
inherit owner = yes
read only = no
guest ok = true
The logic added should certainly fix this issue. Can you describe more how you're reproducing this ?
Jeremy.

Oh, then that is a new bug. Look at the title of this bug :
""inherit owner = yes" doesn't interact correctly with vfs_acl_xattr or vfs_acl_tdb module"
Kind of implies the vfs_acl is part of the bug report :-).
Ok, please open a new bug - the logic changes here are certainly needed for use with acl_xattr. The new bug can track the posix acl-only problem.
Please confirm the fix on this bug works with acl_xattr, and we can get this change into 3.5.9 and 3.6.0-final.
Jeremy.

(In reply to comment #28)
> (In reply to comment #27)
> > Re-assigning to Karolin.
> >
> > Karolin, patch :
> >
> > https://attachments.samba.org/attachment.cgi?id=6541
>
> Unfortunately, it's too late for 3.5.9.
> Do we need to delay the release or is shipping the fix with 3.5.10 ok?
Pushed to v3-5-test.
If there are no objections, it will be included in 3.5.9.
Closing out bug report.
Thanks!