On Mon, Nov 12, 2012 at 01:30:33PM +0100, Sumit Bose wrote: > On Sat, Nov 10, 2012 at 10:05:36PM -0500, Simo Sorce wrote: > > This patch changes the way subdomain users are stored in the database. > > Thank you for the patch. > > I run couple of test and have not see an issue so far. But I have a > couple of comments, see below. > > > > > The reason for changing the way we do it is that the sysdb code, before the > > subdomain patches were added assumed a single domain per cache file. This > > assumption beled in many other interfaces including the way users are read > > and > > returned in the nss responder, as well as potentially how hbac and sudo > > handle > > At least IPA HBAC is using the originalMemberOf attribute to find out > about group memberships efficiently.
Also native IPA netgroups and SELinux rules. > > > rules for checking if users are part of a rule. > > > > In order to make sure subdomain users are univocally recognized as such the > > safest way is to change how users are saved and always save subdomain users > > with sully qualified names. > > Is your plan to store the fully qualified name for users of the > configured domain, too? > > > > > > > With this change we solve one of the most eveident issues we currently have > > where subdomain users are not listed fully wualified in group membership > > when > > they should. > > yes, this works in my tests with you patch now. > > > > > The side effect of this change is that cache files need to be removed if the > > admin decides to change the formatting string for representing fully > > qualified > > users. An action like this has many other important consequences on the > > system > > so I think this limitation is perfectly reasonable. > > Wouldn't is be better to use a generic fully qualified name here and > transform it to the configured style if needed? I know you are afraid > that it will cost too much performance in e.g. fill_members() but to > evaluate this I've written a test for fill_members() to see where we > are: > > Running suite(s): nss > Testcase [0], members [1], fqdn [false], case-sensitive [false], body length > [6], duration [0s 73us] > Testcase [1], members [10], fqdn [false], case-sensitive [false], body length > [60], duration [0s 12us] > Testcase [2], members [100], fqdn [false], case-sensitive [false], body > length [600], duration [0s 104us] > Testcase [3], members [1000], fqdn [false], case-sensitive [false], body > length [6000], duration [0s 512us] > Testcase [4], members [10000], fqdn [false], case-sensitive [false], body > length [60000], duration [0s 4713us] > Testcase [5], members [100000], fqdn [false], case-sensitive [false], body > length [949624], duration [0s 61037us] > Testcase [6], members [1], fqdn [true], case-sensitive [false], body length > [18], duration [0s 45us] > Testcase [7], members [10], fqdn [true], case-sensitive [false], body length > [180], duration [0s 22us] > Testcase [8], members [100], fqdn [true], case-sensitive [false], body length > [1800], duration [0s 120us] > Testcase [9], members [1000], fqdn [true], case-sensitive [false], body > length [18000], duration [0s 794us] > Testcase [10], members [10000], fqdn [true], case-sensitive [false], body > length [180000], duration [0s 7404us] > Testcase [11], members [100000], fqdn [true], case-sensitive [false], body > length [2149624], duration [0s 90789us] > Testcase [12], members [1], fqdn [false], case-sensitive [true], body length > [6], duration [0s 8us] > Testcase [13], members [10], fqdn [false], case-sensitive [true], body length > [60], duration [0s 12us] > Testcase [14], members [100], fqdn [false], case-sensitive [true], body > length [600], duration [0s 36us] > Testcase [15], members [1000], fqdn [false], case-sensitive [true], body > length [6000], duration [0s 149us] > Testcase [16], members [10000], fqdn [false], case-sensitive [true], body > length [60000], duration [0s 1321us] > Testcase [17], members [100000], fqdn [false], case-sensitive [true], body > length [949624], duration [0s 13968us] > Testcase [18], members [1], fqdn [true], case-sensitive [true], body length > [18], duration [0s 14us] > Testcase [19], members [10], fqdn [true], case-sensitive [true], body length > [180], duration [0s 29us] > Testcase [20], members [100], fqdn [true], case-sensitive [true], body length > [1800], duration [0s 86us] > Testcase [21], members [1000], fqdn [true], case-sensitive [true], body > length [18000], duration [0s 478us] > Testcase [22], members [10000], fqdn [true], case-sensitive [true], body > length [180000], duration [0s 4320us] > Testcase [23], members [100000], fqdn [true], case-sensitive [true], body > length [2149624], duration [0s 44809us] > 100%: Checks: 1, Failures: 0, Errors: 0 > > I think the values are quite comfortable and would allow some extra > processing in fill_members(). I plan to test if extracting name and > domain from the DN stored in the member attribute would spoil those > values of if it can be done efficiently. > > > > > This patch is in RFC status because I haven't dealt with database migrations > > to fix existing subdomain users. Would it be acceptable to simply remove > > all subdomain user entries on upgrade ? > > I would say yes. We only have to remember to give it a prominent place > in the release notes to warn about the loss of cached password. +1 given that not many people use the trusted domains yet, this is still OK to do. Better now than later.. > > > > > Also in order to fix this important issue for 1.9 I have refrained from > > significantly changing sysdb interfaces or other code around domain > > manipulation. I think such changes are necessary in future. The consequence > > of not changing sysdb interfaces is that knowledge of the fact the > > subdonains > > users need to be fully qualified bleeds in various other callers. I hope I > > caught all the callers that need to know about this difference but I haven't > > yet checked sudo related code for example. > > > > The patch is so far fully tested and allows password based logins with full > > HABC checking. getent passwd/group commands also return the extected > > outputs. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
