On Fri, Sep 05, 2014 at 04:08:56PM +0200, Jakub Hrozek wrote:
On Fri, Sep 05, 2014 at 04:01:06PM +0200, Sumit Bose wrote:
> > +errno_t
> > +ad_gpo_parse_map_options(struct ad_access_ctx *access_ctx)
> > +{
> > + char *gpo_default_right_config;
> > + enum gpo_map_type gpo_default_right;
> > + errno_t ret;
> > + int i;
> > +
> > + for (i = 0; i < GPO_MAP_NUM_OPTS; i++) {
> > +
> > + struct gpo_map_option_entry entry = gpo_map_option_entries[i];
> > +
> > + char *entry_config = dp_opt_get_string(access_ctx->ad_options,
> > + entry.ad_basic_opt);
> > +
> > + ret = ad_gpo_parse_map_option(access_ctx, entry.gpo_map_type,
> > + access_ctx->gpo_map_options_table,
> > + entry_config, entry.gpo_map_defaults);
>
> ret is not checked here. As a result an invalid configuration where a
> service is added to two different maps is not detected. I would suggest
> to terminate SSSD here because the configuration is invalid and it is
> not clear to the admin to which login right the service is added.
Thank you for the review. Do you think we need to terminate the back
end? Could we get away with denying all access so at least identity
provider works?
That sounds like a good idea, but I'm afraid that this might be much
more work then just terminating. Maybe we can just terminate here and
add a ticket to add a more tolerant solution later?
bye,
Sumit
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel