Bug 4284 - Null Dacl denies access in samba in contradiction to windows
Null Dacl denies access in samba in contradiction to windows
Status: RESOLVED FIXED
Product: Samba 4.0
Classification: Unclassified
Component: Other
unspecified
Other Solaris
: P3 normal
: ---
Assigned To: Andrew Bartlett
Andrew Bartlett
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-12-07 00:42 UTC by Kumar
Modified: 2008-10-28 11:21 UTC (History)
2 users (show)

See Also:


Attachments
Here the proposed patch (483 bytes, patch)
2007-08-25 07:40 UTC, Matthias Dieter Wallnöfer
no flags Details
New smbtorture null dacl test (3.54 KB, patch)
2007-08-27 15:45 UTC, Matthias Dieter Wallnöfer
no flags Details
New smbtortore null dacl test (7.00 KB, patch)
2007-10-22 23:46 UTC, Andrew Bartlett
no flags Details
New smbtortore null dacl test (5.50 KB, patch)
2007-10-23 00:15 UTC, Andrew Bartlett
no flags Details
New smbtorture patch (5.55 KB, patch)
2008-01-07 14:30 UTC, Matthias Dieter Wallnöfer
no flags Details
Latest version of smbtorture NULL DACL patch (5.45 KB, patch)
2008-10-18 05:16 UTC, Matthias Dieter Wallnöfer
no flags Details
Latest version of smbtorture NULL DACL patch (4.81 KB, patch)
2008-10-18 05:29 UTC, Matthias Dieter Wallnöfer
no flags Details
Latest version of smbtorture NULL DACL patch (4.19 KB, patch)
2008-10-25 12:36 UTC, Matthias Dieter Wallnöfer
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kumar 2006-12-07 00:42:23 UTC
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.
Comment 1 Matthias Dieter Wallnöfer 2007-08-25 07:35:56 UTC
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
Comment 2 Matthias Dieter Wallnöfer 2007-08-25 07:40:00 UTC
Created attachment 2887 [details]
Here the proposed patch
Comment 3 Andrew Bartlett 2007-08-26 02:29:40 UTC
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. 
Comment 4 Matthias Dieter Wallnöfer 2007-08-26 03:06:30 UTC
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.
Comment 5 Andrew Bartlett 2007-08-26 19:40:01 UTC
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. 

Comment 6 Matthias Dieter Wallnöfer 2007-08-27 15:45:17 UTC
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.
Comment 7 Matthias Dieter Wallnöfer 2007-08-30 01:45:22 UTC
Have yuo reviewed my patch, abartlet?
Comment 8 Matthias Dieter Wallnöfer 2007-09-24 02:57:34 UTC
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)?
Comment 9 Andrew Bartlett 2007-09-24 06:03:02 UTC
I'll try to get someone who understands this stuff to review it at the CIFS interop event.
Comment 10 Stefan Metzmacher 2007-10-05 13:55:38 UTC
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.
Comment 11 Andrew Bartlett 2007-10-22 23:46:00 UTC
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.
Comment 12 Andrew Bartlett 2007-10-23 00:15:18 UTC
Created attachment 2950 [details]
New smbtortore null dacl test

Oops, the previous patch included unwanted cruft
Comment 13 Matthias Dieter Wallnöfer 2007-11-27 14:03:47 UTC
How far are we with this one?
Comment 14 Andrew Bartlett 2007-11-27 15:48:21 UTC
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. 
Comment 15 Matthias Dieter Wallnöfer 2007-11-28 00:59:36 UTC
I'll be glad if you point me out the items I shall try to integrate into the test.
Comment 16 Andrew Bartlett 2007-11-28 02:09:06 UTC
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. 

Comment 17 Matthias Dieter Wallnöfer 2008-01-04 09:35:13 UTC
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.
Comment 18 Matthias Dieter Wallnöfer 2008-01-07 01:38:24 UTC
Andrew, I begun to fix up your patch. I chased down the problem with the different DACL's. I'll send it here.
Comment 19 Matthias Dieter Wallnöfer 2008-01-07 14:30:48 UTC
Created attachment 3089 [details]
New smbtorture patch

Here a new version of the patch with the mentioned correction.
Comment 20 Matthias Dieter Wallnöfer 2008-05-24 07:39:40 UTC
Comment on attachment 2950 [details]
New smbtortore null dacl test

Andrew, have you looked into this?
Comment 21 Matthias Dieter Wallnöfer 2008-09-25 04:18:05 UTC
This problem should be resolved now. Andrew, do you still prefer to finish the torture suite?
Comment 22 Matthias Dieter Wallnöfer 2008-10-18 05:16:02 UTC
Created attachment 3682 [details]
Latest version of smbtorture NULL DACL patch

This finally passes successfully against Windows (Vista).
Comment 23 Matthias Dieter Wallnöfer 2008-10-18 05:29:48 UTC
Created attachment 3683 [details]
Latest version of smbtorture NULL DACL patch

Removed some unused code.
Comment 24 Matthias Dieter Wallnöfer 2008-10-25 12:36:58 UTC
Created attachment 3689 [details]
Latest version of smbtorture NULL DACL patch

Added some checks on the created DACL.
Comment 25 Stefan Metzmacher 2008-10-28 11:21:19 UTC
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