On Wed, 2014-01-22 at 17:31 +0100, Jakub Hrozek wrote:
> On Wed, Jan 22, 2014 at 05:13:38PM +0100, Pavel Reichl wrote:
> > On Fri, 2014-01-17 at 13:02 +0100, Sumit Bose wrote:
> > > On Fri, Jan 17, 2014 at 12:56:09PM +0100, Jakub Hrozek wrote:
> > > > On Fri, Jan 17, 2014 at 12:43:04PM +0100, Sumit Bose wrote:
> > > > > On Fri, Jan 17, 2014 at 12:04:38PM +0100, Jakub Hrozek wrote:
> > > > > > On Fri, Jan 17, 2014 at 11:55:08AM +0100, Pavel Reichl wrote:
> > > > > > >
> > > > > > > > We just have to remember to touch this code, if we start to
> > > > > > > > support home
> > > > > > > > directories defined in AD.
> > > > > > > >
> > > > > > >
> > > > > > > I'm sorry that I failed to see what you were implying. My patch
> > > > > > > will
> > > > > > > override value of homedir for members of every subdomain by
> > > > > > > subdomain_homedir.
> > > > > > >
> > > > > > > I missed that even if subdomain_homedir is not set explicitly in
> > > > > > > sssd.conf it has a non NULL value.
> > > > > >
> > > > > > Me too and to be honest I failed to test the patch with AD
> > > > > > subdomains
> > > > > > with POSIX attributes. Sorry about that.
> > > > > >
> > > > > > I think Sumit's suggestion makes the most sense now, but the
> > > > > > documentation should be amended, too. Currently the man page implies
> > > > > > that the subdomain_homedir parameter will have equal effect for all
> > > > > > kinds of subdomains, while the code would only use it for IPA-AD
> > > > > > trust
> > > > > > cases (be it for the server mode or the client).
> > > > >
> > > > > I also realized that having the right home directory in sysdb might be
> > > > > useful because krb5_ccname_template allows to expand %h to the user's
> > > > > home directory. And here the home directory is looked up in sysdb.
> > > > >
> > > > > bye,
> > > > > Sumit
> > > >
> > > > But that also affects the other options, right? Sounds like a second
> > > > problem to me, or did you want to solve both together?
> > >
> > > no
> > >
> > > >
> > > > About the subdomain homedir, I think we should:
> > > > 1) revert the original patch that is commited
> > > > 2) fix the IPA server mode the way you described in your reply to
> > > > Pavel's mail
> > > > 3) Amend the subdomain_homedir documentation so that it's clear
> > > > where the option has effect
> > >
> > > +1
> > >
> > > bye,
> > > Sumit
> > >
> > > > _______________________________________________
> > > > sssd-devel mailing list
> > > > sssd-devel@lists.fedorahosted.org
> > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > > _______________________________________________
> > > sssd-devel mailing list
> > > sssd-devel@lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
> > Hello,
> >
> > please see attached patches.
> >
> > PR
> >
>
> I haven't tried the patches yet, just read the diff..
>
> > From 4300f985377bfe16c40faf3f373e1875e5f80433 Mon Sep 17 00:00:00 2001
> > From: Pavel Reichl <prei...@redhat.com>
> > Date: Tue, 21 Jan 2014 15:06:37 +0000
> > Subject: [PATCH 1/2] Revert "NSS: add support for subdomain_homedir"
>
> Looks good to me.
>
> > From 5eb3fd0cc7a11b0f71fa10a3d41028738382bc8e Mon Sep 17 00:00:00 2001
> > From: Pavel Reichl <prei...@redhat.com>
> > Date: Wed, 22 Jan 2014 16:47:22 +0000
> > Subject: [PATCH 2/2] AD: support for subdomain_homedir
> >
>
> See two comments inline
>
> > If users from AD don't have set homedir then subdomain_homedir is used
> > as default.
> >
> > Resolves:
> > https://fedorahosted.org/sssd/ticket/2169
> > ---
> > src/providers/ipa/ipa_subdomains_id.c | 89
> > +++++++++++++++++++++++++++++++++++
> > 1 file changed, 89 insertions(+)
> >
> > diff --git a/src/providers/ipa/ipa_subdomains_id.c
> > b/src/providers/ipa/ipa_subdomains_id.c
> > index
> > c29a2a3047af105966b636422105abd15e8a3992..a498a01e9dcc3e3a4c7b59323da9dae017876686
> > 100644
> > --- a/src/providers/ipa/ipa_subdomains_id.c
> > +++ b/src/providers/ipa/ipa_subdomains_id.c
> > @@ -350,6 +350,70 @@ ipa_get_ad_id_ctx(struct ipa_id_ctx *ipa_ctx,
> > return (iter) ? iter->ad_id_ctx : NULL;
> > }
> >
> > +static errno_t
> > +use_subdomain_homedir(TALLOC_CTX *mem_ctx, const struct sss_domain_info
> > *dom,
> > + const char *name, uint32_t uid)
> > +{
> > + errno_t ret;
> > + char *fqname = NULL;
> > + char *homedir = NULL;
> > + struct sysdb_attrs *attrs = NULL;
> > + struct sysdb_ctx *sysdb = dom->sysdb;
>
> You can create a child context here:
> TALLOC_CTX *tmp_ctx = NULL;
>
> tmp_ctx = talloc_new(mem_ctx); /* or NULL */
> if (tmp_ctx == NULL) {
> return ENOMEM;
> }
> > +
> > + fqname = talloc_asprintf(mem_ctx, "%s@%s", name, dom->name);
>
> Then use tmp_ctx instead of mem_ctx through the function
>
> > + if (fqname == NULL) {
> > + ret = ENOMEM;
> > + goto done;
> > + }
> > +
> > + attrs = sysdb_new_attrs(mem_ctx);
> > + if (attrs == NULL) {
> > + ret = ENOMEM;
> > + goto done;
> > + }
> > +
> > + homedir = expand_homedir_template(mem_ctx, dom->subdomain_homedir,
> > name,
> > + uid, NULL, dom->name,
> > dom->flat_name);
> > + if (homedir == NULL) {
> > + DEBUG(SSSDBG_OP_FAILURE, ("expand_homedir_template failed\n"));
> > + ret = ENOMEM;
> > + goto done;
> > + }
> > +
> > + ret = sysdb_attrs_add_string(attrs, SYSDB_HOMEDIR, homedir);
> > + if (ret) {
> > + DEBUG(SSSDBG_MINOR_FAILURE, ("Error setting homedir: [%s]\n",
> > + strerror(ret)));
> > + goto done;
> > + }
> > +
> > + ret = sysdb_transaction_start(sysdb);
> > + if (ret != EOK) {
> > + DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
> > + goto done;
> > + }
> > +
> > + ret = sysdb_set_user_attr(dom, fqname, attrs, SYSDB_MOD_REP);
> > + if (ret) {
> > + DEBUG(SSSDBG_CRIT_FAILURE,
> > + ("Failed to update Login attempt information!\n"));
> > + }
> > +
> > + ret = sysdb_transaction_commit(sysdb);
> > + if (ret != EOK) {
> > + DEBUG(SSSDBG_CRIT_FAILURE,
> > + ("Cannot commit sysdb transaction [%d]: %s\n",
> > + ret, strerror(ret)));
> > + goto done;
> > + }
> > +
> > +done:
> > + talloc_zfree(attrs);
> > + talloc_zfree(fqname);
> > + talloc_zfree(homedir);
>
> And finally since all three variables are allocated on tmp_ctx, you can just
> free the context:
> talloc_free(tmp_ctx);
>
> > + return ret;
> > +}
> > +
> > static void
> > ipa_get_ad_acct_ad_part_done(struct tevent_req *subreq)
> > {
> > @@ -358,6 +422,9 @@ ipa_get_ad_acct_ad_part_done(struct tevent_req *subreq)
> > struct ipa_get_ad_acct_state *state = tevent_req_data(req,
> > struct
> > ipa_get_ad_acct_state);
> > errno_t ret;
> > + uint32_t uid;
> > + char *homedir;
> > + struct ldb_result *res;
> >
> > ret = ad_handle_acct_info_recv(subreq, &state->dp_error, NULL);
> > talloc_zfree(subreq);
> > @@ -367,6 +434,28 @@ ipa_get_ad_acct_ad_part_done(struct tevent_req *subreq)
> > return;
> > }
> >
> > + ret = sysdb_getpwnam(state, state->user_dom, state->ar->filter_value,
> > &res);
> > + if (ret != EOK) {
> > + DEBUG(SSSDBG_OP_FAILURE,
> > + ("Failed to make request to our cache: [%d]: [%s]\n",
> > + ret, sss_strerror(ret)));
> > + return;
> > + }
>
> I think here ^^^ you also need to branch based on the value of
> ar->filter_type. The filter_type can also be an ID number, in which case
> you'd search the user by ID.
>
> > +
> > + homedir = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_HOMEDIR,
> > NULL);
> > +
> > + if (homedir == NULL) {
> > + uid = ldb_msg_find_attr_as_uint64(res->msgs[0], SYSDB_UIDNUM, 0);
> > + ret = use_subdomain_homedir(state, state->user_dom,
> > + state->ar->filter_value, uid);
> > + if (ret != EOK) {
> > + DEBUG(SSSDBG_OP_FAILURE,
> > + ("use_subdomain_homedir failed: [%d]: [%s]\n",
> > + ret, sss_strerror(ret)));
> > + return;
> > + }
> > + }
> > +
> > if ((state->ar->entry_type & BE_REQ_TYPE_MASK) != BE_REQ_INITGROUPS) {
> > tevent_req_done(req);
> > return;
> > --
> > 1.8.4.2
> >
Hi Jakub,
new patch addressing your comments is attached.
PR
>From 4300f985377bfe16c40faf3f373e1875e5f80433 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <prei...@redhat.com>
Date: Tue, 21 Jan 2014 15:06:37 +0000
Subject: [PATCH 1/2] Revert "NSS: add support for subdomain_homedir"
This reverts commit 1dc7694a1cbc62b0d7e23cc1369579e5ce0071e8.
---
src/responder/nss/nsssrv_cmd.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 6a1e6a06a5e5323c59c2ee1973d207e82b473f93..2e2d7c86adf6d6444652435f888748385c64acf2 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -201,14 +201,6 @@ static const char *get_homedir_override(TALLOC_CTX *mem_ctx,
name, uid, homedir, dom->name, NULL);
}
- /* Override home directory location for subdomains.
- * This option can be overriden by override_homedir.
- */
- if (IS_SUBDOMAIN(dom) && dom->subdomain_homedir) {
- return expand_homedir_template(mem_ctx, dom->subdomain_homedir,
- name, uid, homedir, dom->name, NULL);
- }
-
if (!homedir || *homedir == '\0') {
/* In the case of a NULL or empty homedir, check to see if
* we have a fallback homedir to use.
--
1.8.4.2
>From 4a2a326b383ae22fab1a8f572276a880b5b51ee2 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <prei...@redhat.com>
Date: Wed, 22 Jan 2014 16:47:22 +0000
Subject: [PATCH 2/2] AD: support for subdomain_homedir
If users from AD don't have set homedir then subdomain_homedir is used
as default.
Resolves:
https://fedorahosted.org/sssd/ticket/2169
---
src/providers/ipa/ipa_subdomains_id.c | 147 ++++++++++++++++++++++++++++++++++
1 file changed, 147 insertions(+)
diff --git a/src/providers/ipa/ipa_subdomains_id.c b/src/providers/ipa/ipa_subdomains_id.c
index c29a2a3047af105966b636422105abd15e8a3992..50193b71ae293b0fd3a33714086784766f53ef35 100644
--- a/src/providers/ipa/ipa_subdomains_id.c
+++ b/src/providers/ipa/ipa_subdomains_id.c
@@ -350,6 +350,143 @@ ipa_get_ad_id_ctx(struct ipa_id_ctx *ipa_ctx,
return (iter) ? iter->ad_id_ctx : NULL;
}
+static errno_t
+store_subdomain_homedir(TALLOC_CTX *mem_ctx, const struct sss_domain_info *dom,
+ const char *fqname, uint32_t uid)
+{
+ char *name;
+ errno_t ret;
+ errno_t sret;
+ char *homedir;
+ char* end_of_name;
+ TALLOC_CTX *tmp_ctx;
+ struct sysdb_attrs *attrs;
+ struct sysdb_ctx *sysdb = dom->sysdb;
+
+ tmp_ctx = talloc_new(mem_ctx);
+ if (tmp_ctx == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ name = talloc_strdup(tmp_ctx, fqname);
+ if (attrs == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ /* fqname = name@domain */
+ end_of_name = strchr(name, '@');
+ if (end_of_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, ("%s is not fully qualified name.\n", fqname));
+ ret = EINVAL;
+ goto done;
+ }
+ *end_of_name = '\0';
+
+ attrs = sysdb_new_attrs(tmp_ctx);
+ if (attrs == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ homedir = expand_homedir_template(tmp_ctx, dom->subdomain_homedir, name,
+ uid, NULL, dom->name, dom->flat_name);
+ if (homedir == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, ("expand_homedir_template failed\n"));
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = sysdb_attrs_add_string(attrs, SYSDB_HOMEDIR, homedir);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Error setting homedir: [%s]\n",
+ strerror(ret)));
+ goto done;
+ }
+
+ ret = sysdb_transaction_start(sysdb);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
+ goto done;
+ }
+
+ ret = sysdb_set_user_attr(dom, fqname, attrs, SYSDB_MOD_REP);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Failed to update Login attempt information!\n"));
+ sret = sysdb_transaction_cancel(sysdb);
+ if (sret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Could not cancel transaction\n"));
+ }
+ goto done;
+ }
+
+ ret = sysdb_transaction_commit(sysdb);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Cannot commit sysdb transaction [%d]: %s\n",
+ ret, strerror(ret)));
+ goto done;
+ }
+
+done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
+
+static errno_t
+use_subdomain_homedir(TALLOC_CTX *mem_ctx, const struct sss_domain_info *dom,
+ int filter_type, const char *filter_value)
+{
+ errno_t ret;
+ uint32_t uid;
+ char *fqname;
+ char *homedir;
+ struct ldb_result *res;
+
+ if (filter_type == BE_FILTER_NAME) {
+ ret = sysdb_getpwnam(mem_ctx, dom, filter_value, &res);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("Failed to make request to our cache: [%d]: [%s]\n",
+ ret, sss_strerror(ret)));
+ goto done;
+ }
+ uid = ldb_msg_find_attr_as_uint64(res->msgs[0], SYSDB_UIDNUM, 0);
+ } else if (filter_type == BE_FILTER_IDNUM) {
+ uid = strtouint32(filter_value, NULL, 10);
+ ret = sysdb_getpwuid(mem_ctx, dom, uid, &res);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("Failed to make request to our cache: [%d]: [%s]\n",
+ ret, sss_strerror(ret)));
+ goto done;
+ }
+ } else {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("Unsoported filter type: [%d].\n", filter_type));
+ ret = EINVAL;
+ goto done;
+ }
+
+ homedir = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_HOMEDIR, NULL);
+ fqname = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_NAME, NULL);
+
+ if (homedir == NULL) {
+ ret = store_subdomain_homedir(mem_ctx, dom, fqname, uid);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("use_subdomain_homedir failed: [%d]: [%s]\n",
+ ret, sss_strerror(ret)));
+ return;
+ }
+ }
+done:
+ return ret;
+}
+
static void
ipa_get_ad_acct_ad_part_done(struct tevent_req *subreq)
{
@@ -367,6 +504,16 @@ ipa_get_ad_acct_ad_part_done(struct tevent_req *subreq)
return;
}
+ ret = use_subdomain_homedir(state, state->user_dom,
+ state->ar->filter_type,
+ state->ar->filter_value);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("use_subdomain_homedir failed: [%d]: [%s]\n",
+ ret, sss_strerror(ret)));
+ return;
+ }
+
if ((state->ar->entry_type & BE_REQ_TYPE_MASK) != BE_REQ_INITGROUPS) {
tevent_req_done(req);
return;
--
1.8.4.2
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
