[SSSD] [PATCH] AD: add config and processing support gpo_map_* options

Jakub Hrozek jhrozek at redhat.com
Fri Sep 5 15:08:03 UTC 2014


On Fri, Sep 05, 2014 at 04:40:05PM +0200, Sumit Bose wrote:
> 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?

Sure, this shouldn't block the patch review!



More information about the sssd-devel mailing list