On Mon, Nov 24, 2014 at 02:14:27PM +0100, Lukas Slebodnik wrote:
> On (24/11/14 10:47), Jakub Hrozek wrote:
> >On Mon, Nov 24, 2014 at 09:51:58AM +0100, Lukas Slebodnik wrote:
> >> On (22/11/14 17:25), Jakub Hrozek wrote:
> >> >On Sat, Nov 22, 2014 at 05:12:09PM +0100, Jakub Hrozek wrote:
> >> >> On Tue, Nov 18, 2014 at 12:25:40PM +0100, Lukas Slebodnik wrote:
> >> >> > On (18/11/14 12:22), Lukas Slebodnik wrote:
> >> >> > >On (13/11/14 10:08), Simo Sorce wrote:
> >> >> > >>On Thu, 13 Nov 2014 10:44:45 +0100
> >> >> > >>Jakub Hrozek <jhrozek(a)redhat.com> wrote:
> >> >> > >>
> >> >> > >>> On Wed, Nov 12, 2014 at 08:04:46PM -0500, Simo
Sorce wrote:
> >> >> > >>> > On Wed, 12 Nov 2014 16:36:00 +0100
> >> >> > >>> > Lukas Slebodnik <lslebodn(a)redhat.com>
wrote:
> >> >> > >>> >
> >> >> > >>> > > On (12/11/14 10:00), Simo Sorce wrote:
> >> >> > >>> > > >I would create a helper function to
be called on return that
> >> >> > >>> > > >transforms the error accordingly.
This will allow to write the
> >> >> > >>> > > >code _and_ the comment once.
> >> >> > >>> > > >
> >> >> > >>> > > In this case, Stephan's patch is
better
> >> >> > >>> > >
https://bugzilla.redhat.com/attachment.cgi?id=788567
> >> >> > >>> >
> >> >> > >>> > Yes, this is a valid alternative.
> >> >> > >>> >
> >> >> > >>> > > >The comment should be changed to
something like this in either
> >> >> > >>> > > >case: /* When sssd is stopped
return a safe error code as if sss
> >> >> > >>> > > >was not configured at all in
nsswitch. This prevents bogus
> >> >> > >>> > > >errors from causing issues in
applications, before sssd starts
> >> >> > >>> > > >or if it fails to respond. */
> >> >> > >>> > > >
> >> >> > >>> > > >No need to mention that sss is by
default in nsswitch, as it is
> >> >> > >>> > > >not in all distributions and it
really is inconsequential, the
> >> >> > >>> > > >same behaviour change hleps when
sss is not the default but is
> >> >> > >>> > > >has been manually added and sssd is
stopped or not started yet
> >> >> > >>> > > >(for example during boot).
> >> >> > >>> > > nss-pam-ldapd has the same behaviour in
the same situation.
> >> >> > >>> > > Will we patch it as well? It's very
likely we won't.
> >> >> > >>> >
> >> >> > >>> > Sorry, I do not see how that matters :)
> >> >> > >>> >
> >> >> > >>> > > The biggest problem is that sss is by
default in nsswitch on
> >> >> > >>> > > fedora/rhel>=7 due to glibc caching
and problem with GNOME,
> >> >> > >>> > > a) sssd-client is installed by default
on this platforms.
> >> >> > >>> > > b) sssd need't be configured by
default and in most cases won't be
> >> >> > >>> > > => sssd cannot run
> >> >> > >>> > > c) glibc developers don't want to
adjust the error return code in
> >> >> > >>> > > glibc
> >> >> > >>> > >
> >> >> > >>> > > As a result of this, we need to patch
sssd.
> >> >> > >>> > > I would say we should patch sssd just
in downstream and
> >> >> > >>> > > Stephan's patch works well. I
tested it.
> >> >> > >>> >
> >> >> > >>> > Ok, then let's go with Stephen's
patch.
> >> >> > >>> >
> >> >> > >>> > Simo.
> >> >> > >>> >
> >> >> > >>>
> >> >> > >>> I don't personally mind whether we use
Lukas' or Stephen's patch but I
> >> >> > >>> would /strongly/ prefer the patch to be pushed
upstream.
> >> >> > >>>
> >> >> > >>> Non-upstream patches should be considered valid
only as a temporary
> >> >> > >>> workarounds..
> >> >> > >>
> >> >> > >>I was not aware the patch was not proposed for
upstream. I strongly
> >> >> > >>disagree on this kind of patches being platform
specific too.
> >> >> > >>
> >> >> > >New version of patch is attached.
> >> >> > >
> >> >> > Ups, one more time.
> >> >> >
> >> >> > LS
> >> >>
> >> >> > From eb617a980d50c7081ecb6eda0d489fc0eb8963f7 Mon Sep 17
00:00:00 2001
> >> >> > From: Lukas Slebodnik <lslebodn(a)redhat.com>
> >> >> > Date: Tue, 18 Nov 2014 12:02:10 +0100
> >> >> > Subject: [PATCH] sss_client: Work around glibc bug
> >> >> >
> >> >> > glibc is inconsistent with how it treats and returns
NSS_STATUS_UNAVAIL.
> >> >> >
> >> >> > The sss nss plugin is present in nsswitch by default on some
platforms
> >> >> > due to glibc caching and problem with long living applications
(e.g. GNOME).
> >> >> > The SSSD nss plugin should behave as if it was functioning but
had no data even
> >> >> > thought sssd is not running. The errors have to be passed from
nss plugin up to
> >> >> > the user with minimal moidiffication.
> >> >> >
> >> >> > Thanks to Stephen Gallagher for initial patch.
> >> >> >
> >> >> > Resolves:
> >> >> >
https://fedorahosted.org/sssd/ticket/2439
> >> >> > ---
> >> >> > src/conf_macros.m4 | 13 +++++++++++++
> >> >> > src/sss_client/common.c | 10 ++++++++++
> >> >> > 2 files changed, 23 insertions(+)
> >> >> >
> >> >> > diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
> >> >> > index
fbee81f56e484b618379f7c987ecee50ae48917e..bb283ef248ee017fd608bd85cc68666db0300fd9 100644
> >> >> > --- a/src/conf_macros.m4
> >> >> > +++ b/src/conf_macros.m4
> >> >> > @@ -708,6 +708,19 @@ AC_ARG_ENABLE([dbus-tests],
> >> >> > [build_dbus_tests=yes])
> >> >> > AM_CONDITIONAL([BUILD_DBUS_TESTS], [test x$build_dbus_tests =
xyes])
> >> >> >
> >> >> > +AC_ARG_ENABLE([sss-default-nss-plugin],
> >> >> > +
[AS_HELP_STRING([--enable-sss-default-nss-plugin],
> >> >> > + [This option change standard
behaviour of sss nss
> >> >> > + plugin. If this option is
enabled the sss nss
> >> >> > + plugin will behave as it was
not in
> >> >> > + nsswitch.conf when sssd is not
running.
> >> >> > + [default=no]])],
> >> >> > + [],
> >> >>
> >> >> Didn't you mean to add:
> >> >> enable_sss-default-nss-plugin=$enablevar
> >> >>
> >> >> here?
> >> >>
> >> >> Additionally, it would be nice to explain in the commit message why
the
> >> >> feature is disabled -- explain the effect enabling by default would
have
> >> >> on searches that don't find anything.
> >> >
> >> >Actually, you also need to convert dashes to underscores in the
variable
> >> >name. As the autoconf docs say:
> >> > The option's argument is available to the shell commands
> >> > action-if-given in the shell variable enableval, which is actually
> >> > just the value of the shell variable named enable_feature, with any
> >> > non-alphanumeric characters in feature changed into ‘_’.
> >> >
> >> >This change did the trick for me and I was able to test the return
value
> >> >in gdb.
> >> In gdb? really
> >> Could you ellaborate?
> >
> >Once I fixed the config option so the build-time switch actually worked:
> >
> I thought you was able to see value of bash variable
> enable_sss-default-nss-plugin :-)
>
> New version is attached.
>
> LS
Thank you, the code looks good. Were you able to reproduce the exact
same error as the user saw, though?
I tested with "files sss ldap" in nsswitch.conf and SSSD was off. Even
without the patch, getent group $ipagroup was able to fall back to
ldap..so I guess I must be missing something.
Current version of sssd nsswitch plugin return error codeas as it is specified
in glibc documentation. ldap nsswitch plugin does the same.
So if sssd(nslcd) is stopped then sssd(ldap) plugin will return
NSS_STATUS_UNAVAIL.
The problem is with some distributions where sss is by default in
/etc/nsswitch.conf and sssd is not configured thus cannot be running.
The purpose of this patch is that sss plugin should behave as it wasn't in
nsswitch.conf.
So you should not test with ldap plugin, because ldap is not by default in
nsswitch.conf
So the test shoudl be:
a)
passwd: files sss
&& sssd service should be stopped
b)
passwd: files
and behaviour should be the same.
The C reprodurer is in this thread, public BZ [1] an private BZ[2]
[1]