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 wualified 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.
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 ?
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(-)
The original sysdb code had a strong assumption that only users from one domain are saved in the databse, with the subdomain feature, we have changed reality, but have not adjusted all the code arund the sysdb calls to not rely on the original assumption.
One of the side effects of this incongrunece is that currently group memberships do not return fully qualified names for subdomain users as they should.
In oreder to fix this and other potential issues surrounding the violation of the original assumption, we need to fully qualify subdomain user names. By savin them fully qualified we do not risk aliasing local users and have group memberhips or other name based matching code mistake a domain user with subdomain usr or vice versa. --- 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(-)
diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c index 1ab94770003a205485488c879c9deba856183291..cefac216809c0fec8e266a5f51d00db8dbf1bf44 100644 --- a/src/db/sysdb_search.c +++ b/src/db/sysdb_search.c @@ -358,6 +358,7 @@ int sysdb_initgroups(TALLOC_CTX *mem_ctx, struct ldb_control **ctrl; struct ldb_asq_control *control; static const char *attrs[] = SYSDB_INITGR_ATTRS; + char *src_name; int ret;
tmp_ctx = talloc_new(NULL); @@ -365,13 +366,27 @@ int sysdb_initgroups(TALLOC_CTX *mem_ctx, return ENOMEM; }
- ret = sysdb_getpwnam(tmp_ctx, sysdb, name, &res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + if (sysdb->domain->parent) { + src_name = talloc_asprintf(mem_ctx, sysdb->domain->names->fq_fmt, + name, sysdb->domain->name); + if (!src_name) return ENOMEM; + } else { + src_name = name; + } + + ret = sysdb_getpwnam(tmp_ctx, sysdb, src_name, &res); if (ret != EOK) { DEBUG(1, ("sysdb_getpwnam failed: [%d][%s]\n", ret, strerror(ret))); goto done; }
+ if (name != src_name) { + talloc_zfree(src_name); + } + if (res->count == 0) { /* User is not cached yet */ *_res = talloc_steal(mem_ctx, res); diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index 0ac83f43be797555388d2a2dc1c41a25ddd53c05..6bfb3bc17d6259ebd455e1296c6c203d2e849c9f 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -2186,6 +2186,17 @@ int be_process_init(TALLOC_CTX *mem_ctx, goto fail; }
+ /* We need this for subdomains support, as they have to store fully + * qualified user and group names for now */ + ret = sss_names_init(ctx->domain, cdb, + ctx->domain->name, &ctx->domain->names); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + ("fatal error setting fully qualified name format for %s\n", + ctx->domain->name)); + goto fail; + } + ret = be_srv_init(ctx); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, ("fatal error setting up server bus\n")); diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index 1a81c860901dc18a3d443266889701a8980fe031..8fc22819bf72eb194563da1bb3ded2678ac494e6 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -591,6 +591,9 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) uint64_t timeout = 10*60*60; /* FIXME: find a better timeout ! */ const char *homedir = NULL; struct sysdb_attrs *user_attrs = NULL; + char *name; + char *realm; + char *upn;
ret = ipa_s2n_exop_recv(subreq, state, &result, &retoid, &retdata); talloc_zfree(subreq); @@ -640,21 +643,64 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) goto done; }
- ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, - attrs->a.user.pw_name); + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.user.pw_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, name); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); + goto done; + } + + /* We also have to store a fake UPN here, because otherwise the + * krb5 child later won't be able to properly construct one as + * the username is fully qualified but the child doesn't have + * access to the regex to deconstruct it */ + /* FIXME: The real UPN is available from the PAC, we should get + * it from there. */ + realm = get_uppercase_realm(state, state->dom->name); + if (!realm) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to get realm.\n")); + ret = ENOMEM; + goto done; + } + upn = talloc_asprintf(state, "%s@%s", + attrs->a.user.pw_name, realm); + if (!upn) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format UPN.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_UPN, upn); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); goto done; }
- ret = sysdb_store_domuser(state->dom, attrs->a.user.pw_name, NULL, + ret = sysdb_store_domuser(state->dom, name, NULL, attrs->a.user.pw_uid, 0, NULL, /* gecos */ homedir, NULL, user_attrs, NULL, timeout, now); break; case RESP_GROUP: - ret = sysdb_store_domgroup(state->dom, attrs->a.group.gr_name, + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.group.gr_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name,\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_store_domgroup(state->dom, name, attrs->a.group.gr_gid, NULL, timeout, now); break; diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 036e88f4c0428dd592b8067d7ec341728ef38ddb..dd9f228abc65821619dc5fc2f3f79f9bb5a22bd6 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -610,6 +610,7 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) struct sss_domain_info *dom = dctx->domain; struct cli_ctx *cctx = cmdctx->cctx; char *name = NULL; + char *src_name = NULL; struct sysdb_ctx *sysdb; struct nss_ctx *nctx; int ret; @@ -635,7 +636,7 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) dctx->domain = dom;
talloc_free(name); - name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive); + name = sss_get_cased_name(cmdctx, cmdctx->name, dom->case_sensitive); if (!name) return ENOMEM;
/* verify this user has not yet been negatively cached, @@ -666,12 +667,26 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getpwnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name int he database */ + if (dom->parent) { + src_name = talloc_asprintf(cmdctx, dom->names->fq_fmt, + name, dom->name); + if (!src_name) return ENOMEM; + } else { + src_name = name; + } + + ret = sysdb_getpwnam(cmdctx, sysdb, src_name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; }
+ if (name != src_name) { + talloc_zfree(src_name); + } + if (dctx->res->count > 1) { DEBUG(0, ("getpwnam call returned more than one result !?!\n")); return ENOENT; @@ -2145,6 +2160,7 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) struct sss_domain_info *dom = dctx->domain; struct cli_ctx *cctx = cmdctx->cctx; char *name = NULL; + char *src_name = NULL; struct sysdb_ctx *sysdb; struct nss_ctx *nctx; int ret; @@ -2201,12 +2217,26 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getgrnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name int he database */ + if (dom->parent) { + src_name = talloc_asprintf(cmdctx, dom->names->fq_fmt, + name, dom->name); + if (!src_name) return ENOMEM; + } else { + src_name = name; + } + + ret = sysdb_getgrnam(cmdctx, sysdb, src_name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; }
+ if (name != src_name) { + talloc_zfree(src_name); + } + if (dctx->res->count > 1) { DEBUG(0, ("getgrnam call returned more than one result !?!\n")); return ENOENT; diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c index 5721d9262f88387e95b926ae4c3fe4d2033d15de..202765a594aab1748b9a071b98e5175c1d3876ae 100644 --- a/src/responder/pac/pacsrv_cmd.c +++ b/src/responder/pac/pacsrv_cmd.c @@ -53,6 +53,7 @@ struct pac_req_ctx { struct pac_ctx *pac_ctx; const char *domain_name; const char *user_name; + char *fq_name; struct sss_domain_info *dom;
struct PAC_LOGON_INFO *logon_info; @@ -201,6 +202,16 @@ static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx) struct dom_sid *my_dom_sid; struct local_mapping_ranges *my_range_map;
+ /* this is a subdomain so we need to search for the fully qualified + * name in the database */ + pr_ctx->fq_name = talloc_asprintf(pr_ctx, pr_ctx->dom->names->fq_fmt, + pr_ctx->user_name, pr_ctx->dom->name); + if (!pr_ctx->fq_name) { + ret = ENOMEM; + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + goto done; + } + ret = save_pac_user(pr_ctx); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("save_pac_user failed.\n")); @@ -365,7 +376,7 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx) goto done; }
- ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->user_name, attrs, + ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->fq_name, attrs, &msg); if (ret == EOK) { /* TODO: check id uid and gid are equal. */ @@ -423,7 +434,7 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx) }
state->gid_iter = 0; - state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->user_name); + state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->fq_name); if (state->user_dn == NULL) { ret = ENOMEM; goto done; diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c index c9551c998332b053833aaa1d08e435bcfec0ec21..53113fb0da18cb235c0c049b14bd836556f647ec 100644 --- a/src/responder/pac/pacsrv_utils.c +++ b/src/responder/pac/pacsrv_utils.c @@ -502,6 +502,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, struct sysdb_attrs *attrs = NULL; struct netr_SamBaseInfo *base_info; int ret; + char *lname; char *uc_realm; char *upn;
@@ -513,36 +514,41 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
base_info = &logon_info->info3.base;
- if (base_info->account_name.size != 0) { - /* To be compatible with winbind based lookups we have to use lower - * case names only, effectively making the domain case-insenvitive. */ - pwd->pw_name = sss_tc_utf8_str_tolower(pwd, - base_info->account_name.string); - if (pwd->pw_name == NULL) { - DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); - ret = ENOMEM; - goto done; - } - } else { + if (base_info->account_name.size == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing account name in PAC.\n")); ret = EINVAL; goto done; } - - if (base_info->rid > 0) { - ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, - base_info->domain_sid, - base_info->rid, &pwd->pw_uid); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); - goto done; - } - } else { + if (base_info->rid == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing user RID in PAC.\n")); ret = EINVAL; goto done; }
+ /* To be compatible with winbind based lookups we have to use lower + * case names only, effectively making the domain case-insenvitive. */ + lname = sss_tc_utf8_str_tolower(pwd, base_info->account_name.string); + if (lname == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); + ret = ENOMEM; + goto done; + } + pwd->pw_name = talloc_asprintf(pwd, dom->names->fq_fmt, + lname, dom->name); + if (!pwd->pw_name) { + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + ret = ENOMEM; + goto done; + } + + ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, + base_info->domain_sid, + base_info->rid, &pwd->pw_uid); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); + goto done; + } + pwd->pw_gid = 0; /* We use MPGs for sub-domains */
if (base_info->full_name.size != 0) { @@ -559,7 +565,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
if (dom->subdomain_homedir) { pwd->pw_dir = expand_homedir_template(pwd, dom->subdomain_homedir, - pwd->pw_name, pwd->pw_uid, + lname, pwd->pw_uid, dom->name); if (pwd->pw_dir == NULL) { ret = ENOMEM; @@ -583,7 +589,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, goto done; }
- upn = talloc_asprintf(mem_ctx, "%s@%s", pwd->pw_name, uc_realm); + upn = talloc_asprintf(mem_ctx, "%s@%s", lname, uc_realm); talloc_free(uc_realm); if (upn == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("talloc_asprintf failed.\n")); diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index bb0d8db384cbbc8a3beed5dc5be0fe32956992a9..2b84d6b97fe6ba6f8869f7dc661aa3e492902ccf 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1163,6 +1163,7 @@ static int pam_check_user_search(struct pam_auth_req *preq) { struct sss_domain_info *dom = preq->domain; char *name = NULL; + char *src_name = NULL; struct sysdb_ctx *sysdb; time_t cacheExpire; int ret; @@ -1222,12 +1223,27 @@ static int pam_check_user_search(struct pam_auth_req *preq) preq->pd->pam_status = PAM_SYSTEM_ERR; return EFAULT; } - ret = sysdb_getpwnam(preq, sysdb, name, &preq->res); + + /* if this is a subdomain we need to search for the fully qualified + * name int he database */ + if (dom->parent) { + src_name = talloc_asprintf(preq, dom->names->fq_fmt, + name, dom->name); + if (!src_name) return ENOMEM; + } else { + src_name = name; + } + + ret = sysdb_getpwnam(preq, sysdb, src_name, &preq->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; }
+ if (name != src_name) { + talloc_zfree(src_name); + } + if (preq->res->count > 1) { DEBUG(0, ("getpwnam call returned more than one result !?!\n")); return ENOENT; diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index 6ee3545520f52171e04bdc096d84fc67dbf82f2e..a6aa5c73327456d2323898af786cb43e0b69d7da 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -73,7 +73,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, }
dom->enumerate = false; - dom->fqnames = true; + dom->fqnames = false; /* FIXME: get ranges from the server */ dom->id_min = 0; dom->id_max = 0xffffffff;
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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
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.
On Mon, 2012-11-12 at 13:30 +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.
YEah I know, in fact I had to add your patches (which I review and pushed) in order to make HBAC work.
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?
No. My long term plan is to revisit this whole subdomain concept and see if we need to change anything. This patchset is a stopgap measure to avoid potential issues (and fix the actual group membership problem, but I am not entirely happy with as a final status.
However we need a working fix for now, and this is the shortest path, any reconsideration of the database layout will require very careful consideration and design decisions that we'll need to discuss a bit.
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 thought about it, but we have 2 issues: 1) We do not have any reserve chars we can safely use to say: 'hey this is special do not return as is in fill_grent()' 2) the point of storing memberuid is to make fill_grent fast in the default case by not having to munge values. If we were to analyze every single entry there fill_grent() would be impacted more than I'd like to do on a short notice.
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:
It is just not performance it is correctness about all sysdb users. I wanted the least intrusive patch that made absolutely certain users were unique. Anything else I think should be deferred to a reconsideration of this whole business during 1.10 life cycle or later.
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.
not just a performance issue, I would take correctness over performance every day given we have the mmap cache anyway.
I am shooting for least intrusive patch at this time.
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.
Ok, I'll wait for more opinions on this specific point.
Simo.
On 11/12/2012 04:02 PM, Simo Sorce wrote:
On Mon, 2012-11-12 at 13:30 +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.
YEah I know, in fact I had to add your patches (which I review and pushed) in order to make HBAC work.
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?
No. My long term plan is to revisit this whole subdomain concept and see if we need to change anything. This patchset is a stopgap measure to avoid potential issues (and fix the actual group membership problem, but I am not entirely happy with as a final status.
However we need a working fix for now, and this is the shortest path, any reconsideration of the database layout will require very careful consideration and design decisions that we'll need to discuss a bit.
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 thought about it, but we have 2 issues:
- We do not have any reserve chars we can safely use to say: 'hey this
is special do not return as is in fill_grent()' 2) the point of storing memberuid is to make fill_grent fast in the default case by not having to munge values. If we were to analyze every single entry there fill_grent() would be impacted more than I'd like to do on a short notice.
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:
It is just not performance it is correctness about all sysdb users. I wanted the least intrusive patch that made absolutely certain users were unique. Anything else I think should be deferred to a reconsideration of this whole business during 1.10 life cycle or later.
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.
not just a performance issue, I would take correctness over performance every day given we have the mmap cache anyway.
I am shooting for least intrusive patch at this time.
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.
Ok, I'll wait for more opinions on this specific point.
I'm OK with it given the circumstances.
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.
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(-)
The original sysdb code had a strong assumption that only users from one domain are saved in the databse, with the subdomain feature, we have changed reality, but have not adjusted all the code arund the sysdb calls to not rely on the original assumption.
One of the side effects of this incongrunece is that currently group memberships do not return fully qualified names for subdomain users as they should.
In oreder to fix this and other potential issues surrounding the violation of the original assumption, we need to fully qualify subdomain user names. By savin them fully qualified we do not risk aliasing local users and have group memberhips or other name based matching code mistake a domain user with subdomain usr or vice versa. --- 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(-)
diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c index 1ab94770003a205485488c879c9deba856183291..528536726bf03508e3eca88605fb7e175e989c7e 100644 --- a/src/db/sysdb_search.c +++ b/src/db/sysdb_search.c @@ -358,6 +358,7 @@ int sysdb_initgroups(TALLOC_CTX *mem_ctx, struct ldb_control **ctrl; struct ldb_asq_control *control; static const char *attrs[] = SYSDB_INITGR_ATTRS; + const char *src_name; int ret;
tmp_ctx = talloc_new(NULL); @@ -365,13 +366,27 @@ int sysdb_initgroups(TALLOC_CTX *mem_ctx, return ENOMEM; }
- ret = sysdb_getpwnam(tmp_ctx, sysdb, name, &res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + if (sysdb->domain->parent) { + src_name = talloc_asprintf(mem_ctx, sysdb->domain->names->fq_fmt, + name, sysdb->domain->name); + if (!src_name) return ENOMEM; + } else { + src_name = name; + } + + ret = sysdb_getpwnam(tmp_ctx, sysdb, src_name, &res); if (ret != EOK) { DEBUG(1, ("sysdb_getpwnam failed: [%d][%s]\n", ret, strerror(ret))); goto done; }
+ if (name != src_name) { + talloc_zfree(src_name); + } + if (res->count == 0) { /* User is not cached yet */ *_res = talloc_steal(mem_ctx, res); diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index 253dc46fb3010b2270ef454bb97e47243a18ceed..c43f32d5186f89d952e7ddf233040abb11c016e9 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -2188,6 +2188,17 @@ int be_process_init(TALLOC_CTX *mem_ctx, goto fail; }
+ /* We need this for subdomains support, as they have to store fully + * qualified user and group names for now */ + ret = sss_names_init(ctx->domain, cdb, + ctx->domain->name, &ctx->domain->names); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + ("fatal error setting fully qualified name format for %s\n", + ctx->domain->name)); + goto fail; + } + ret = be_srv_init(ctx); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, ("fatal error setting up server bus\n")); diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index 1a81c860901dc18a3d443266889701a8980fe031..8fc22819bf72eb194563da1bb3ded2678ac494e6 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -591,6 +591,9 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) uint64_t timeout = 10*60*60; /* FIXME: find a better timeout ! */ const char *homedir = NULL; struct sysdb_attrs *user_attrs = NULL; + char *name; + char *realm; + char *upn;
ret = ipa_s2n_exop_recv(subreq, state, &result, &retoid, &retdata); talloc_zfree(subreq); @@ -640,21 +643,64 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) goto done; }
- ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, - attrs->a.user.pw_name); + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.user.pw_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, name); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); + goto done; + } + + /* We also have to store a fake UPN here, because otherwise the + * krb5 child later won't be able to properly construct one as + * the username is fully qualified but the child doesn't have + * access to the regex to deconstruct it */ + /* FIXME: The real UPN is available from the PAC, we should get + * it from there. */ + realm = get_uppercase_realm(state, state->dom->name); + if (!realm) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to get realm.\n")); + ret = ENOMEM; + goto done; + } + upn = talloc_asprintf(state, "%s@%s", + attrs->a.user.pw_name, realm); + if (!upn) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format UPN.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_UPN, upn); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); goto done; }
- ret = sysdb_store_domuser(state->dom, attrs->a.user.pw_name, NULL, + ret = sysdb_store_domuser(state->dom, name, NULL, attrs->a.user.pw_uid, 0, NULL, /* gecos */ homedir, NULL, user_attrs, NULL, timeout, now); break; case RESP_GROUP: - ret = sysdb_store_domgroup(state->dom, attrs->a.group.gr_name, + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.group.gr_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name,\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_store_domgroup(state->dom, name, attrs->a.group.gr_gid, NULL, timeout, now); break; diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 036e88f4c0428dd592b8067d7ec341728ef38ddb..dd9f228abc65821619dc5fc2f3f79f9bb5a22bd6 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -610,6 +610,7 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) struct sss_domain_info *dom = dctx->domain; struct cli_ctx *cctx = cmdctx->cctx; char *name = NULL; + char *src_name = NULL; struct sysdb_ctx *sysdb; struct nss_ctx *nctx; int ret; @@ -635,7 +636,7 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) dctx->domain = dom;
talloc_free(name); - name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive); + name = sss_get_cased_name(cmdctx, cmdctx->name, dom->case_sensitive); if (!name) return ENOMEM;
/* verify this user has not yet been negatively cached, @@ -666,12 +667,26 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getpwnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name int he database */ + if (dom->parent) { + src_name = talloc_asprintf(cmdctx, dom->names->fq_fmt, + name, dom->name); + if (!src_name) return ENOMEM; + } else { + src_name = name; + } + + ret = sysdb_getpwnam(cmdctx, sysdb, src_name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; }
+ if (name != src_name) { + talloc_zfree(src_name); + } + if (dctx->res->count > 1) { DEBUG(0, ("getpwnam call returned more than one result !?!\n")); return ENOENT; @@ -2145,6 +2160,7 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) struct sss_domain_info *dom = dctx->domain; struct cli_ctx *cctx = cmdctx->cctx; char *name = NULL; + char *src_name = NULL; struct sysdb_ctx *sysdb; struct nss_ctx *nctx; int ret; @@ -2201,12 +2217,26 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getgrnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name int he database */ + if (dom->parent) { + src_name = talloc_asprintf(cmdctx, dom->names->fq_fmt, + name, dom->name); + if (!src_name) return ENOMEM; + } else { + src_name = name; + } + + ret = sysdb_getgrnam(cmdctx, sysdb, src_name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; }
+ if (name != src_name) { + talloc_zfree(src_name); + } + if (dctx->res->count > 1) { DEBUG(0, ("getgrnam call returned more than one result !?!\n")); return ENOENT; diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c index 5721d9262f88387e95b926ae4c3fe4d2033d15de..202765a594aab1748b9a071b98e5175c1d3876ae 100644 --- a/src/responder/pac/pacsrv_cmd.c +++ b/src/responder/pac/pacsrv_cmd.c @@ -53,6 +53,7 @@ struct pac_req_ctx { struct pac_ctx *pac_ctx; const char *domain_name; const char *user_name; + char *fq_name; struct sss_domain_info *dom;
struct PAC_LOGON_INFO *logon_info; @@ -201,6 +202,16 @@ static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx) struct dom_sid *my_dom_sid; struct local_mapping_ranges *my_range_map;
+ /* this is a subdomain so we need to search for the fully qualified + * name in the database */ + pr_ctx->fq_name = talloc_asprintf(pr_ctx, pr_ctx->dom->names->fq_fmt, + pr_ctx->user_name, pr_ctx->dom->name); + if (!pr_ctx->fq_name) { + ret = ENOMEM; + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + goto done; + } + ret = save_pac_user(pr_ctx); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("save_pac_user failed.\n")); @@ -365,7 +376,7 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx) goto done; }
- ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->user_name, attrs, + ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->fq_name, attrs, &msg); if (ret == EOK) { /* TODO: check id uid and gid are equal. */ @@ -423,7 +434,7 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx) }
state->gid_iter = 0; - state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->user_name); + state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->fq_name); if (state->user_dn == NULL) { ret = ENOMEM; goto done; diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c index c9551c998332b053833aaa1d08e435bcfec0ec21..53113fb0da18cb235c0c049b14bd836556f647ec 100644 --- a/src/responder/pac/pacsrv_utils.c +++ b/src/responder/pac/pacsrv_utils.c @@ -502,6 +502,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, struct sysdb_attrs *attrs = NULL; struct netr_SamBaseInfo *base_info; int ret; + char *lname; char *uc_realm; char *upn;
@@ -513,36 +514,41 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
base_info = &logon_info->info3.base;
- if (base_info->account_name.size != 0) { - /* To be compatible with winbind based lookups we have to use lower - * case names only, effectively making the domain case-insenvitive. */ - pwd->pw_name = sss_tc_utf8_str_tolower(pwd, - base_info->account_name.string); - if (pwd->pw_name == NULL) { - DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); - ret = ENOMEM; - goto done; - } - } else { + if (base_info->account_name.size == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing account name in PAC.\n")); ret = EINVAL; goto done; } - - if (base_info->rid > 0) { - ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, - base_info->domain_sid, - base_info->rid, &pwd->pw_uid); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); - goto done; - } - } else { + if (base_info->rid == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing user RID in PAC.\n")); ret = EINVAL; goto done; }
+ /* To be compatible with winbind based lookups we have to use lower + * case names only, effectively making the domain case-insenvitive. */ + lname = sss_tc_utf8_str_tolower(pwd, base_info->account_name.string); + if (lname == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); + ret = ENOMEM; + goto done; + } + pwd->pw_name = talloc_asprintf(pwd, dom->names->fq_fmt, + lname, dom->name); + if (!pwd->pw_name) { + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + ret = ENOMEM; + goto done; + } + + ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, + base_info->domain_sid, + base_info->rid, &pwd->pw_uid); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); + goto done; + } + pwd->pw_gid = 0; /* We use MPGs for sub-domains */
if (base_info->full_name.size != 0) { @@ -559,7 +565,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
if (dom->subdomain_homedir) { pwd->pw_dir = expand_homedir_template(pwd, dom->subdomain_homedir, - pwd->pw_name, pwd->pw_uid, + lname, pwd->pw_uid, dom->name); if (pwd->pw_dir == NULL) { ret = ENOMEM; @@ -583,7 +589,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, goto done; }
- upn = talloc_asprintf(mem_ctx, "%s@%s", pwd->pw_name, uc_realm); + upn = talloc_asprintf(mem_ctx, "%s@%s", lname, uc_realm); talloc_free(uc_realm); if (upn == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("talloc_asprintf failed.\n")); diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 1702a0e91413387e457dc2352cc9d4e693119212..a9e8c3ba10e80becd2487bffcdcb359b6615455d 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1158,6 +1158,7 @@ static int pam_check_user_search(struct pam_auth_req *preq) { struct sss_domain_info *dom = preq->domain; char *name = NULL; + char *src_name = NULL; struct sysdb_ctx *sysdb; time_t cacheExpire; int ret; @@ -1217,12 +1218,27 @@ static int pam_check_user_search(struct pam_auth_req *preq) preq->pd->pam_status = PAM_SYSTEM_ERR; return EFAULT; } - ret = sysdb_getpwnam(preq, sysdb, name, &preq->res); + + /* if this is a subdomain we need to search for the fully qualified + * name int he database */ + if (dom->parent) { + src_name = talloc_asprintf(preq, dom->names->fq_fmt, + name, dom->name); + if (!src_name) return ENOMEM; + } else { + src_name = name; + } + + ret = sysdb_getpwnam(preq, sysdb, src_name, &preq->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; }
+ if (name != src_name) { + talloc_zfree(src_name); + } + if (preq->res->count > 1) { DEBUG(0, ("getpwnam call returned more than one result !?!\n")); return ENOENT; diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index 6ee3545520f52171e04bdc096d84fc67dbf82f2e..a6aa5c73327456d2323898af786cb43e0b69d7da 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -73,7 +73,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, }
dom->enumerate = false; - dom->fqnames = true; + dom->fqnames = false; /* FIXME: get ranges from the server */ dom->id_min = 0; dom->id_max = 0xffffffff;
From: Sumit Bose sbose@redhat.com
Currently the sysdb context is pointed to the subdomain subtree containing user the user to be checked at the beginning of a HBAC request. As a result all HBAC rules and related data is save in the subdomain tree as well. But since the HBAC rules of the configured domain apply to all users it is sufficient to save them once in the subtree of the configured domain.
Since most of the sysdb operations during a HBAC request are related to the HBAC rules and related data this patch does not change the default sysdb context but only create a special context to look up subdomain users. --- src/providers/ipa/ipa_access.c | 10 ---------- src/providers/ipa/ipa_hbac_common.c | 19 ++++++++++++++++--- src/providers/ldap/sdap_access.c | 19 ++++++++++++++++--- 3 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c index 5c97575fca6302e8f1f89a67e14af6a723e1ef03..3a34864c4bb059b837fc38bd2083390c03ae315c 100644 --- a/src/providers/ipa/ipa_access.c +++ b/src/providers/ipa/ipa_access.c @@ -85,16 +85,6 @@ void ipa_access_handler(struct be_req *be_req) be_req->be_ctx->bet_info[BET_ACCESS].pvt_bet_data, struct ipa_access_ctx);
- if (strcasecmp(pd->domain, be_req->be_ctx->domain->name) != 0) { - be_req->domain = new_subdomain(be_req, be_req->be_ctx->domain, pd->domain, NULL, NULL); - if (be_req->domain == NULL) { - DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); - be_req->fn(be_req, DP_ERR_FATAL, PAM_SYSTEM_ERR, NULL); - return; - } - be_req->sysdb = be_req->domain->sysdb; - } - /* First, verify that this account isn't locked. * We need to do this in case the auth phase was * skipped (such as during GSSAPI single-sign-on diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c index 54628d80b6202b61ea3917353688f0705412c83e..33d1944eee682295f0f19c611917684f98df5a92 100644 --- a/src/providers/ipa/ipa_hbac_common.c +++ b/src/providers/ipa/ipa_hbac_common.c @@ -440,6 +440,7 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain = hbac_ctx_be(hbac_ctx)->domain; const char *rhost; const char *thost; + struct sss_domain_info *user_dom;
tmp_ctx = talloc_new(mem_ctx); if (tmp_ctx == NULL) return ENOMEM; @@ -452,9 +453,21 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx,
eval_req->request_time = time(NULL);
- /* Get user the user name and groups */ - ret = hbac_eval_user_element(eval_req, sysdb, - pd->user, &eval_req->user); + /* Get user the user name and groups, + * take care of subdomain users as well */ + if (strcasecmp(pd->domain, domain->name) != 0) { + user_dom = new_subdomain(tmp_ctx, domain, pd->domain, NULL, NULL); + if (user_dom == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); + ret = ENOMEM; + goto done; + } + ret = hbac_eval_user_element(eval_req, user_dom->sysdb, + pd->user, &eval_req->user); + } else { + ret = hbac_eval_user_element(eval_req, sysdb, + pd->user, &eval_req->user); + } if (ret != EOK) goto done;
/* Get the PAM service and service groups */ diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c index 88b52e26b2553eaa64a360b6aee9e19da1fa4326..b198e043580c4d1c3f65c41bc44c6d4749489e4b 100644 --- a/src/providers/ldap/sdap_access.c +++ b/src/providers/ldap/sdap_access.c @@ -139,6 +139,7 @@ sdap_access_send(TALLOC_CTX *mem_ctx, struct tevent_req *req; struct ldb_result *res; const char *attrs[] = { "*", NULL }; + struct sss_domain_info *user_dom;
req = tevent_req_create(mem_ctx, &state, struct sdap_access_req_ctx); if (req == NULL) { @@ -162,9 +163,21 @@ sdap_access_send(TALLOC_CTX *mem_ctx, goto done; }
- /* Get original user DN */ - ret = sysdb_get_user_attr(state, be_req->sysdb, - pd->user, attrs, &res); + /* Get original user DN, take care of subdomain users as well */ + if (strcasecmp(pd->domain, be_req->be_ctx->domain->name) != 0) { + user_dom = new_subdomain(state, be_req->be_ctx->domain, pd->domain, + NULL, NULL); + if (user_dom == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); + ret = ENOMEM; + goto done; + } + ret = sysdb_get_user_attr(state, user_dom->sysdb, + pd->user, attrs, &res); + } else { + ret = sysdb_get_user_attr(state, be_req->sysdb, + pd->user, attrs, &res); + } if (ret != EOK) { if (ret == ENOENT) { /* If we can't find the user, return permission denied */
In subdomains we have to use fully qualified usernames. Unfortunately we have no other good option than simply removing caches for users of subdomains. This is because the memberof plugin does not support the rename operation. --- src/db/sysdb.c | 7 ++++ src/db/sysdb_private.h | 4 ++- src/db/sysdb_upgrade.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 9685163b3f00cc2a45617072efa353589de111ce..e11df05aa8576b3c7118d217d14ecf487d519e40 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1114,6 +1114,13 @@ int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx, } }
+ if (strcmp(version, SYSDB_VERSION_0_13) == 0) { + ret = sysdb_upgrade_13(sysdb, &version); + if (ret != EOK) { + goto done; + } + } + /* The version should now match SYSDB_VERSION. * If not, it means we didn't match any of the * known older versions. The DB might be diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index bde4c603897be496755e773905b0408558376120..a2af8b93fee0f13f80d926b8ef964fd5de206cdb 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -23,6 +23,7 @@ #ifndef __INT_SYS_DB_H__ #define __INT_SYS_DB_H__
+#define SYSDB_VERSION_0_14 "0.14" #define SYSDB_VERSION_0_13 "0.13" #define SYSDB_VERSION_0_12 "0.12" #define SYSDB_VERSION_0_11 "0.11" @@ -37,7 +38,7 @@ #define SYSDB_VERSION_0_2 "0.2" #define SYSDB_VERSION_0_1 "0.1"
-#define SYSDB_VERSION SYSDB_VERSION_0_13 +#define SYSDB_VERSION SYSDB_VERSION_0_14
#define SYSDB_BASE_LDIF \ "dn: @ATTRIBUTES\n" \ @@ -111,6 +112,7 @@ int sysdb_upgrade_09(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_10(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_11(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_12(struct sysdb_ctx *sysdb, const char **ver); +int sysdb_upgrade_13(struct sysdb_ctx *sysdb, const char **ver);
int add_string(struct ldb_message *msg, int flags, const char *attr, const char *value); diff --git a/src/db/sysdb_upgrade.c b/src/db/sysdb_upgrade.c index c4ca64a48339eec90a5f071548496bd02f00646a..10c4e5775515b287dbdb63fcf66f8aeca8515245 100644 --- a/src/db/sysdb_upgrade.c +++ b/src/db/sysdb_upgrade.c @@ -1273,6 +1273,94 @@ done: return ret; }
+int sysdb_upgrade_13(struct sysdb_ctx *sysdb, const char **ver) +{ + struct upgrade_ctx *ctx; + struct ldb_result *dom_res; + struct ldb_result *res; + struct ldb_dn *basedn; + const char *attrs[] = { "cn", "name", NULL }; + const char *tmp_str; + errno_t ret; + int i, j, l, n; + + ret = commence_upgrade(sysdb, sysdb->ldb, SYSDB_VERSION_0_14, &ctx); + if (ret) { + return ret; + } + + basedn = ldb_dn_new(ctx, sysdb->ldb, SYSDB_BASE); + if (!basedn) { + DEBUG(SSSDBG_OP_FAILURE, ("Failed to build base dn\n")); + ret = EIO; + goto done; + } + + ret = ldb_search(sysdb->ldb, ctx, &dom_res, + basedn, LDB_SCOPE_ONELEVEL, + attrs, "objectclass=%s", SYSDB_SUBDOMAIN_CLASS); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, ("Failed to search subdomains\n")); + ret = EIO; + goto done; + } + + for (i = 0; i < dom_res->count; i++) { + + tmp_str = ldb_msg_find_attr_as_string(dom_res->msgs[i], "cn", NULL); + if (tmp_str == NULL) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("The object [%s] doesn't have a name\n", + ldb_dn_get_linearized(res->msgs[i]->dn))); + continue; + } + + basedn = ldb_dn_new_fmt(ctx, sysdb->ldb, SYSDB_DOM_BASE, tmp_str); + if (!basedn) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to build base dn for subdomain %s\n", tmp_str)); + continue; + } + + ret = ldb_search(sysdb->ldb, ctx, &res, + basedn, LDB_SCOPE_SUBTREE, attrs, NULL); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to search subdomain %s\n", tmp_str)); + talloc_free(basedn); + continue; + } + + l = ldb_dn_get_comp_num(basedn); + for (j = 0; j < res->count; j++) { + n = ldb_dn_get_comp_num(res->msgs[j]->dn); + if (n <= l + 1) { + /* Do not remove subdomain containers, only their contents */ + continue; + } + ret = ldb_delete(sysdb->ldb, res->msgs[j]->dn); + if (ret) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to delete %s\n", res->msgs[j]->dn)); + continue; + } + } + + talloc_free(basedn); + talloc_free(res); + } + + talloc_free(dom_res); + + /* conversion done, update version number */ + ret = update_version(ctx); + +done: + ret = finish_upgrade(ret, &ctx, ver); + return ret; +} + + /* * Example template for future upgrades. * Copy and change version numbers as appropriate.
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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Thu, 2012-11-15 at 11:52 +0100, Sumit Bose wrote:
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;
Well, I did not want to add wrapper functions because this whole business is just wrong, the subdomain concept needs to be a first citizen of the basic API not something slapped on top. However the code is ugly enough that a function is better for now, hopefully we'll be able to refactor this sysdb + subdomains business soon.
This would make the main code cleaner and would avoid talloc_zfree with const char issue as well.
Yes.
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?
What I want is to get the UPN from PAC anyway, and save the right one, so I don't think we should spend much time on this.
Simo.
Added wrappers as Sumit requested.
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.h | 9 ++++ src/db/sysdb_private.h | 4 +- src/db/sysdb_search.c | 4 +- src/db/sysdb_subdomains.c | 41 ++++++++++++++++ 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 | 10 +++- src/responder/pac/pacsrv_cmd.c | 15 +++++- src/responder/pac/pacsrv_utils.c | 52 +++++++++++--------- src/responder/pam/pamsrv_cmd.c | 5 ++- src/util/domain_info_utils.c | 2 +- 16 files changed, 298 insertions(+), 52 deletions(-)
The original sysdb code had a strong assumption that only users from one domain are saved in the databse, with the subdomain feature, we have changed reality, but have not adjusted all the code arund the sysdb calls to not rely on the original assumption.
One of the side effects of this incongrunece is that currently group memberships do not return fully qualified names for subdomain users as they should.
In oreder to fix this and other potential issues surrounding the violation of the original assumption, we need to fully qualify subdomain user names. By savin them fully qualified we do not risk aliasing local users and have group memberhips or other name based matching code mistake a domain user with subdomain usr or vice versa. --- src/db/sysdb.h | 9 ++++++ src/db/sysdb_search.c | 4 ++- src/db/sysdb_subdomains.c | 41 ++++++++++++++++++++++++++++ src/providers/data_provider_be.c | 11 +++++++ src/providers/ipa/ipa_s2n_exop.c | 54 +++++++++++++++++++++++++++++++++++--- src/responder/nss/nsssrv_cmd.c | 10 +++++-- src/responder/pac/pacsrv_cmd.c | 15 +++++++++- src/responder/pac/pacsrv_utils.c | 52 ++++++++++++++++++++---------------- src/responder/pam/pamsrv_cmd.c | 5 +++- src/util/domain_info_utils.c | 2 +- 10 files changed, 168 insertions(+), 35 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 39e0f9f5245c67f3798055823e98c2db0fbdebba..7bbb5ba59dd89406b21184914167708489996714 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -422,6 +422,15 @@ errno_t sysdb_store_domgroup(struct sss_domain_info *domain, errno_t sysdb_delete_domgroup(struct sss_domain_info *domain, const char *name, gid_t gid);
+int sysdb_subdom_getpwnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **_res); +int sysdb_subdom_getgrnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **_res); + errno_t sysdb_get_ranges(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb, size_t *range_count, struct range_info ***range_list); diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c index 1ab94770003a205485488c879c9deba856183291..49f628bfd3d15aba77b169b163c6ba15a7813744 100644 --- a/src/db/sysdb_search.c +++ b/src/db/sysdb_search.c @@ -365,7 +365,9 @@ int sysdb_initgroups(TALLOC_CTX *mem_ctx, return ENOMEM; }
- ret = sysdb_getpwnam(tmp_ctx, sysdb, name, &res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getpwnam(tmp_ctx, sysdb, name, &res); if (ret != EOK) { DEBUG(1, ("sysdb_getpwnam failed: [%d][%s]\n", ret, strerror(ret))); diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index 2e0170f4d37f04468e9a56dbca9b8176c017c9ff..2efa05728e5bf92c4df325aff7783334856e5696 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -668,3 +668,44 @@ errno_t sysdb_delete_domgroup(struct sss_domain_info *domain,
return sysdb_delete_group(domain->sysdb, name, gid); } + +int sysdb_subdom_getpwnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **res) +{ + const char *src_name = NULL; + int ret; + + 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(mem_ctx, sysdb, src_name ? src_name : name, &res); + talloc_zfree(src_name); + + return ret; +} + +int sysdb_subdom_getgrnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **res) +{ + const char *src_name = NULL; + int ret; + + 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_getgrnam(mem_ctx, sysdb, src_name ? src_name : name, &res); + talloc_zfree(src_name); + + return ret; +} + diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index 253dc46fb3010b2270ef454bb97e47243a18ceed..c43f32d5186f89d952e7ddf233040abb11c016e9 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -2188,6 +2188,17 @@ int be_process_init(TALLOC_CTX *mem_ctx, goto fail; }
+ /* We need this for subdomains support, as they have to store fully + * qualified user and group names for now */ + ret = sss_names_init(ctx->domain, cdb, + ctx->domain->name, &ctx->domain->names); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + ("fatal error setting fully qualified name format for %s\n", + ctx->domain->name)); + goto fail; + } + ret = be_srv_init(ctx); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, ("fatal error setting up server bus\n")); diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index 1a81c860901dc18a3d443266889701a8980fe031..8fc22819bf72eb194563da1bb3ded2678ac494e6 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -591,6 +591,9 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) uint64_t timeout = 10*60*60; /* FIXME: find a better timeout ! */ const char *homedir = NULL; struct sysdb_attrs *user_attrs = NULL; + char *name; + char *realm; + char *upn;
ret = ipa_s2n_exop_recv(subreq, state, &result, &retoid, &retdata); talloc_zfree(subreq); @@ -640,21 +643,64 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) goto done; }
- ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, - attrs->a.user.pw_name); + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.user.pw_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, name); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); + goto done; + } + + /* We also have to store a fake UPN here, because otherwise the + * krb5 child later won't be able to properly construct one as + * the username is fully qualified but the child doesn't have + * access to the regex to deconstruct it */ + /* FIXME: The real UPN is available from the PAC, we should get + * it from there. */ + realm = get_uppercase_realm(state, state->dom->name); + if (!realm) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to get realm.\n")); + ret = ENOMEM; + goto done; + } + upn = talloc_asprintf(state, "%s@%s", + attrs->a.user.pw_name, realm); + if (!upn) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format UPN.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_UPN, upn); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); goto done; }
- ret = sysdb_store_domuser(state->dom, attrs->a.user.pw_name, NULL, + ret = sysdb_store_domuser(state->dom, name, NULL, attrs->a.user.pw_uid, 0, NULL, /* gecos */ homedir, NULL, user_attrs, NULL, timeout, now); break; case RESP_GROUP: - ret = sysdb_store_domgroup(state->dom, attrs->a.group.gr_name, + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.group.gr_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name,\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_store_domgroup(state->dom, name, attrs->a.group.gr_gid, NULL, timeout, now); break; diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 036e88f4c0428dd592b8067d7ec341728ef38ddb..c667a3f90f849e8b9af3806d91157a7ff37117a8 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -635,7 +635,7 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) dctx->domain = dom;
talloc_free(name); - name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive); + name = sss_get_cased_name(cmdctx, cmdctx->name, dom->case_sensitive); if (!name) return ENOMEM;
/* verify this user has not yet been negatively cached, @@ -666,7 +666,9 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getpwnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getpwnam(cmdctx, sysdb, name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; @@ -2201,7 +2203,9 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getgrnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getgrnam(cmdctx, sysdb, name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c index 5721d9262f88387e95b926ae4c3fe4d2033d15de..202765a594aab1748b9a071b98e5175c1d3876ae 100644 --- a/src/responder/pac/pacsrv_cmd.c +++ b/src/responder/pac/pacsrv_cmd.c @@ -53,6 +53,7 @@ struct pac_req_ctx { struct pac_ctx *pac_ctx; const char *domain_name; const char *user_name; + char *fq_name; struct sss_domain_info *dom;
struct PAC_LOGON_INFO *logon_info; @@ -201,6 +202,16 @@ static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx) struct dom_sid *my_dom_sid; struct local_mapping_ranges *my_range_map;
+ /* this is a subdomain so we need to search for the fully qualified + * name in the database */ + pr_ctx->fq_name = talloc_asprintf(pr_ctx, pr_ctx->dom->names->fq_fmt, + pr_ctx->user_name, pr_ctx->dom->name); + if (!pr_ctx->fq_name) { + ret = ENOMEM; + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + goto done; + } + ret = save_pac_user(pr_ctx); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("save_pac_user failed.\n")); @@ -365,7 +376,7 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx) goto done; }
- ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->user_name, attrs, + ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->fq_name, attrs, &msg); if (ret == EOK) { /* TODO: check id uid and gid are equal. */ @@ -423,7 +434,7 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx) }
state->gid_iter = 0; - state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->user_name); + state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->fq_name); if (state->user_dn == NULL) { ret = ENOMEM; goto done; diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c index c9551c998332b053833aaa1d08e435bcfec0ec21..53113fb0da18cb235c0c049b14bd836556f647ec 100644 --- a/src/responder/pac/pacsrv_utils.c +++ b/src/responder/pac/pacsrv_utils.c @@ -502,6 +502,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, struct sysdb_attrs *attrs = NULL; struct netr_SamBaseInfo *base_info; int ret; + char *lname; char *uc_realm; char *upn;
@@ -513,36 +514,41 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
base_info = &logon_info->info3.base;
- if (base_info->account_name.size != 0) { - /* To be compatible with winbind based lookups we have to use lower - * case names only, effectively making the domain case-insenvitive. */ - pwd->pw_name = sss_tc_utf8_str_tolower(pwd, - base_info->account_name.string); - if (pwd->pw_name == NULL) { - DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); - ret = ENOMEM; - goto done; - } - } else { + if (base_info->account_name.size == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing account name in PAC.\n")); ret = EINVAL; goto done; } - - if (base_info->rid > 0) { - ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, - base_info->domain_sid, - base_info->rid, &pwd->pw_uid); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); - goto done; - } - } else { + if (base_info->rid == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing user RID in PAC.\n")); ret = EINVAL; goto done; }
+ /* To be compatible with winbind based lookups we have to use lower + * case names only, effectively making the domain case-insenvitive. */ + lname = sss_tc_utf8_str_tolower(pwd, base_info->account_name.string); + if (lname == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); + ret = ENOMEM; + goto done; + } + pwd->pw_name = talloc_asprintf(pwd, dom->names->fq_fmt, + lname, dom->name); + if (!pwd->pw_name) { + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + ret = ENOMEM; + goto done; + } + + ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, + base_info->domain_sid, + base_info->rid, &pwd->pw_uid); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); + goto done; + } + pwd->pw_gid = 0; /* We use MPGs for sub-domains */
if (base_info->full_name.size != 0) { @@ -559,7 +565,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
if (dom->subdomain_homedir) { pwd->pw_dir = expand_homedir_template(pwd, dom->subdomain_homedir, - pwd->pw_name, pwd->pw_uid, + lname, pwd->pw_uid, dom->name); if (pwd->pw_dir == NULL) { ret = ENOMEM; @@ -583,7 +589,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, goto done; }
- upn = talloc_asprintf(mem_ctx, "%s@%s", pwd->pw_name, uc_realm); + upn = talloc_asprintf(mem_ctx, "%s@%s", lname, uc_realm); talloc_free(uc_realm); if (upn == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("talloc_asprintf failed.\n")); diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 1702a0e91413387e457dc2352cc9d4e693119212..4269642206cc0295c0046de4e59a3ad8f1044d1a 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1217,7 +1217,10 @@ static int pam_check_user_search(struct pam_auth_req *preq) preq->pd->pam_status = PAM_SYSTEM_ERR; return EFAULT; } - ret = sysdb_getpwnam(preq, sysdb, name, &preq->res); + + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getpwnam(preq, sysdb, name, &preq->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index 6ee3545520f52171e04bdc096d84fc67dbf82f2e..a6aa5c73327456d2323898af786cb43e0b69d7da 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -73,7 +73,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, }
dom->enumerate = false; - dom->fqnames = true; + dom->fqnames = false; /* FIXME: get ranges from the server */ dom->id_min = 0; dom->id_max = 0xffffffff;
From: Sumit Bose sbose@redhat.com
Currently the sysdb context is pointed to the subdomain subtree containing user the user to be checked at the beginning of a HBAC request. As a result all HBAC rules and related data is save in the subdomain tree as well. But since the HBAC rules of the configured domain apply to all users it is sufficient to save them once in the subtree of the configured domain.
Since most of the sysdb operations during a HBAC request are related to the HBAC rules and related data this patch does not change the default sysdb context but only create a special context to look up subdomain users. --- src/providers/ipa/ipa_access.c | 10 ---------- src/providers/ipa/ipa_hbac_common.c | 19 ++++++++++++++++--- src/providers/ldap/sdap_access.c | 19 ++++++++++++++++--- 3 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c index 5c97575fca6302e8f1f89a67e14af6a723e1ef03..3a34864c4bb059b837fc38bd2083390c03ae315c 100644 --- a/src/providers/ipa/ipa_access.c +++ b/src/providers/ipa/ipa_access.c @@ -85,16 +85,6 @@ void ipa_access_handler(struct be_req *be_req) be_req->be_ctx->bet_info[BET_ACCESS].pvt_bet_data, struct ipa_access_ctx);
- if (strcasecmp(pd->domain, be_req->be_ctx->domain->name) != 0) { - be_req->domain = new_subdomain(be_req, be_req->be_ctx->domain, pd->domain, NULL, NULL); - if (be_req->domain == NULL) { - DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); - be_req->fn(be_req, DP_ERR_FATAL, PAM_SYSTEM_ERR, NULL); - return; - } - be_req->sysdb = be_req->domain->sysdb; - } - /* First, verify that this account isn't locked. * We need to do this in case the auth phase was * skipped (such as during GSSAPI single-sign-on diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c index 54628d80b6202b61ea3917353688f0705412c83e..33d1944eee682295f0f19c611917684f98df5a92 100644 --- a/src/providers/ipa/ipa_hbac_common.c +++ b/src/providers/ipa/ipa_hbac_common.c @@ -440,6 +440,7 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain = hbac_ctx_be(hbac_ctx)->domain; const char *rhost; const char *thost; + struct sss_domain_info *user_dom;
tmp_ctx = talloc_new(mem_ctx); if (tmp_ctx == NULL) return ENOMEM; @@ -452,9 +453,21 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx,
eval_req->request_time = time(NULL);
- /* Get user the user name and groups */ - ret = hbac_eval_user_element(eval_req, sysdb, - pd->user, &eval_req->user); + /* Get user the user name and groups, + * take care of subdomain users as well */ + if (strcasecmp(pd->domain, domain->name) != 0) { + user_dom = new_subdomain(tmp_ctx, domain, pd->domain, NULL, NULL); + if (user_dom == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); + ret = ENOMEM; + goto done; + } + ret = hbac_eval_user_element(eval_req, user_dom->sysdb, + pd->user, &eval_req->user); + } else { + ret = hbac_eval_user_element(eval_req, sysdb, + pd->user, &eval_req->user); + } if (ret != EOK) goto done;
/* Get the PAM service and service groups */ diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c index 88b52e26b2553eaa64a360b6aee9e19da1fa4326..b198e043580c4d1c3f65c41bc44c6d4749489e4b 100644 --- a/src/providers/ldap/sdap_access.c +++ b/src/providers/ldap/sdap_access.c @@ -139,6 +139,7 @@ sdap_access_send(TALLOC_CTX *mem_ctx, struct tevent_req *req; struct ldb_result *res; const char *attrs[] = { "*", NULL }; + struct sss_domain_info *user_dom;
req = tevent_req_create(mem_ctx, &state, struct sdap_access_req_ctx); if (req == NULL) { @@ -162,9 +163,21 @@ sdap_access_send(TALLOC_CTX *mem_ctx, goto done; }
- /* Get original user DN */ - ret = sysdb_get_user_attr(state, be_req->sysdb, - pd->user, attrs, &res); + /* Get original user DN, take care of subdomain users as well */ + if (strcasecmp(pd->domain, be_req->be_ctx->domain->name) != 0) { + user_dom = new_subdomain(state, be_req->be_ctx->domain, pd->domain, + NULL, NULL); + if (user_dom == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); + ret = ENOMEM; + goto done; + } + ret = sysdb_get_user_attr(state, user_dom->sysdb, + pd->user, attrs, &res); + } else { + ret = sysdb_get_user_attr(state, be_req->sysdb, + pd->user, attrs, &res); + } if (ret != EOK) { if (ret == ENOENT) { /* If we can't find the user, return permission denied */
In subdomains we have to use fully qualified usernames. Unfortunately we have no other good option than simply removing caches for users of subdomains. This is because the memberof plugin does not support the rename operation. --- src/db/sysdb.c | 7 ++++ src/db/sysdb_private.h | 4 ++- src/db/sysdb_upgrade.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 9685163b3f00cc2a45617072efa353589de111ce..e11df05aa8576b3c7118d217d14ecf487d519e40 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1114,6 +1114,13 @@ int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx, } }
+ if (strcmp(version, SYSDB_VERSION_0_13) == 0) { + ret = sysdb_upgrade_13(sysdb, &version); + if (ret != EOK) { + goto done; + } + } + /* The version should now match SYSDB_VERSION. * If not, it means we didn't match any of the * known older versions. The DB might be diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index bde4c603897be496755e773905b0408558376120..a2af8b93fee0f13f80d926b8ef964fd5de206cdb 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -23,6 +23,7 @@ #ifndef __INT_SYS_DB_H__ #define __INT_SYS_DB_H__
+#define SYSDB_VERSION_0_14 "0.14" #define SYSDB_VERSION_0_13 "0.13" #define SYSDB_VERSION_0_12 "0.12" #define SYSDB_VERSION_0_11 "0.11" @@ -37,7 +38,7 @@ #define SYSDB_VERSION_0_2 "0.2" #define SYSDB_VERSION_0_1 "0.1"
-#define SYSDB_VERSION SYSDB_VERSION_0_13 +#define SYSDB_VERSION SYSDB_VERSION_0_14
#define SYSDB_BASE_LDIF \ "dn: @ATTRIBUTES\n" \ @@ -111,6 +112,7 @@ int sysdb_upgrade_09(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_10(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_11(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_12(struct sysdb_ctx *sysdb, const char **ver); +int sysdb_upgrade_13(struct sysdb_ctx *sysdb, const char **ver);
int add_string(struct ldb_message *msg, int flags, const char *attr, const char *value); diff --git a/src/db/sysdb_upgrade.c b/src/db/sysdb_upgrade.c index c4ca64a48339eec90a5f071548496bd02f00646a..10c4e5775515b287dbdb63fcf66f8aeca8515245 100644 --- a/src/db/sysdb_upgrade.c +++ b/src/db/sysdb_upgrade.c @@ -1273,6 +1273,94 @@ done: return ret; }
+int sysdb_upgrade_13(struct sysdb_ctx *sysdb, const char **ver) +{ + struct upgrade_ctx *ctx; + struct ldb_result *dom_res; + struct ldb_result *res; + struct ldb_dn *basedn; + const char *attrs[] = { "cn", "name", NULL }; + const char *tmp_str; + errno_t ret; + int i, j, l, n; + + ret = commence_upgrade(sysdb, sysdb->ldb, SYSDB_VERSION_0_14, &ctx); + if (ret) { + return ret; + } + + basedn = ldb_dn_new(ctx, sysdb->ldb, SYSDB_BASE); + if (!basedn) { + DEBUG(SSSDBG_OP_FAILURE, ("Failed to build base dn\n")); + ret = EIO; + goto done; + } + + ret = ldb_search(sysdb->ldb, ctx, &dom_res, + basedn, LDB_SCOPE_ONELEVEL, + attrs, "objectclass=%s", SYSDB_SUBDOMAIN_CLASS); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, ("Failed to search subdomains\n")); + ret = EIO; + goto done; + } + + for (i = 0; i < dom_res->count; i++) { + + tmp_str = ldb_msg_find_attr_as_string(dom_res->msgs[i], "cn", NULL); + if (tmp_str == NULL) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("The object [%s] doesn't have a name\n", + ldb_dn_get_linearized(res->msgs[i]->dn))); + continue; + } + + basedn = ldb_dn_new_fmt(ctx, sysdb->ldb, SYSDB_DOM_BASE, tmp_str); + if (!basedn) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to build base dn for subdomain %s\n", tmp_str)); + continue; + } + + ret = ldb_search(sysdb->ldb, ctx, &res, + basedn, LDB_SCOPE_SUBTREE, attrs, NULL); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to search subdomain %s\n", tmp_str)); + talloc_free(basedn); + continue; + } + + l = ldb_dn_get_comp_num(basedn); + for (j = 0; j < res->count; j++) { + n = ldb_dn_get_comp_num(res->msgs[j]->dn); + if (n <= l + 1) { + /* Do not remove subdomain containers, only their contents */ + continue; + } + ret = ldb_delete(sysdb->ldb, res->msgs[j]->dn); + if (ret) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to delete %s\n", res->msgs[j]->dn)); + continue; + } + } + + talloc_free(basedn); + talloc_free(res); + } + + talloc_free(dom_res); + + /* conversion done, update version number */ + ret = update_version(ctx); + +done: + ret = finish_upgrade(ret, &ctx, ver); + return ret; +} + + /* * Example template for future upgrades. * Copy and change version numbers as appropriate.
On Fri, 2012-11-16 at 13:34 -0500, Simo Sorce wrote:
Added wrappers as Sumit requested.
Please ignore this set, I introduced a crash bug in refactoring the getXXnam() calls as Sumit asked. I'll send a new patchset soon.
Simo.
Resolved segfault, everything else as before.
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.h | 9 ++++ src/db/sysdb_private.h | 4 +- src/db/sysdb_search.c | 4 +- src/db/sysdb_subdomains.c | 41 ++++++++++++++++ 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 | 10 +++- src/responder/pac/pacsrv_cmd.c | 15 +++++- src/responder/pac/pacsrv_utils.c | 52 +++++++++++--------- src/responder/pam/pamsrv_cmd.c | 5 ++- src/util/domain_info_utils.c | 2 +- 16 files changed, 298 insertions(+), 52 deletions(-)
The original sysdb code had a strong assumption that only users from one domain are saved in the databse, with the subdomain feature, we have changed reality, but have not adjusted all the code arund the sysdb calls to not rely on the original assumption.
One of the side effects of this incongrunece is that currently group memberships do not return fully qualified names for subdomain users as they should.
In oreder to fix this and other potential issues surrounding the violation of the original assumption, we need to fully qualify subdomain user names. By savin them fully qualified we do not risk aliasing local users and have group memberhips or other name based matching code mistake a domain user with subdomain usr or vice versa. --- src/db/sysdb.h | 9 ++++++ src/db/sysdb_search.c | 4 ++- src/db/sysdb_subdomains.c | 41 ++++++++++++++++++++++++++++ src/providers/data_provider_be.c | 11 +++++++ src/providers/ipa/ipa_s2n_exop.c | 54 +++++++++++++++++++++++++++++++++++--- src/responder/nss/nsssrv_cmd.c | 10 +++++-- src/responder/pac/pacsrv_cmd.c | 15 +++++++++- src/responder/pac/pacsrv_utils.c | 52 ++++++++++++++++++++---------------- src/responder/pam/pamsrv_cmd.c | 5 +++- src/util/domain_info_utils.c | 2 +- 10 files changed, 168 insertions(+), 35 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 39e0f9f5245c67f3798055823e98c2db0fbdebba..7bbb5ba59dd89406b21184914167708489996714 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -422,6 +422,15 @@ errno_t sysdb_store_domgroup(struct sss_domain_info *domain, errno_t sysdb_delete_domgroup(struct sss_domain_info *domain, const char *name, gid_t gid);
+int sysdb_subdom_getpwnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **_res); +int sysdb_subdom_getgrnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **_res); + errno_t sysdb_get_ranges(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb, size_t *range_count, struct range_info ***range_list); diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c index 1ab94770003a205485488c879c9deba856183291..49f628bfd3d15aba77b169b163c6ba15a7813744 100644 --- a/src/db/sysdb_search.c +++ b/src/db/sysdb_search.c @@ -365,7 +365,9 @@ int sysdb_initgroups(TALLOC_CTX *mem_ctx, return ENOMEM; }
- ret = sysdb_getpwnam(tmp_ctx, sysdb, name, &res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getpwnam(tmp_ctx, sysdb, name, &res); if (ret != EOK) { DEBUG(1, ("sysdb_getpwnam failed: [%d][%s]\n", ret, strerror(ret))); diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index 2e0170f4d37f04468e9a56dbca9b8176c017c9ff..9431792075af985c190d6c46189a57ef0e0c4b4a 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -668,3 +668,44 @@ errno_t sysdb_delete_domgroup(struct sss_domain_info *domain,
return sysdb_delete_group(domain->sysdb, name, gid); } + +int sysdb_subdom_getpwnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **res) +{ + const char *src_name = NULL; + int ret; + + 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(mem_ctx, sysdb, src_name ? src_name : name, res); + talloc_zfree(src_name); + + return ret; +} + +int sysdb_subdom_getgrnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **res) +{ + const char *src_name = NULL; + int ret; + + 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_getgrnam(mem_ctx, sysdb, src_name ? src_name : name, res); + talloc_zfree(src_name); + + return ret; +} + diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index 253dc46fb3010b2270ef454bb97e47243a18ceed..c43f32d5186f89d952e7ddf233040abb11c016e9 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -2188,6 +2188,17 @@ int be_process_init(TALLOC_CTX *mem_ctx, goto fail; }
+ /* We need this for subdomains support, as they have to store fully + * qualified user and group names for now */ + ret = sss_names_init(ctx->domain, cdb, + ctx->domain->name, &ctx->domain->names); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + ("fatal error setting fully qualified name format for %s\n", + ctx->domain->name)); + goto fail; + } + ret = be_srv_init(ctx); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, ("fatal error setting up server bus\n")); diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index 1a81c860901dc18a3d443266889701a8980fe031..8fc22819bf72eb194563da1bb3ded2678ac494e6 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -591,6 +591,9 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) uint64_t timeout = 10*60*60; /* FIXME: find a better timeout ! */ const char *homedir = NULL; struct sysdb_attrs *user_attrs = NULL; + char *name; + char *realm; + char *upn;
ret = ipa_s2n_exop_recv(subreq, state, &result, &retoid, &retdata); talloc_zfree(subreq); @@ -640,21 +643,64 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) goto done; }
- ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, - attrs->a.user.pw_name); + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.user.pw_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, name); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); + goto done; + } + + /* We also have to store a fake UPN here, because otherwise the + * krb5 child later won't be able to properly construct one as + * the username is fully qualified but the child doesn't have + * access to the regex to deconstruct it */ + /* FIXME: The real UPN is available from the PAC, we should get + * it from there. */ + realm = get_uppercase_realm(state, state->dom->name); + if (!realm) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to get realm.\n")); + ret = ENOMEM; + goto done; + } + upn = talloc_asprintf(state, "%s@%s", + attrs->a.user.pw_name, realm); + if (!upn) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format UPN.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_UPN, upn); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); goto done; }
- ret = sysdb_store_domuser(state->dom, attrs->a.user.pw_name, NULL, + ret = sysdb_store_domuser(state->dom, name, NULL, attrs->a.user.pw_uid, 0, NULL, /* gecos */ homedir, NULL, user_attrs, NULL, timeout, now); break; case RESP_GROUP: - ret = sysdb_store_domgroup(state->dom, attrs->a.group.gr_name, + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.group.gr_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name,\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_store_domgroup(state->dom, name, attrs->a.group.gr_gid, NULL, timeout, now); break; diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 036e88f4c0428dd592b8067d7ec341728ef38ddb..c667a3f90f849e8b9af3806d91157a7ff37117a8 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -635,7 +635,7 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) dctx->domain = dom;
talloc_free(name); - name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive); + name = sss_get_cased_name(cmdctx, cmdctx->name, dom->case_sensitive); if (!name) return ENOMEM;
/* verify this user has not yet been negatively cached, @@ -666,7 +666,9 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getpwnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getpwnam(cmdctx, sysdb, name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; @@ -2201,7 +2203,9 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getgrnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getgrnam(cmdctx, sysdb, name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c index 5721d9262f88387e95b926ae4c3fe4d2033d15de..202765a594aab1748b9a071b98e5175c1d3876ae 100644 --- a/src/responder/pac/pacsrv_cmd.c +++ b/src/responder/pac/pacsrv_cmd.c @@ -53,6 +53,7 @@ struct pac_req_ctx { struct pac_ctx *pac_ctx; const char *domain_name; const char *user_name; + char *fq_name; struct sss_domain_info *dom;
struct PAC_LOGON_INFO *logon_info; @@ -201,6 +202,16 @@ static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx) struct dom_sid *my_dom_sid; struct local_mapping_ranges *my_range_map;
+ /* this is a subdomain so we need to search for the fully qualified + * name in the database */ + pr_ctx->fq_name = talloc_asprintf(pr_ctx, pr_ctx->dom->names->fq_fmt, + pr_ctx->user_name, pr_ctx->dom->name); + if (!pr_ctx->fq_name) { + ret = ENOMEM; + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + goto done; + } + ret = save_pac_user(pr_ctx); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("save_pac_user failed.\n")); @@ -365,7 +376,7 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx) goto done; }
- ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->user_name, attrs, + ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->fq_name, attrs, &msg); if (ret == EOK) { /* TODO: check id uid and gid are equal. */ @@ -423,7 +434,7 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx) }
state->gid_iter = 0; - state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->user_name); + state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->fq_name); if (state->user_dn == NULL) { ret = ENOMEM; goto done; diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c index c9551c998332b053833aaa1d08e435bcfec0ec21..53113fb0da18cb235c0c049b14bd836556f647ec 100644 --- a/src/responder/pac/pacsrv_utils.c +++ b/src/responder/pac/pacsrv_utils.c @@ -502,6 +502,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, struct sysdb_attrs *attrs = NULL; struct netr_SamBaseInfo *base_info; int ret; + char *lname; char *uc_realm; char *upn;
@@ -513,36 +514,41 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
base_info = &logon_info->info3.base;
- if (base_info->account_name.size != 0) { - /* To be compatible with winbind based lookups we have to use lower - * case names only, effectively making the domain case-insenvitive. */ - pwd->pw_name = sss_tc_utf8_str_tolower(pwd, - base_info->account_name.string); - if (pwd->pw_name == NULL) { - DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); - ret = ENOMEM; - goto done; - } - } else { + if (base_info->account_name.size == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing account name in PAC.\n")); ret = EINVAL; goto done; } - - if (base_info->rid > 0) { - ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, - base_info->domain_sid, - base_info->rid, &pwd->pw_uid); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); - goto done; - } - } else { + if (base_info->rid == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing user RID in PAC.\n")); ret = EINVAL; goto done; }
+ /* To be compatible with winbind based lookups we have to use lower + * case names only, effectively making the domain case-insenvitive. */ + lname = sss_tc_utf8_str_tolower(pwd, base_info->account_name.string); + if (lname == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); + ret = ENOMEM; + goto done; + } + pwd->pw_name = talloc_asprintf(pwd, dom->names->fq_fmt, + lname, dom->name); + if (!pwd->pw_name) { + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + ret = ENOMEM; + goto done; + } + + ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, + base_info->domain_sid, + base_info->rid, &pwd->pw_uid); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); + goto done; + } + pwd->pw_gid = 0; /* We use MPGs for sub-domains */
if (base_info->full_name.size != 0) { @@ -559,7 +565,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
if (dom->subdomain_homedir) { pwd->pw_dir = expand_homedir_template(pwd, dom->subdomain_homedir, - pwd->pw_name, pwd->pw_uid, + lname, pwd->pw_uid, dom->name); if (pwd->pw_dir == NULL) { ret = ENOMEM; @@ -583,7 +589,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, goto done; }
- upn = talloc_asprintf(mem_ctx, "%s@%s", pwd->pw_name, uc_realm); + upn = talloc_asprintf(mem_ctx, "%s@%s", lname, uc_realm); talloc_free(uc_realm); if (upn == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("talloc_asprintf failed.\n")); diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 1702a0e91413387e457dc2352cc9d4e693119212..4269642206cc0295c0046de4e59a3ad8f1044d1a 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1217,7 +1217,10 @@ static int pam_check_user_search(struct pam_auth_req *preq) preq->pd->pam_status = PAM_SYSTEM_ERR; return EFAULT; } - ret = sysdb_getpwnam(preq, sysdb, name, &preq->res); + + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getpwnam(preq, sysdb, name, &preq->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index 6ee3545520f52171e04bdc096d84fc67dbf82f2e..a6aa5c73327456d2323898af786cb43e0b69d7da 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -73,7 +73,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, }
dom->enumerate = false; - dom->fqnames = true; + dom->fqnames = false; /* FIXME: get ranges from the server */ dom->id_min = 0; dom->id_max = 0xffffffff;
From: Sumit Bose sbose@redhat.com
Currently the sysdb context is pointed to the subdomain subtree containing user the user to be checked at the beginning of a HBAC request. As a result all HBAC rules and related data is save in the subdomain tree as well. But since the HBAC rules of the configured domain apply to all users it is sufficient to save them once in the subtree of the configured domain.
Since most of the sysdb operations during a HBAC request are related to the HBAC rules and related data this patch does not change the default sysdb context but only create a special context to look up subdomain users. --- src/providers/ipa/ipa_access.c | 10 ---------- src/providers/ipa/ipa_hbac_common.c | 19 ++++++++++++++++--- src/providers/ldap/sdap_access.c | 19 ++++++++++++++++--- 3 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c index 5c97575fca6302e8f1f89a67e14af6a723e1ef03..3a34864c4bb059b837fc38bd2083390c03ae315c 100644 --- a/src/providers/ipa/ipa_access.c +++ b/src/providers/ipa/ipa_access.c @@ -85,16 +85,6 @@ void ipa_access_handler(struct be_req *be_req) be_req->be_ctx->bet_info[BET_ACCESS].pvt_bet_data, struct ipa_access_ctx);
- if (strcasecmp(pd->domain, be_req->be_ctx->domain->name) != 0) { - be_req->domain = new_subdomain(be_req, be_req->be_ctx->domain, pd->domain, NULL, NULL); - if (be_req->domain == NULL) { - DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); - be_req->fn(be_req, DP_ERR_FATAL, PAM_SYSTEM_ERR, NULL); - return; - } - be_req->sysdb = be_req->domain->sysdb; - } - /* First, verify that this account isn't locked. * We need to do this in case the auth phase was * skipped (such as during GSSAPI single-sign-on diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c index 54628d80b6202b61ea3917353688f0705412c83e..33d1944eee682295f0f19c611917684f98df5a92 100644 --- a/src/providers/ipa/ipa_hbac_common.c +++ b/src/providers/ipa/ipa_hbac_common.c @@ -440,6 +440,7 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain = hbac_ctx_be(hbac_ctx)->domain; const char *rhost; const char *thost; + struct sss_domain_info *user_dom;
tmp_ctx = talloc_new(mem_ctx); if (tmp_ctx == NULL) return ENOMEM; @@ -452,9 +453,21 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx,
eval_req->request_time = time(NULL);
- /* Get user the user name and groups */ - ret = hbac_eval_user_element(eval_req, sysdb, - pd->user, &eval_req->user); + /* Get user the user name and groups, + * take care of subdomain users as well */ + if (strcasecmp(pd->domain, domain->name) != 0) { + user_dom = new_subdomain(tmp_ctx, domain, pd->domain, NULL, NULL); + if (user_dom == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); + ret = ENOMEM; + goto done; + } + ret = hbac_eval_user_element(eval_req, user_dom->sysdb, + pd->user, &eval_req->user); + } else { + ret = hbac_eval_user_element(eval_req, sysdb, + pd->user, &eval_req->user); + } if (ret != EOK) goto done;
/* Get the PAM service and service groups */ diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c index 88b52e26b2553eaa64a360b6aee9e19da1fa4326..b198e043580c4d1c3f65c41bc44c6d4749489e4b 100644 --- a/src/providers/ldap/sdap_access.c +++ b/src/providers/ldap/sdap_access.c @@ -139,6 +139,7 @@ sdap_access_send(TALLOC_CTX *mem_ctx, struct tevent_req *req; struct ldb_result *res; const char *attrs[] = { "*", NULL }; + struct sss_domain_info *user_dom;
req = tevent_req_create(mem_ctx, &state, struct sdap_access_req_ctx); if (req == NULL) { @@ -162,9 +163,21 @@ sdap_access_send(TALLOC_CTX *mem_ctx, goto done; }
- /* Get original user DN */ - ret = sysdb_get_user_attr(state, be_req->sysdb, - pd->user, attrs, &res); + /* Get original user DN, take care of subdomain users as well */ + if (strcasecmp(pd->domain, be_req->be_ctx->domain->name) != 0) { + user_dom = new_subdomain(state, be_req->be_ctx->domain, pd->domain, + NULL, NULL); + if (user_dom == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); + ret = ENOMEM; + goto done; + } + ret = sysdb_get_user_attr(state, user_dom->sysdb, + pd->user, attrs, &res); + } else { + ret = sysdb_get_user_attr(state, be_req->sysdb, + pd->user, attrs, &res); + } if (ret != EOK) { if (ret == ENOENT) { /* If we can't find the user, return permission denied */
In subdomains we have to use fully qualified usernames. Unfortunately we have no other good option than simply removing caches for users of subdomains. This is because the memberof plugin does not support the rename operation. --- src/db/sysdb.c | 7 ++++ src/db/sysdb_private.h | 4 ++- src/db/sysdb_upgrade.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 9685163b3f00cc2a45617072efa353589de111ce..e11df05aa8576b3c7118d217d14ecf487d519e40 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1114,6 +1114,13 @@ int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx, } }
+ if (strcmp(version, SYSDB_VERSION_0_13) == 0) { + ret = sysdb_upgrade_13(sysdb, &version); + if (ret != EOK) { + goto done; + } + } + /* The version should now match SYSDB_VERSION. * If not, it means we didn't match any of the * known older versions. The DB might be diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index bde4c603897be496755e773905b0408558376120..a2af8b93fee0f13f80d926b8ef964fd5de206cdb 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -23,6 +23,7 @@ #ifndef __INT_SYS_DB_H__ #define __INT_SYS_DB_H__
+#define SYSDB_VERSION_0_14 "0.14" #define SYSDB_VERSION_0_13 "0.13" #define SYSDB_VERSION_0_12 "0.12" #define SYSDB_VERSION_0_11 "0.11" @@ -37,7 +38,7 @@ #define SYSDB_VERSION_0_2 "0.2" #define SYSDB_VERSION_0_1 "0.1"
-#define SYSDB_VERSION SYSDB_VERSION_0_13 +#define SYSDB_VERSION SYSDB_VERSION_0_14
#define SYSDB_BASE_LDIF \ "dn: @ATTRIBUTES\n" \ @@ -111,6 +112,7 @@ int sysdb_upgrade_09(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_10(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_11(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_12(struct sysdb_ctx *sysdb, const char **ver); +int sysdb_upgrade_13(struct sysdb_ctx *sysdb, const char **ver);
int add_string(struct ldb_message *msg, int flags, const char *attr, const char *value); diff --git a/src/db/sysdb_upgrade.c b/src/db/sysdb_upgrade.c index c4ca64a48339eec90a5f071548496bd02f00646a..10c4e5775515b287dbdb63fcf66f8aeca8515245 100644 --- a/src/db/sysdb_upgrade.c +++ b/src/db/sysdb_upgrade.c @@ -1273,6 +1273,94 @@ done: return ret; }
+int sysdb_upgrade_13(struct sysdb_ctx *sysdb, const char **ver) +{ + struct upgrade_ctx *ctx; + struct ldb_result *dom_res; + struct ldb_result *res; + struct ldb_dn *basedn; + const char *attrs[] = { "cn", "name", NULL }; + const char *tmp_str; + errno_t ret; + int i, j, l, n; + + ret = commence_upgrade(sysdb, sysdb->ldb, SYSDB_VERSION_0_14, &ctx); + if (ret) { + return ret; + } + + basedn = ldb_dn_new(ctx, sysdb->ldb, SYSDB_BASE); + if (!basedn) { + DEBUG(SSSDBG_OP_FAILURE, ("Failed to build base dn\n")); + ret = EIO; + goto done; + } + + ret = ldb_search(sysdb->ldb, ctx, &dom_res, + basedn, LDB_SCOPE_ONELEVEL, + attrs, "objectclass=%s", SYSDB_SUBDOMAIN_CLASS); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, ("Failed to search subdomains\n")); + ret = EIO; + goto done; + } + + for (i = 0; i < dom_res->count; i++) { + + tmp_str = ldb_msg_find_attr_as_string(dom_res->msgs[i], "cn", NULL); + if (tmp_str == NULL) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("The object [%s] doesn't have a name\n", + ldb_dn_get_linearized(res->msgs[i]->dn))); + continue; + } + + basedn = ldb_dn_new_fmt(ctx, sysdb->ldb, SYSDB_DOM_BASE, tmp_str); + if (!basedn) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to build base dn for subdomain %s\n", tmp_str)); + continue; + } + + ret = ldb_search(sysdb->ldb, ctx, &res, + basedn, LDB_SCOPE_SUBTREE, attrs, NULL); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to search subdomain %s\n", tmp_str)); + talloc_free(basedn); + continue; + } + + l = ldb_dn_get_comp_num(basedn); + for (j = 0; j < res->count; j++) { + n = ldb_dn_get_comp_num(res->msgs[j]->dn); + if (n <= l + 1) { + /* Do not remove subdomain containers, only their contents */ + continue; + } + ret = ldb_delete(sysdb->ldb, res->msgs[j]->dn); + if (ret) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to delete %s\n", res->msgs[j]->dn)); + continue; + } + } + + talloc_free(basedn); + talloc_free(res); + } + + talloc_free(dom_res); + + /* conversion done, update version number */ + ret = update_version(ctx); + +done: + ret = finish_upgrade(ret, &ctx, ver); + return ret; +} + + /* * Example template for future upgrades. * Copy and change version numbers as appropriate.
Sumit found 2 issues in the patch.
1. the 2 new wrapper proptotypes used _res as variable names but that symbol is now used in glibc: /usr/include/resolv.h:#define _res (*__res_state())
Changed to 'res'
2. I was still using const char *src_name but it is unnecessary
Dropped the const
This should be hte last revision (last famous words :-)
Simo.
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.h | 9 ++++ src/db/sysdb_private.h | 4 +- src/db/sysdb_search.c | 4 +- src/db/sysdb_subdomains.c | 41 ++++++++++++++++ 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 | 10 +++- src/responder/pac/pacsrv_cmd.c | 15 +++++- src/responder/pac/pacsrv_utils.c | 52 +++++++++++--------- src/responder/pam/pamsrv_cmd.c | 5 ++- src/util/domain_info_utils.c | 2 +- 16 files changed, 298 insertions(+), 52 deletions(-)
The original sysdb code had a strong assumption that only users from one domain are saved in the databse, with the subdomain feature, we have changed reality, but have not adjusted all the code arund the sysdb calls to not rely on the original assumption.
One of the side effects of this incongrunece is that currently group memberships do not return fully qualified names for subdomain users as they should.
In oreder to fix this and other potential issues surrounding the violation of the original assumption, we need to fully qualify subdomain user names. By savin them fully qualified we do not risk aliasing local users and have group memberhips or other name based matching code mistake a domain user with subdomain usr or vice versa. --- src/db/sysdb.h | 9 ++++++ src/db/sysdb_search.c | 4 ++- src/db/sysdb_subdomains.c | 41 ++++++++++++++++++++++++++++ src/providers/data_provider_be.c | 11 +++++++ src/providers/ipa/ipa_s2n_exop.c | 54 +++++++++++++++++++++++++++++++++++--- src/responder/nss/nsssrv_cmd.c | 10 +++++-- src/responder/pac/pacsrv_cmd.c | 15 +++++++++- src/responder/pac/pacsrv_utils.c | 52 ++++++++++++++++++++---------------- src/responder/pam/pamsrv_cmd.c | 5 +++- src/util/domain_info_utils.c | 2 +- 10 files changed, 168 insertions(+), 35 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 39e0f9f5245c67f3798055823e98c2db0fbdebba..03a4644768a2e2e1c4b3d101a208d0289ef009d6 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -422,6 +422,15 @@ errno_t sysdb_store_domgroup(struct sss_domain_info *domain, errno_t sysdb_delete_domgroup(struct sss_domain_info *domain, const char *name, gid_t gid);
+int sysdb_subdom_getpwnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **res); +int sysdb_subdom_getgrnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **res); + errno_t sysdb_get_ranges(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb, size_t *range_count, struct range_info ***range_list); diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c index 1ab94770003a205485488c879c9deba856183291..49f628bfd3d15aba77b169b163c6ba15a7813744 100644 --- a/src/db/sysdb_search.c +++ b/src/db/sysdb_search.c @@ -365,7 +365,9 @@ int sysdb_initgroups(TALLOC_CTX *mem_ctx, return ENOMEM; }
- ret = sysdb_getpwnam(tmp_ctx, sysdb, name, &res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getpwnam(tmp_ctx, sysdb, name, &res); if (ret != EOK) { DEBUG(1, ("sysdb_getpwnam failed: [%d][%s]\n", ret, strerror(ret))); diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index 2e0170f4d37f04468e9a56dbca9b8176c017c9ff..d3ec7438ccff951e0d60a7e919b772684e6de415 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -668,3 +668,44 @@ errno_t sysdb_delete_domgroup(struct sss_domain_info *domain,
return sysdb_delete_group(domain->sysdb, name, gid); } + +int sysdb_subdom_getpwnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **res) +{ + char *src_name = NULL; + int ret; + + 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(mem_ctx, sysdb, src_name ? src_name : name, res); + talloc_zfree(src_name); + + return ret; +} + +int sysdb_subdom_getgrnam(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + const char *name, + struct ldb_result **res) +{ + char *src_name = NULL; + int ret; + + 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_getgrnam(mem_ctx, sysdb, src_name ? src_name : name, res); + talloc_zfree(src_name); + + return ret; +} + diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index 253dc46fb3010b2270ef454bb97e47243a18ceed..c43f32d5186f89d952e7ddf233040abb11c016e9 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -2188,6 +2188,17 @@ int be_process_init(TALLOC_CTX *mem_ctx, goto fail; }
+ /* We need this for subdomains support, as they have to store fully + * qualified user and group names for now */ + ret = sss_names_init(ctx->domain, cdb, + ctx->domain->name, &ctx->domain->names); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + ("fatal error setting fully qualified name format for %s\n", + ctx->domain->name)); + goto fail; + } + ret = be_srv_init(ctx); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, ("fatal error setting up server bus\n")); diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index 1a81c860901dc18a3d443266889701a8980fe031..8fc22819bf72eb194563da1bb3ded2678ac494e6 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -591,6 +591,9 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) uint64_t timeout = 10*60*60; /* FIXME: find a better timeout ! */ const char *homedir = NULL; struct sysdb_attrs *user_attrs = NULL; + char *name; + char *realm; + char *upn;
ret = ipa_s2n_exop_recv(subreq, state, &result, &retoid, &retdata); talloc_zfree(subreq); @@ -640,21 +643,64 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) goto done; }
- ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, - attrs->a.user.pw_name); + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.user.pw_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, name); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); + goto done; + } + + /* We also have to store a fake UPN here, because otherwise the + * krb5 child later won't be able to properly construct one as + * the username is fully qualified but the child doesn't have + * access to the regex to deconstruct it */ + /* FIXME: The real UPN is available from the PAC, we should get + * it from there. */ + realm = get_uppercase_realm(state, state->dom->name); + if (!realm) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to get realm.\n")); + ret = ENOMEM; + goto done; + } + upn = talloc_asprintf(state, "%s@%s", + attrs->a.user.pw_name, realm); + if (!upn) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format UPN.\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_UPN, upn); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); goto done; }
- ret = sysdb_store_domuser(state->dom, attrs->a.user.pw_name, NULL, + ret = sysdb_store_domuser(state->dom, name, NULL, attrs->a.user.pw_uid, 0, NULL, /* gecos */ homedir, NULL, user_attrs, NULL, timeout, now); break; case RESP_GROUP: - ret = sysdb_store_domgroup(state->dom, attrs->a.group.gr_name, + /* we always use the fully qualified name for subdomain users */ + name = talloc_asprintf(state, state->dom->names->fq_fmt, + attrs->a.group.gr_name, state->dom->name); + if (!name) { + DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name,\n")); + ret = ENOMEM; + goto done; + } + + ret = sysdb_store_domgroup(state->dom, name, attrs->a.group.gr_gid, NULL, timeout, now); break; diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 036e88f4c0428dd592b8067d7ec341728ef38ddb..c667a3f90f849e8b9af3806d91157a7ff37117a8 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -635,7 +635,7 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) dctx->domain = dom;
talloc_free(name); - name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive); + name = sss_get_cased_name(cmdctx, cmdctx->name, dom->case_sensitive); if (!name) return ENOMEM;
/* verify this user has not yet been negatively cached, @@ -666,7 +666,9 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getpwnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getpwnam(cmdctx, sysdb, name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; @@ -2201,7 +2203,9 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) return EIO; }
- ret = sysdb_getgrnam(cmdctx, sysdb, name, &dctx->res); + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getgrnam(cmdctx, sysdb, name, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c index 5721d9262f88387e95b926ae4c3fe4d2033d15de..202765a594aab1748b9a071b98e5175c1d3876ae 100644 --- a/src/responder/pac/pacsrv_cmd.c +++ b/src/responder/pac/pacsrv_cmd.c @@ -53,6 +53,7 @@ struct pac_req_ctx { struct pac_ctx *pac_ctx; const char *domain_name; const char *user_name; + char *fq_name; struct sss_domain_info *dom;
struct PAC_LOGON_INFO *logon_info; @@ -201,6 +202,16 @@ static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx) struct dom_sid *my_dom_sid; struct local_mapping_ranges *my_range_map;
+ /* this is a subdomain so we need to search for the fully qualified + * name in the database */ + pr_ctx->fq_name = talloc_asprintf(pr_ctx, pr_ctx->dom->names->fq_fmt, + pr_ctx->user_name, pr_ctx->dom->name); + if (!pr_ctx->fq_name) { + ret = ENOMEM; + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + goto done; + } + ret = save_pac_user(pr_ctx); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("save_pac_user failed.\n")); @@ -365,7 +376,7 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx) goto done; }
- ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->user_name, attrs, + ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->fq_name, attrs, &msg); if (ret == EOK) { /* TODO: check id uid and gid are equal. */ @@ -423,7 +434,7 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx) }
state->gid_iter = 0; - state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->user_name); + state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->fq_name); if (state->user_dn == NULL) { ret = ENOMEM; goto done; diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c index c9551c998332b053833aaa1d08e435bcfec0ec21..53113fb0da18cb235c0c049b14bd836556f647ec 100644 --- a/src/responder/pac/pacsrv_utils.c +++ b/src/responder/pac/pacsrv_utils.c @@ -502,6 +502,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, struct sysdb_attrs *attrs = NULL; struct netr_SamBaseInfo *base_info; int ret; + char *lname; char *uc_realm; char *upn;
@@ -513,36 +514,41 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
base_info = &logon_info->info3.base;
- if (base_info->account_name.size != 0) { - /* To be compatible with winbind based lookups we have to use lower - * case names only, effectively making the domain case-insenvitive. */ - pwd->pw_name = sss_tc_utf8_str_tolower(pwd, - base_info->account_name.string); - if (pwd->pw_name == NULL) { - DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); - ret = ENOMEM; - goto done; - } - } else { + if (base_info->account_name.size == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing account name in PAC.\n")); ret = EINVAL; goto done; } - - if (base_info->rid > 0) { - ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, - base_info->domain_sid, - base_info->rid, &pwd->pw_uid); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); - goto done; - } - } else { + if (base_info->rid == 0) { DEBUG(SSSDBG_OP_FAILURE, ("Missing user RID in PAC.\n")); ret = EINVAL; goto done; }
+ /* To be compatible with winbind based lookups we have to use lower + * case names only, effectively making the domain case-insenvitive. */ + lname = sss_tc_utf8_str_tolower(pwd, base_info->account_name.string); + if (lname == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n")); + ret = ENOMEM; + goto done; + } + pwd->pw_name = talloc_asprintf(pwd, dom->names->fq_fmt, + lname, dom->name); + if (!pwd->pw_name) { + DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n")); + ret = ENOMEM; + goto done; + } + + ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name, + base_info->domain_sid, + base_info->rid, &pwd->pw_uid); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n")); + goto done; + } + pwd->pw_gid = 0; /* We use MPGs for sub-domains */
if (base_info->full_name.size != 0) { @@ -559,7 +565,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
if (dom->subdomain_homedir) { pwd->pw_dir = expand_homedir_template(pwd, dom->subdomain_homedir, - pwd->pw_name, pwd->pw_uid, + lname, pwd->pw_uid, dom->name); if (pwd->pw_dir == NULL) { ret = ENOMEM; @@ -583,7 +589,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx, goto done; }
- upn = talloc_asprintf(mem_ctx, "%s@%s", pwd->pw_name, uc_realm); + upn = talloc_asprintf(mem_ctx, "%s@%s", lname, uc_realm); talloc_free(uc_realm); if (upn == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("talloc_asprintf failed.\n")); diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 1702a0e91413387e457dc2352cc9d4e693119212..4269642206cc0295c0046de4e59a3ad8f1044d1a 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1217,7 +1217,10 @@ static int pam_check_user_search(struct pam_auth_req *preq) preq->pd->pam_status = PAM_SYSTEM_ERR; return EFAULT; } - ret = sysdb_getpwnam(preq, sysdb, name, &preq->res); + + /* if this is a subdomain we need to search for the fully qualified + * name in the database */ + ret = sysdb_subdom_getpwnam(preq, sysdb, name, &preq->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); return EIO; diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index 6ee3545520f52171e04bdc096d84fc67dbf82f2e..a6aa5c73327456d2323898af786cb43e0b69d7da 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -73,7 +73,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, }
dom->enumerate = false; - dom->fqnames = true; + dom->fqnames = false; /* FIXME: get ranges from the server */ dom->id_min = 0; dom->id_max = 0xffffffff;
From: Sumit Bose sbose@redhat.com
Currently the sysdb context is pointed to the subdomain subtree containing user the user to be checked at the beginning of a HBAC request. As a result all HBAC rules and related data is save in the subdomain tree as well. But since the HBAC rules of the configured domain apply to all users it is sufficient to save them once in the subtree of the configured domain.
Since most of the sysdb operations during a HBAC request are related to the HBAC rules and related data this patch does not change the default sysdb context but only create a special context to look up subdomain users. --- src/providers/ipa/ipa_access.c | 10 ---------- src/providers/ipa/ipa_hbac_common.c | 19 ++++++++++++++++--- src/providers/ldap/sdap_access.c | 19 ++++++++++++++++--- 3 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c index 5c97575fca6302e8f1f89a67e14af6a723e1ef03..3a34864c4bb059b837fc38bd2083390c03ae315c 100644 --- a/src/providers/ipa/ipa_access.c +++ b/src/providers/ipa/ipa_access.c @@ -85,16 +85,6 @@ void ipa_access_handler(struct be_req *be_req) be_req->be_ctx->bet_info[BET_ACCESS].pvt_bet_data, struct ipa_access_ctx);
- if (strcasecmp(pd->domain, be_req->be_ctx->domain->name) != 0) { - be_req->domain = new_subdomain(be_req, be_req->be_ctx->domain, pd->domain, NULL, NULL); - if (be_req->domain == NULL) { - DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); - be_req->fn(be_req, DP_ERR_FATAL, PAM_SYSTEM_ERR, NULL); - return; - } - be_req->sysdb = be_req->domain->sysdb; - } - /* First, verify that this account isn't locked. * We need to do this in case the auth phase was * skipped (such as during GSSAPI single-sign-on diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c index 54628d80b6202b61ea3917353688f0705412c83e..33d1944eee682295f0f19c611917684f98df5a92 100644 --- a/src/providers/ipa/ipa_hbac_common.c +++ b/src/providers/ipa/ipa_hbac_common.c @@ -440,6 +440,7 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain = hbac_ctx_be(hbac_ctx)->domain; const char *rhost; const char *thost; + struct sss_domain_info *user_dom;
tmp_ctx = talloc_new(mem_ctx); if (tmp_ctx == NULL) return ENOMEM; @@ -452,9 +453,21 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx,
eval_req->request_time = time(NULL);
- /* Get user the user name and groups */ - ret = hbac_eval_user_element(eval_req, sysdb, - pd->user, &eval_req->user); + /* Get user the user name and groups, + * take care of subdomain users as well */ + if (strcasecmp(pd->domain, domain->name) != 0) { + user_dom = new_subdomain(tmp_ctx, domain, pd->domain, NULL, NULL); + if (user_dom == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); + ret = ENOMEM; + goto done; + } + ret = hbac_eval_user_element(eval_req, user_dom->sysdb, + pd->user, &eval_req->user); + } else { + ret = hbac_eval_user_element(eval_req, sysdb, + pd->user, &eval_req->user); + } if (ret != EOK) goto done;
/* Get the PAM service and service groups */ diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c index 88b52e26b2553eaa64a360b6aee9e19da1fa4326..b198e043580c4d1c3f65c41bc44c6d4749489e4b 100644 --- a/src/providers/ldap/sdap_access.c +++ b/src/providers/ldap/sdap_access.c @@ -139,6 +139,7 @@ sdap_access_send(TALLOC_CTX *mem_ctx, struct tevent_req *req; struct ldb_result *res; const char *attrs[] = { "*", NULL }; + struct sss_domain_info *user_dom;
req = tevent_req_create(mem_ctx, &state, struct sdap_access_req_ctx); if (req == NULL) { @@ -162,9 +163,21 @@ sdap_access_send(TALLOC_CTX *mem_ctx, goto done; }
- /* Get original user DN */ - ret = sysdb_get_user_attr(state, be_req->sysdb, - pd->user, attrs, &res); + /* Get original user DN, take care of subdomain users as well */ + if (strcasecmp(pd->domain, be_req->be_ctx->domain->name) != 0) { + user_dom = new_subdomain(state, be_req->be_ctx->domain, pd->domain, + NULL, NULL); + if (user_dom == NULL) { + DEBUG(SSSDBG_OP_FAILURE, ("new_subdomain failed.\n")); + ret = ENOMEM; + goto done; + } + ret = sysdb_get_user_attr(state, user_dom->sysdb, + pd->user, attrs, &res); + } else { + ret = sysdb_get_user_attr(state, be_req->sysdb, + pd->user, attrs, &res); + } if (ret != EOK) { if (ret == ENOENT) { /* If we can't find the user, return permission denied */
In subdomains we have to use fully qualified usernames. Unfortunately we have no other good option than simply removing caches for users of subdomains. This is because the memberof plugin does not support the rename operation. --- src/db/sysdb.c | 7 ++++ src/db/sysdb_private.h | 4 ++- src/db/sysdb_upgrade.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 9685163b3f00cc2a45617072efa353589de111ce..e11df05aa8576b3c7118d217d14ecf487d519e40 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1114,6 +1114,13 @@ int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx, } }
+ if (strcmp(version, SYSDB_VERSION_0_13) == 0) { + ret = sysdb_upgrade_13(sysdb, &version); + if (ret != EOK) { + goto done; + } + } + /* The version should now match SYSDB_VERSION. * If not, it means we didn't match any of the * known older versions. The DB might be diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index bde4c603897be496755e773905b0408558376120..a2af8b93fee0f13f80d926b8ef964fd5de206cdb 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -23,6 +23,7 @@ #ifndef __INT_SYS_DB_H__ #define __INT_SYS_DB_H__
+#define SYSDB_VERSION_0_14 "0.14" #define SYSDB_VERSION_0_13 "0.13" #define SYSDB_VERSION_0_12 "0.12" #define SYSDB_VERSION_0_11 "0.11" @@ -37,7 +38,7 @@ #define SYSDB_VERSION_0_2 "0.2" #define SYSDB_VERSION_0_1 "0.1"
-#define SYSDB_VERSION SYSDB_VERSION_0_13 +#define SYSDB_VERSION SYSDB_VERSION_0_14
#define SYSDB_BASE_LDIF \ "dn: @ATTRIBUTES\n" \ @@ -111,6 +112,7 @@ int sysdb_upgrade_09(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_10(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_11(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_12(struct sysdb_ctx *sysdb, const char **ver); +int sysdb_upgrade_13(struct sysdb_ctx *sysdb, const char **ver);
int add_string(struct ldb_message *msg, int flags, const char *attr, const char *value); diff --git a/src/db/sysdb_upgrade.c b/src/db/sysdb_upgrade.c index c4ca64a48339eec90a5f071548496bd02f00646a..10c4e5775515b287dbdb63fcf66f8aeca8515245 100644 --- a/src/db/sysdb_upgrade.c +++ b/src/db/sysdb_upgrade.c @@ -1273,6 +1273,94 @@ done: return ret; }
+int sysdb_upgrade_13(struct sysdb_ctx *sysdb, const char **ver) +{ + struct upgrade_ctx *ctx; + struct ldb_result *dom_res; + struct ldb_result *res; + struct ldb_dn *basedn; + const char *attrs[] = { "cn", "name", NULL }; + const char *tmp_str; + errno_t ret; + int i, j, l, n; + + ret = commence_upgrade(sysdb, sysdb->ldb, SYSDB_VERSION_0_14, &ctx); + if (ret) { + return ret; + } + + basedn = ldb_dn_new(ctx, sysdb->ldb, SYSDB_BASE); + if (!basedn) { + DEBUG(SSSDBG_OP_FAILURE, ("Failed to build base dn\n")); + ret = EIO; + goto done; + } + + ret = ldb_search(sysdb->ldb, ctx, &dom_res, + basedn, LDB_SCOPE_ONELEVEL, + attrs, "objectclass=%s", SYSDB_SUBDOMAIN_CLASS); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, ("Failed to search subdomains\n")); + ret = EIO; + goto done; + } + + for (i = 0; i < dom_res->count; i++) { + + tmp_str = ldb_msg_find_attr_as_string(dom_res->msgs[i], "cn", NULL); + if (tmp_str == NULL) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("The object [%s] doesn't have a name\n", + ldb_dn_get_linearized(res->msgs[i]->dn))); + continue; + } + + basedn = ldb_dn_new_fmt(ctx, sysdb->ldb, SYSDB_DOM_BASE, tmp_str); + if (!basedn) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to build base dn for subdomain %s\n", tmp_str)); + continue; + } + + ret = ldb_search(sysdb->ldb, ctx, &res, + basedn, LDB_SCOPE_SUBTREE, attrs, NULL); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to search subdomain %s\n", tmp_str)); + talloc_free(basedn); + continue; + } + + l = ldb_dn_get_comp_num(basedn); + for (j = 0; j < res->count; j++) { + n = ldb_dn_get_comp_num(res->msgs[j]->dn); + if (n <= l + 1) { + /* Do not remove subdomain containers, only their contents */ + continue; + } + ret = ldb_delete(sysdb->ldb, res->msgs[j]->dn); + if (ret) { + DEBUG(SSSDBG_OP_FAILURE, + ("Failed to delete %s\n", res->msgs[j]->dn)); + continue; + } + } + + talloc_free(basedn); + talloc_free(res); + } + + talloc_free(dom_res); + + /* conversion done, update version number */ + ret = update_version(ctx); + +done: + ret = finish_upgrade(ret, &ctx, ver); + return ret; +} + + /* * Example template for future upgrades. * Copy and change version numbers as appropriate.
On Fri, Nov 16, 2012 at 04:25:41PM -0500, Simo Sorce wrote:
Sumit found 2 issues in the patch.
- the 2 new wrapper proptotypes used _res as variable names but that symbol is now used in glibc: /usr/include/resolv.h:#define _res (*__res_state())
Changed to 'res'
- I was still using const char *src_name but it is unnecessary
Dropped the const
This should be hte last revision (last famous words :-)
:-)
ACK
bye, Sumit
Simo.
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.h | 9 ++++ src/db/sysdb_private.h | 4 +- src/db/sysdb_search.c | 4 +- src/db/sysdb_subdomains.c | 41 ++++++++++++++++ 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 | 10 +++- src/responder/pac/pacsrv_cmd.c | 15 +++++- src/responder/pac/pacsrv_utils.c | 52 +++++++++++--------- src/responder/pam/pamsrv_cmd.c | 5 ++- src/util/domain_info_utils.c | 2 +- 16 files changed, 298 insertions(+), 52 deletions(-)
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Sun, Nov 18, 2012 at 06:05:45PM +0100, Sumit Bose wrote:
On Fri, Nov 16, 2012 at 04:25:41PM -0500, Simo Sorce wrote:
Sumit found 2 issues in the patch.
- the 2 new wrapper proptotypes used _res as variable names but that symbol is now used in glibc: /usr/include/resolv.h:#define _res (*__res_state())
Changed to 'res'
- I was still using const char *src_name but it is unnecessary
Dropped the const
This should be hte last revision (last famous words :-)
:-)
ACK
bye, Sumit
I can't apply patch 3 on top of origin/master, can you rebase?
On Sun, 2012-11-18 at 18:27 +0100, Jakub Hrozek wrote:
On Sun, Nov 18, 2012 at 06:05:45PM +0100, Sumit Bose wrote:
On Fri, Nov 16, 2012 at 04:25:41PM -0500, Simo Sorce wrote:
Sumit found 2 issues in the patch.
- the 2 new wrapper proptotypes used _res as variable names but that symbol is now used in glibc: /usr/include/resolv.h:#define _res (*__res_state())
Changed to 'res'
- I was still using const char *src_name but it is unnecessary
Dropped the const
This should be hte last revision (last famous words :-)
:-)
ACK
bye, Sumit
I can't apply patch 3 on top of origin/master, can you rebase? _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Have you applied this https://patchwork.acksyn.org/patch/336/ patch first ?
It's a dependency.
Simo.
On Sun, Nov 18, 2012 at 03:54:54PM -0500, Simo Sorce wrote:
On Sun, 2012-11-18 at 18:27 +0100, Jakub Hrozek wrote:
On Sun, Nov 18, 2012 at 06:05:45PM +0100, Sumit Bose wrote:
On Fri, Nov 16, 2012 at 04:25:41PM -0500, Simo Sorce wrote:
Sumit found 2 issues in the patch.
- the 2 new wrapper proptotypes used _res as variable names but that symbol is now used in glibc: /usr/include/resolv.h:#define _res (*__res_state())
Changed to 'res'
- I was still using const char *src_name but it is unnecessary
Dropped the const
This should be hte last revision (last famous words :-)
:-)
ACK
bye, Sumit
I can't apply patch 3 on top of origin/master, can you rebase? _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Have you applied this https://patchwork.acksyn.org/patch/336/ patch first ?
It's a dependency.
Simo.
No, sorry, I forgot. I'll get it reviewed so that we can push this patchset.
On Sun, Nov 18, 2012 at 10:29:46PM +0100, Jakub Hrozek wrote:
On Sun, Nov 18, 2012 at 03:54:54PM -0500, Simo Sorce wrote:
On Sun, 2012-11-18 at 18:27 +0100, Jakub Hrozek wrote:
On Sun, Nov 18, 2012 at 06:05:45PM +0100, Sumit Bose wrote:
On Fri, Nov 16, 2012 at 04:25:41PM -0500, Simo Sorce wrote:
Sumit found 2 issues in the patch.
- the 2 new wrapper proptotypes used _res as variable names but that symbol is now used in glibc: /usr/include/resolv.h:#define _res (*__res_state())
Changed to 'res'
- I was still using const char *src_name but it is unnecessary
Dropped the const
This should be hte last revision (last famous words :-)
:-)
ACK
bye, Sumit
I can't apply patch 3 on top of origin/master, can you rebase? _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Have you applied this https://patchwork.acksyn.org/patch/336/ patch first ?
It's a dependency.
Simo.
No, sorry, I forgot. I'll get it reviewed so that we can push this patchset.
Sumit did the upgrade patch review and I pushed all three patches in this thread to master and sssd-1-9
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Pavel Březina -
Simo Sorce -
Sumit Bose