[SSSD] [PATCH 0/1] RFC: Fix group memberships for subdomain users

Sumit Bose sbose at redhat.com
Mon Nov 12 12:30:33 UTC 2012


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.

> 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.

> 
> 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.
> 
> Please review carefully.
> 
> Simo Sorce (1):
>   Refactor the way subdomain accounts are saved
> 
>  src/db/sysdb_search.c            |   17 +++++++++++-
>  src/providers/data_provider_be.c |   11 +++++++
>  src/providers/ipa/ipa_s2n_exop.c |   54 +++++++++++++++++++++++++++++++++++---
>  src/responder/nss/nsssrv_cmd.c   |   36 +++++++++++++++++++++++--
>  src/responder/pac/pacsrv_cmd.c   |   15 +++++++++-
>  src/responder/pac/pacsrv_utils.c |   52 ++++++++++++++++++++----------------
>  src/responder/pam/pamsrv_cmd.c   |   18 ++++++++++++-
>  src/util/domain_info_utils.c     |    2 +-
>  8 files changed, 170 insertions(+), 35 deletions(-)
> 
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel



More information about the sssd-devel mailing list