As pointed out by an OEM, the code within smbd/posix_acl.c, even though passed a const pointer to a security descriptor, still modifies the ACE entries within it (which are not const pointers). This means ACLs stored in the extended attribute by the acl_xattr module have already been modified by the POSIX acl layer, and are not the original intent of storing the "unmodified" ACL from the client. Simple patch for 3.5.next to follow. Jeremy.
Created attachment 6000 [details] git-am patch for 3.5.next Volker please check and re-assign to Karolin if you're ok with it. Jeremy.
Comment on attachment 6000 [details] git-am patch for 3.5.next Looks simple enough.
Created attachment 6043 [details] ACL jumbo patch for 3.5.7. tgz of git-am format patch list. Karolin, here is the ACL jumbo patch for 3.5.7. I'm going to use this bug as the placeholder for this patch (as requested). Jeremy.
*** Bug 7733 has been marked as a duplicate of this bug. ***
*** Bug 7734 has been marked as a duplicate of this bug. ***
Jeremy please do not mark as patch when it is a tgz.
Pushed to v3-5-test. Closing out bug report. Thanks!
Karolin - the patches in the ACL jumbo patch for 3.5.7 attachment also need to be applied before we close this one out. That's the tgz attachment 6043 [details] on this bug. Thanks, Jeremy.
Volker, is your review flag also valid for the jumbo patch?
Created attachment 6070 [details] git-am tgz of new jumbo patch. This is a new version of the jumbo patch for 3.5.x that applies on top of the first patch that Karolin has already applied. This contains one extra fix discovered in testing at an OEM. "commit ce27f98705f564dfb2f5c43fb061da9e89f1f3dc Author: Jeremy Allison <jra@samba.org> Date: Wed Nov 17 15:58:15 2010 -0800 Fix our privileges code to display privileges with the "high" 32-bit value set. SeSecurityPrivilege is the first LUID we have added that has a non-zero "high" value, ensure our LUID code correctly supports it. Jeremy." Volker, please review then re-assign to Karolin once you're ok with it. Jeremy.
Created attachment 6073 [details] git am jumbo patch Replace the SeSecurityPrivilege LUID with the correct value (low = 0x8, high = 0), otherwise - same as previous patch. Jeremy.
Comment on attachment 6073 [details] git am jumbo patch Jeremy, when you upload a tar.gz file, please do not set "patch" in the type field. This makes bugzilla set the mime-type so that the browser will not download the file but display it as garbage. If you want it to be displayed directly, you could just upload the output of "git format-patch --stdout", which is semantically exactly the same as the .tar.gz, due to missing compression it is just a bit larger. If you insist on putting in individual patch files (why is that required in this case, git format-patch --stdout would also do it), then please set the mime type to application/octet-stream. Thanks, Volker
Karolin, sorry, I know this is confusing, but final versions of the Jumbo ACL patch keep flowing in on a frequent basis. They all look sane, what I have just acked looks very sane to me. I do not have tested all the test cases that Jeremy's OEM has tested (Jeremy, do you have the complete list somewhere?), so I can't really say this is the final word. But what I can say is that this patch is probably better than what we have in 3.5.6, so I would think it is worth being put into the next 3.5 release. Volker
Sorry about the marking as a patch, I'll try and remember the next time not to do that for a .tgz file. The difference between the last two versions of the patch are changing the SeSecurityPrivilege LUID from {8,0} to {0,8} (which is correct) so not much changed there. The tests performed were passing the smbtorture4 RAW-ACL test, and also passing a test using ROBOCOPY at an OEM site (don't have details of the ACLs being copied in the second test, sorry). Hope that helps clarify things. Jeremy.
NB. Once the jumbo patch is applied we also need the patch in bug:https://bugzilla.samba.org/show_bug.cgi?id=7812 to complete the fixes for ACLs. Jeremy.
(In reply to comment #11) > Created an attachment (id=6073) [details] > git am jumbo patch > > Replace the SeSecurityPrivilege LUID with the correct value (low = 0x8, high = > 0), otherwise - same as previous patch. > Jeremy. > Pushed to v3-5-test. Closing out bug report. Thanks!
(In reply to comment #15) > NB. Once the jumbo patch is applied we also need the patch in > > bug:https://bugzilla.samba.org/show_bug.cgi?id=7812 > > to complete the fixes for ACLs. > > Jeremy. > Done.