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:
[jhrozek@client] sssd $ [(review)] gdb getent
GNU gdb (GDB) Fedora 7.8-20.fc21
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <
http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<
http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<
http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
<
http://www.gnu.org/software/gdb/documentation/>.
[0/1593]
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from getent...
warning: the debug information found in "/usr/lib/debug//usr/bin/getent.debug"
does not match "/usr/bin/getent" (CRC mismatch).
warning: the debug information found in "/usr/lib/debug/usr/bin/getent.debug"
does not match "/usr/bin/getent" (CRC mismatch).
(no debugging symbols found)...done.
Missing separate debuginfos, use: debuginfo-install glibc-common-2.20-5.fc21.x86_64
(gdb) set args group ipausers
(gdb) b sss_nss_make_request
Function "sss_nss_make_request" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (sss_nss_make_request) pending.
(gdb) r
Starting program: /usr/bin/getent group ipausers
Got object file from memory but can't read symbols: File truncated.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Breakpoint 1, sss_nss_make_request (cmd=SSS_NSS_GETGRNAM, rd=0x7fffffffddf0,
repbuf=0x7fffffffddd0, replen=0x7fffffffddd8, errnop=0x7ffff7fea690)
at src/sss_client/common.c:720
720 envval = getenv("_SSS_LOOPS");
(gdb) n
721 if (envval && strcmp(envval, "NO") == 0) {
(gdb)
725 ret = sss_cli_check_socket(errnop, SSS_NSS_SOCKET_NAME);
(gdb)
726 if (ret != SSS_STATUS_SUCCESS) {
(gdb)
728 errno = 0;
(gdb)
729 return NSS_STATUS_NOTFOUND;
(gdb) list
724
725 ret = sss_cli_check_socket(errnop, SSS_NSS_SOCKET_NAME);
726 if (ret != SSS_STATUS_SUCCESS) {
727 #ifdef NONSTANDARD_SSS_NSS_BEHAVIOUR
728 errno = 0;
729 return NSS_STATUS_NOTFOUND;
730 #else
731 return NSS_STATUS_UNAVAIL;
732 #endif
733 }
(gdb) quit
A debugging session is active.
Inferior 1 [process 28114] will be killed.
Quit anyway? (y or n) y
[jhrozek@client] sssd $ [(review)]