[SSSD] Unexpected behavior with 'simple_allow_users ='

Simo Sorce simo at redhat.com
Thu Nov 1 20:51:16 UTC 2012


On Thu, 2012-11-01 at 16:09 -0400, Dmitri Pal wrote:
> On 11/01/2012 03:04 PM, Simo Sorce wrote:
> > 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.
> >
> 
> May be it should be "$none$" to express none to not ever collide with
> allowed names.
> Then an empty value would be treated as not defined i.e. everyone allowed.
> However in this specific case does "none" really makes sense? Can you
> imagine the real world use case that would lead to setting this option
> to "none"? Nobody would be able to log into the system is this a use
> case we need to worry about? 

Probably not, we should just ignore it I guess.

Simo.


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




More information about the sssd-devel mailing list