[SSSD] Unexpected behavior with 'simple_allow_users ='

Simo Sorce simo at redhat.com
Thu Nov 1 19:04:11 UTC 2012


On Thu, 2012-11-01 at 10:53 -0400, Dmitri Pal wrote:
> On 11/01/2012 09:11 AM, Simo Sorce wrote:
> > On Thu, 2012-11-01 at 12:03 +0100, Michal Židek wrote:
> >> On 10/30/2012 04:53 PM, Ondrej Kos wrote:
> >>> On 10/17/2012 03:28 PM, Michal Židek wrote:
> >>>> On 10/16/2012 03:45 PM, Stef Walter wrote:
> >>>>> On 10/16/2012 02:04 PM, Jakub Hrozek wrote:
> >>>>>> I was wondering for a while whether to change the behaviour directly in
> >>>>>> confdb_get_string_as_list() but I think the attached patch takes a
> >>>>>> better
> >>>>>> approach because the other consumers of confdb_get_string_as_list() do
> >>>>>> not see any difference between empty and missing parameter.
> >>>>> Yeah figures.
> >>>>>
> >>>>>> The patch works as advertized, there is just one compilation warning:
> >>>>>>
> >>>>>> src/providers/simple/simple_access.c: In function
> >>>>>> 'get_conf_list_or_empty':
> >>>>>> src/providers/simple/simple_access.c:284:9: warning: unused variable
> >>>>>> 'r'
> >>>>>> [-Wunused-variable]
> >>>>> New patch attached.
> >>>>>
> >>>>> Thanks for the review.
> >>>>>
> >>>>> Stef
> >>>>>
> >>>> Hi Stef,
> >>>>
> >>>> your patch solves the problem with *empty* 'simple_allow_users = ', but
> >>>> it introduces new problem with *nonexistent* simple_allow_users. With
> >>>> your patch, if no simple_allow_users list was specified, it behaves as
> >>>> if empty simple_allow_users was provided and denies access to all users.
> >>>>
> >>>> I think this new behaviour could brake existing configurations and it
> >>>> also differs from the behaviour described in man pages.
> >>>>
> >>>> Michal
> >>>>
> >>>> _______________________________________________
> >>>> sssd-devel mailing list
> >>>> sssd-devel at lists.fedorahosted.org
> >>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >>> I fixed the new problem and prepared patch that applies on top of this one.
> >>> attached
> >>>
> >>> O.
> >>>
> >> It is still not working correctly. With your patch *empty* 
> >> simple_allow_users list allows access to everyone (like it should be 
> >> with *nonexistent* list).
> >>
> >> The behaviour should be like this:
> >> simple_allow_users = user1, user2 // allow access to user1 and user2
> >> simple_allow_users =              // do not allow access to anyone
> >> /* nothing */                     // allow access to everyone
> >>
> >>
> >> I think that the problem is that our .conf file parser ignores empty 
> >> values. Do we have ticket to track this issue?
> > Nope, and it is not that easy to change IIRC.
> >
> > I think for now it would be easier to use a user name of 'none' to
> > indicate none has access. The only issue would be if a user 'none'
> > exists.
> >
> > Simo.
> >
> Is this hard because config DB pre-populates all the default values in
> the DB and then overwrites it from INI file?
> Is this the case?

It's because the DB cannot hold an empty value.
With the current interface empty value = No value.

> It seems that it should be easy to differentiate whether something is
> defined and empty value.

We might be able to do it, maybe we should have a ticket.

> I do not think it is an INI problem. Seems like something is not done
> correctly in the config DB layer.
> IMO we should have a ticket to improve that if this is really a problem.

It is a confdb limitation, but it has never been a problem before, we
never had to make a difference, and in general I do not like much to use
these semantics, I feel confused about this syntax if I put on my admin
hat.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list