[SSSD] [PATCH 0/3] Fix group memberships for subdomain users

Sumit Bose sbose at redhat.com
Thu Nov 15 10:52:12 UTC 2012


On Wed, Nov 14, 2012 at 12:30:24PM -0500, Simo Sorce wrote:
> This patch changes the way subdomain users are stored in the database.
> 
> 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
> 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.
> 
> 
> With this change we solve one of the most eveident issues we currently have
> where subdomain users are not listed fully qualified in group membership when
> they should.
> 
> 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.
> 
> Database migration has been implemented as well and it consist in simply
> dropping any content from the subdomain. Unfortunately the memberof plugin
> does not implement support for renaming users and adding all that complexity
> seem not worth for a one time change.
> 
> The side effect is that subdomain users may loose local login access because
> their cached credentials are dropped. Having subdomain users log in locally
> when offline is not very common yet so the impact of this side effect should
> be low and reasonable.
> 
> 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.

I can confirm this. I've also done some update tests which worked well
as well.

Nevertheless I have some general comments.

I would recommend to add wrapper functions for sysdb_getpwnam() and
sysdb_getpwnam() doing something like 


+    if (sysdb->domain->parent) {
+        src_name = talloc_asprintf(mem_ctx, sysdb->domain->names->fq_fmt,
+                                   name, sysdb->domain->name);
+        if (!src_name) {
+            return ENOMEM;
+        }
+        ret = sysdb_getpwnam(tmp_ctx, sysdb, src_name, &res);
+        talloc_free(src_name);
+    } else {
+        ret = sysdb_getpwnam(tmp_ctx, sysdb, name, &res);
+    }
+
+    return ret;

This would make the main code cleaner and would avoid talloc_zfree with
const char issue as well.

The UPN is not constructed in the krb5_child but in the main krb5
responder code by find_or_guess_upn() so in general the regex would be
available and can be used there. But I'm thinking of a different way.
What about store the plain name without the domain component for
subdomain users in an attribute like flatName or originalUserName and
use this in find_or_guess_upn() if no UPN is stored for the user?

bye,
Sumit
> 
> An additional patch from Sumit to fix an improper placement of HABC rules is
> also added, the issue was laregely cosmetic and is handled by the migration
> code as well.
> 
> 
> This patchset depends on the previous patchset named: "Simplify writing update
> functions", avilable here: https://patchwork.acksyn.org/patch/336/
> 
> 
> This patchset fixes: https://fedorahosted.org/sssd/ticket/1629
> 
> 
> Simo Sorce (2):
>   Refactor the way subdomain accounts are saved
>   Handle conversion to fully qualified usernames
> 
> Sumit Bose (1):
>   Do not save HBAC rules in subdomain subtree
> 
>  src/db/sysdb.c                      |    7 +++
>  src/db/sysdb_private.h              |    4 +-
>  src/db/sysdb_search.c               |   17 ++++++-
>  src/db/sysdb_upgrade.c              |   88 +++++++++++++++++++++++++++++++++++
>  src/providers/data_provider_be.c    |   11 ++++
>  src/providers/ipa/ipa_access.c      |   10 ----
>  src/providers/ipa/ipa_hbac_common.c |   19 ++++++-
>  src/providers/ipa/ipa_s2n_exop.c    |   54 ++++++++++++++++++++--
>  src/providers/ldap/sdap_access.c    |   19 ++++++-
>  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 +-
>  14 files changed, 300 insertions(+), 52 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