Widows says that NULL DACL permits all types of access to all users. Ref: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/secbp/security/creating_a_dacl.asp But in samba its the other way. ------------------------------------------------------------------ NTSTATUS sec_access_check(const struct security_descriptor *sd, const struct security_token *token, uint32_t access_desired, uint32_t *access_granted)/* empty dacl denies access */ : : : if (sd->dacl == NULL || sd->dacl->num_aces == 0) { return NT_STATUS_ACCESS_DENIED; ------------------------------------------------------------------------ Shouldn't it be same as what windows states ie allow access. Infact PCNl 2.0 does that.
It seems that Kumar is right with his thesis. If we would like to match the Windows behaviour, I suggest to change this. I'll write a patch
Created attachment 2887 [details] Here the proposed patch
Any chance you could write a testsuite to prove this? A test that I can run against windows and Samba, and compare would give me the confidence to apply this patch.
If you help me to understand how I should write one I could try it, Andrew! I'm not so familiar yet with the SAMBA source.
You need to find some way (RAW-ACLs test perhaps?) to add an ACL to an object (file, etc), and test what happens when you set an ACL with the NULL DACL.
Created attachment 2895 [details] New smbtorture null dacl test I've written now a new smbtorture test function for that. Andrew, please check her and run the test. I wanted to run her against my Win2k VM and got a segfault. I remembered that I had there only a FAT32 filesystem. So maybe the test should be changed to look first for a NTFS share before it starts.
Have yuo reviewed my patch, abartlet?
Andrew, any chance to review my testsuite and check in it then, so we can prove this (Null Dacl denies access in samba in contradiction to windows)?
I'll try to get someone who understands this stuff to review it at the CIFS interop event.
I assume NULL DACL is different from a DACL with 0 elements, The torture test should also very fine if there's a difference or not. Also the CHECK_ACCESS_FLAGS() macro should be used like in other tests to demonstrate which access bits are granted, with what desired access bits.
Created attachment 2949 [details] New smbtortore null dacl test This patch fixes the segfaults, and starts to fix up the patch, but this is clearly a completely untested patch. I do need you to find a machine that supports ACLs, and finish fixing the test, until it passes.
Created attachment 2950 [details] New smbtortore null dacl test Oops, the previous patch included unwanted cruft
How far are we with this one?
I need you to finish writing the test. As I mention below, I've fixed a few things, but there is still a lot of work to do, and I need you to do it.
I'll be glad if you point me out the items I shall try to integrate into the test.
Make it work, make it pass against windows, make it not segfault. I do need you to find a machine that supports ACLs, and finish fixing the test, until it passes.
Okay. I tried to run the test now again a Windows Vista-Box, where the "nttrans_create_null_dacl" but also the "sd_get_set" test failed. The NULL DACL test seems to fail because of the inheritance feature.
Andrew, I begun to fix up your patch. I chased down the problem with the different DACL's. I'll send it here.
Created attachment 3089 [details] New smbtorture patch Here a new version of the patch with the mentioned correction.
Comment on attachment 2950 [details] New smbtortore null dacl test Andrew, have you looked into this?
This problem should be resolved now. Andrew, do you still prefer to finish the torture suite?
Created attachment 3682 [details] Latest version of smbtorture NULL DACL patch This finally passes successfully against Windows (Vista).
Created attachment 3683 [details] Latest version of smbtorture NULL DACL patch Removed some unused code.
Created attachment 3689 [details] Latest version of smbtorture NULL DACL patch Added some checks on the created DACL.
I've extended the torture test to really test a NULL DACL:-) I've also fixed the server to pass the test. See 67c5aca1e871ccd3675a0cc586753134f76239e9~2, 67c5aca1e871ccd3675a0cc586753134f76239e9~1 and 67c5aca1e871ccd3675a0cc586753134f76239e9 in the master branch