Bug 8620 - Read ACL are not enabled by default on DS
Summary: Read ACL are not enabled by default on DS
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: samba4-qa@samba.org
URL:
Keywords:
Depends on:
Blocks: 8621 8622 9306
  Show dependency treegraph
 
Reported: 2011-11-18 13:23 UTC by Matthieu Patou
Modified: 2012-12-03 19:31 UTC (History)
3 users (show)

See Also:


Attachments
Patches cherry-picked from master for confidential attribute support (26.24 KB, patch)
2012-11-12 01:05 UTC, Andrew Bartlett
metze: review+
Details
Patches for v4-0-test (48.00 KB, patch)
2012-12-03 10:14 UTC, Stefan Metzmacher
obnox: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthieu Patou 2011-11-18 13:23:03 UTC
Read ACL for Directory object is not enabled by default, currently the read is just denied to non authenticated user. 
Supporting real ACL is needed for production use.
The code for the read ACL is here but desactivated due to performance concerns.
Comment 1 Nadezhda Ivanova 2011-11-22 19:14:18 UTC
Actually its not just performance, it causes some failures in make test. I fixed them by using a flag that is only set if the request comes from ldap, but then the flag was changed to mean something else and it messed things up again. The code itself is finished in terms of implementation, but these issues need to be resolved.
Comment 2 Andrew Bartlett 2012-10-18 07:22:31 UTC
There is no way this is going to be changed for a 4.0 release.  Unblocking.
Comment 3 Stefan Metzmacher 2012-11-07 11:43:58 UTC
We need at least protect attributes with searchFlags: fCONFIDENTIAL for 4.0
Comment 4 Matthieu Patou 2012-11-07 20:09:36 UTC
We used to have some code for this but it's off by default,
so maybe we should put it back to on and see what is breaking ?
Comment 5 Andrew Bartlett 2012-11-12 01:05:48 UTC
Created attachment 8182 [details]
Patches cherry-picked from master for confidential attribute support
Comment 6 Stefan Metzmacher 2012-11-12 07:25:10 UTC
Comment on attachment 8182 [details]
Patches cherry-picked from master for confidential attribute support

Looks good
Comment 7 Stefan Metzmacher 2012-11-12 07:26:50 UTC
Karolin, please pick this to v4-0-test and remove it as blocker for bug #8622
and reassigned it to me for the full fix.
Comment 8 Karolin Seeger 2012-11-12 08:20:37 UTC
(In reply to comment #7)
> Karolin, please pick this to v4-0-test and remove it as blocker for bug #8622
> and reassigned it to me for the full fix.

Pushed to autobuild-v4-0-test.
Removed as blocker.
Comment 9 Karolin Seeger 2012-11-12 10:45:13 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!
Comment 10 Stefan Metzmacher 2012-11-19 15:12:59 UTC
I have some patches which allow us to turn on "acl:search=true" without slowing
down make test.

https://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-ad-acls
https://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=9b17dfc1e0515deb25

For now add mark it as blocker for 4.0 until we decide whether we
would like to switch this on by default. At least we should try to
support it optional.
Comment 11 Stefan Metzmacher 2012-11-19 15:13:50 UTC
(In reply to comment #10)
> I have some patches which allow us to turn on "acl:search=true" without slowing
> down make test.
> 
> https://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-ad-acls
> https://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=9b17dfc1e0515deb25

The patches still need some cleanup...
Comment 12 Stefan Metzmacher 2012-12-03 10:14:58 UTC
Created attachment 8256 [details]
Patches for v4-0-test
Comment 13 Michael Adam 2012-12-03 11:46:03 UTC
Comment on attachment 8256 [details]
Patches for v4-0-test

ACK
Comment 14 Michael Adam 2012-12-03 11:46:44 UTC
==> Karolin for 4.0
Comment 15 Karolin Seeger 2012-12-03 12:19:28 UTC
Pushed to autobuild-v4-0-test.
Comment 16 Karolin Seeger 2012-12-03 19:31:12 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!