On Thu, Oct 15, 2009 at 12:12:57PM +0200, Sumit Bose wrote: > On Wed, Oct 14, 2009 at 01:33:18PM -0400, Stephen Gallagher wrote: > > -----BEGIN PGP SIGNED MESSAGE----- > > Hash: SHA1 > > > > On 10/14/2009 07:24 AM, Sumit Bose wrote: > > > Hi, > > > > > > if auth_provider is set to a provider which can handle change password > > > request, e.g. LDAP, but chpass_provider is not set at all the user might > > > be surprised that the password connet be changed. This patch sets > > > chpass_provider implicit if an auth_provider is available and can handle > > > change password requests. If you want to disallow password changes > > > explicitly use 'chpass_provider = none'. > > > > > > This patch should fix one part of #220. > > > > > > bye, > > > Sumit > > > > > > > > > > > > _______________________________________________ > > > sssd-devel mailing list > > > sssd-devel@lists.fedorahosted.org > > > https://fedorahosted.org/mailman/listinfo/sssd-devel > > > > As discussed on IRC, if auth_provider is unspecified, we should use > > id_provider as the default for both auth_provider and access_provider > > (if the id_provider offers that option). > > > > I think access provider should default to 'permit' unless otherwise > > specified. > > > > > > - -- > > Stephen Gallagher > > RHCE 804006346421761 > > > > ok, I've created a new patch (0002) with the changes, 0001 is rebased > but unchanged. I've also added a 'deny' option for access_provider and a > man page entry. > > bye, > Sumit
Please find attached rebased versions. bye, Sumit
>From f3a0e41ae3123cd68d9e9ac8193fb20ed9d52d68 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Wed, 14 Oct 2009 12:49:10 +0200 Subject: [PATCH 1/2] set chpass_provider implicit if not set explicit - if chpass_provider is not given in the configuration file but an auth_provider and the auth_provider can also handle change password requests it is used as chpass_provider. --- server/man/sssd.conf.5.xml | 7 +++ server/providers/data_provider_be.c | 79 ++++++++++++++++++++++++++--------- server/providers/dp_backend.h | 1 + 3 files changed, 67 insertions(+), 20 deletions(-) diff --git a/server/man/sssd.conf.5.xml b/server/man/sssd.conf.5.xml index 3eab235..7af2292 100644 --- a/server/man/sssd.conf.5.xml +++ b/server/man/sssd.conf.5.xml @@ -499,6 +499,13 @@ <quote>proxy</quote> for relaying password changes to some other PAM target. </para> + <para> + <quote>none</quote> disallows password changes explicitly. + </para> + <para> + Default: <quote>auth_provider</quote> is used if it + is set and can handle change password request. + </para> </listitem> </varlistentry> </variablelist> diff --git a/server/providers/data_provider_be.c b/server/providers/data_provider_be.c index 86218d6..f7830c9 100644 --- a/server/providers/data_provider_be.c +++ b/server/providers/data_provider_be.c @@ -45,6 +45,9 @@ #define MSG_TARGET_NO_CONFIGURED "sssd_be: The requested target is not configured" +#define ACCESS_PERMIT "permit" +#define NO_PROVIDER "none" + struct sbus_method monitor_be_methods[] = { { MON_CLI_METHOD_PING, monitor_common_pong }, { MON_CLI_METHOD_RES_INIT, monitor_common_res_init }, @@ -798,8 +801,8 @@ static struct bet_ops be_target_access_permit_ops = { static int load_backend_module(struct be_ctx *ctx, enum bet_type bet_type, - struct bet_ops **be_ops, - void **be_pvt_data) + struct bet_info *bet_info, + const char *default_mod_name) { TALLOC_CTX *tmp_ctx; int ret = EINVAL; @@ -811,6 +814,10 @@ static int load_backend_module(struct be_ctx *ctx, char *mod_init_fn_name = NULL; bet_init_fn_t mod_init_fn = NULL; + (*bet_info).mod_name = NULL; + (*bet_info).bet_ops = NULL; + (*bet_info).pvt_bet_data = NULL; + if (bet_type <= BET_NULL || bet_type >= BET_MAX || bet_type != bet_data[bet_type].bet_type) { DEBUG(2, ("invalid bet_type or bet_data corrupted.\n")); @@ -831,10 +838,30 @@ static int load_backend_module(struct be_ctx *ctx, goto done; } if (!mod_name) { + if (default_mod_name != NULL) { + DEBUG(5, ("no module name found in confdb, using [%s].\n", + default_mod_name)); + mod_name = talloc_strdup(ctx, default_mod_name); + } else { + ret = ENOENT; + goto done; + } + } + + if (strcasecmp(mod_name, NO_PROVIDER) == 0) { ret = ENOENT; goto done; } + if (strcmp(mod_name, ACCESS_PERMIT) == 0) { + (*bet_info).bet_ops = &be_target_access_permit_ops; + (*bet_info).pvt_bet_data = NULL; + (*bet_info).mod_name = talloc_strdup(ctx, ACCESS_PERMIT); + + ret = EOK; + goto done; + } + mod_init_fn_name = talloc_asprintf(tmp_ctx, bet_data[bet_type].mod_init_fn_name_fmt, mod_name); @@ -886,17 +913,27 @@ static int load_backend_module(struct be_ctx *ctx, if (mod_init_fn == NULL) { DEBUG(0, ("Unable to load init fn %s from module %s, error: %s\n", mod_init_fn_name, mod_name, dlerror())); - ret = ELIBBAD; + if (default_mod_name != NULL && + strcmp(default_mod_name, mod_name) == 0 ) { + /* If the default is used and fails we indicate this to the caller + * by returning ENOENT. Ths way the caller can decide how to + * handle the different types of error conditions. */ + ret = ENOENT; + } else { + ret = ELIBBAD; + } goto done; } - ret = mod_init_fn(ctx, be_ops, be_pvt_data); + ret = mod_init_fn(ctx, &(*bet_info).bet_ops, &(*bet_info).pvt_bet_data); if (ret != EOK) { DEBUG(0, ("Error (%d) in module (%s) initialization (%s)!\n", ret, mod_name, mod_init_fn_name)); goto done; } + (*bet_info).mod_name = talloc_strdup(ctx, mod_name); + ret = EOK; done: @@ -951,16 +988,16 @@ int be_process_init(TALLOC_CTX *mem_ctx, } ret = load_backend_module(ctx, BET_ID, - &ctx->bet_info[BET_ID].bet_ops, - &ctx->bet_info[BET_ID].pvt_bet_data); + &ctx->bet_info[BET_ID], NULL); if (ret != EOK) { DEBUG(0, ("fatal error initializing data providers\n")); return ret; } + DEBUG(9, ("ID backend target successfully loaded from provider [%s].\n", + ctx->bet_info[BET_ID].mod_name)); ret = load_backend_module(ctx, BET_AUTH, - &ctx->bet_info[BET_AUTH].bet_ops, - &ctx->bet_info[BET_AUTH].pvt_bet_data); + &ctx->bet_info[BET_AUTH], NULL); if (ret != EOK) { if (ret != ENOENT) { DEBUG(0, ("fatal error initializing data providers\n")); @@ -968,25 +1005,24 @@ int be_process_init(TALLOC_CTX *mem_ctx, } DEBUG(1, ("No authentication module provided for [%s] !!\n", be_domain)); + } else { + DEBUG(9, ("AUTH backend target successfully loaded " + "from provider [%s].\n", ctx->bet_info[BET_AUTH].mod_name)); } ret = load_backend_module(ctx, BET_ACCESS, - &ctx->bet_info[BET_ACCESS].bet_ops, - &ctx->bet_info[BET_ACCESS].pvt_bet_data); + &ctx->bet_info[BET_ACCESS], ACCESS_PERMIT); if (ret != EOK) { - if (ret != ENOENT) { - DEBUG(0, ("fatal error initializing data providers\n")); - return ret; - } - DEBUG(1, ("No access control module provided for [%s] " - "using be_target_access_permit!!\n", be_domain)); - ctx->bet_info[BET_ACCESS].bet_ops = &be_target_access_permit_ops; - ctx->bet_info[BET_ACCESS].pvt_bet_data = NULL; + DEBUG(0, ("No ACCESS backend target available.\n")); + return ret; + } else { + DEBUG(9, ("ACCESS backend target successfully loaded " + "from provider [%s].\n", ctx->bet_info[BET_ACCESS].mod_name)); } ret = load_backend_module(ctx, BET_CHPASS, - &ctx->bet_info[BET_CHPASS].bet_ops, - &ctx->bet_info[BET_CHPASS].pvt_bet_data); + &ctx->bet_info[BET_CHPASS], + ctx->bet_info[BET_AUTH].mod_name); if (ret != EOK) { if (ret != ENOENT) { DEBUG(0, ("fatal error initializing data providers\n")); @@ -994,6 +1030,9 @@ int be_process_init(TALLOC_CTX *mem_ctx, } DEBUG(1, ("No change password module provided for [%s] !!\n", be_domain)); + } else { + DEBUG(9, ("CHPASS backend target successfully loaded " + "from provider [%s].\n", ctx->bet_info[BET_CHPASS].mod_name)); } return EOK; diff --git a/server/providers/dp_backend.h b/server/providers/dp_backend.h index bee1a85..1a8c6c4 100644 --- a/server/providers/dp_backend.h +++ b/server/providers/dp_backend.h @@ -58,6 +58,7 @@ struct bet_info { enum bet_type bet_type; struct bet_ops *bet_ops; void *pvt_bet_data; + char *mod_name; }; struct be_offline_status { -- 1.6.2.5
>From 55ce9a58358d743917acb71c8cab7342878df031 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Thu, 15 Oct 2009 12:03:31 +0200 Subject: [PATCH 2/2] more implicit provider target settings If auth_provider or access_provider is ont set explicitly id_provider is used if it can handle auth or access control requests respectively. If not auth defaults to 'none' and the access_provider is set to 'permit'. The option 'deny' is added for the access_provider to explicitly deny access. --- server/man/sssd.conf.5.xml | 29 ++++++++++++++++- server/providers/data_provider_be.c | 61 +++++++++++++++++++++++++++------- 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/server/man/sssd.conf.5.xml b/server/man/sssd.conf.5.xml index 7af2292..4b8a92f 100644 --- a/server/man/sssd.conf.5.xml +++ b/server/man/sssd.conf.5.xml @@ -469,6 +469,33 @@ <para> <quote>proxy</quote> for relaying authentication to some other PAM target. </para> + <para> + <quote>none</quote> disables authentication explicitly. + </para> + <para> + Default: <quote>id_provider</quote> is used if it + is set and can handle authentication requests. + </para> + </listitem> + </varlistentry> + <varlistentry> + <term>access_provider (string)</term> + <listitem> + <para> + The access control provider used for the domain. + Supported access providers are: + </para> + <para> + <quote>permit</quote> always allow access. + </para> + <para> + <quote>deny</quote> always deny access. + </para> + <para> + Default: <quote>id_provider</quote> is used if it + is set and can handle access control requests or + <quote>permit</quote> otherwise. + </para> </listitem> </varlistentry> <varlistentry> @@ -504,7 +531,7 @@ </para> <para> Default: <quote>auth_provider</quote> is used if it - is set and can handle change password request. + is set and can handle change password requests. </para> </listitem> </varlistentry> diff --git a/server/providers/data_provider_be.c b/server/providers/data_provider_be.c index f7830c9..a5f1b64 100644 --- a/server/providers/data_provider_be.c +++ b/server/providers/data_provider_be.c @@ -46,6 +46,7 @@ #define MSG_TARGET_NO_CONFIGURED "sssd_be: The requested target is not configured" #define ACCESS_PERMIT "permit" +#define ACCESS_DENY "deny" #define NO_PROVIDER "none" struct sbus_method monitor_be_methods[] = { @@ -799,6 +800,21 @@ static struct bet_ops be_target_access_permit_ops = { .finalize = NULL }; +static void be_target_access_deny(struct be_req *be_req) +{ + struct pam_data *pd = talloc_get_type(be_req->req_data, struct pam_data); + DEBUG(9, ("be_target_access_deny called, returning PAM_PERM_DENIED.\n")); + + pd->pam_status = PAM_PERM_DENIED; + be_req->fn(be_req, DP_ERR_OK, PAM_PERM_DENIED, NULL); +} + +static struct bet_ops be_target_access_deny_ops = { + .check_online = NULL, + .handler = be_target_access_deny, + .finalize = NULL +}; + static int load_backend_module(struct be_ctx *ctx, enum bet_type bet_type, struct bet_info *bet_info, @@ -853,13 +869,23 @@ static int load_backend_module(struct be_ctx *ctx, goto done; } - if (strcmp(mod_name, ACCESS_PERMIT) == 0) { - (*bet_info).bet_ops = &be_target_access_permit_ops; - (*bet_info).pvt_bet_data = NULL; - (*bet_info).mod_name = talloc_strdup(ctx, ACCESS_PERMIT); + if (bet_type == BET_ACCESS) { + if (strcmp(mod_name, ACCESS_PERMIT) == 0) { + (*bet_info).bet_ops = &be_target_access_permit_ops; + (*bet_info).pvt_bet_data = NULL; + (*bet_info).mod_name = talloc_strdup(ctx, ACCESS_PERMIT); - ret = EOK; - goto done; + ret = EOK; + goto done; + } + if (strcmp(mod_name, ACCESS_DENY) == 0) { + (*bet_info).bet_ops = &be_target_access_deny_ops; + (*bet_info).pvt_bet_data = NULL; + (*bet_info).mod_name = talloc_strdup(ctx, ACCESS_DENY); + + ret = EOK; + goto done; + } } mod_init_fn_name = talloc_asprintf(tmp_ctx, @@ -997,7 +1023,8 @@ int be_process_init(TALLOC_CTX *mem_ctx, ctx->bet_info[BET_ID].mod_name)); ret = load_backend_module(ctx, BET_AUTH, - &ctx->bet_info[BET_AUTH], NULL); + &ctx->bet_info[BET_AUTH], + ctx->bet_info[BET_ID].mod_name); if (ret != EOK) { if (ret != ENOENT) { DEBUG(0, ("fatal error initializing data providers\n")); @@ -1011,14 +1038,22 @@ int be_process_init(TALLOC_CTX *mem_ctx, } ret = load_backend_module(ctx, BET_ACCESS, - &ctx->bet_info[BET_ACCESS], ACCESS_PERMIT); + &ctx->bet_info[BET_ACCESS], + ctx->bet_info[BET_ID].mod_name); if (ret != EOK) { - DEBUG(0, ("No ACCESS backend target available.\n")); - return ret; - } else { - DEBUG(9, ("ACCESS backend target successfully loaded " - "from provider [%s].\n", ctx->bet_info[BET_ACCESS].mod_name)); + if (ret != ENOENT) { + DEBUG(0, ("No ACCESS backend target available.\n")); + return ret; + } + ret = load_backend_module(ctx, BET_ACCESS, + &ctx->bet_info[BET_ACCESS], ACCESS_PERMIT); + if (ret != EOK) { + DEBUG(0, ("Failed to set ACCESS backend to default (permit).\n")); + return ret; + } } + DEBUG(9, ("ACCESS backend target successfully loaded " + "from provider [%s].\n", ctx->bet_info[BET_ACCESS].mod_name)); ret = load_backend_module(ctx, BET_CHPASS, &ctx->bet_info[BET_CHPASS], -- 1.6.2.5
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel