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 <jhro...@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 <lsleb...@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 <lsleb...@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]])], + [], + [enable_sss-default-nss-plugin=no]) +AS_IF([test x$enable_sss-default-nss-plugin = xyes], + AC_DEFINE_UNQUOTED([NONSTANDARD_SSS_NSS_BEHAVIOUR], [1], + [whether to build sssd nss plugin with nonstandard glibc behaviour])) + AC_DEFUN([WITH_NFS], [ AC_ARG_WITH([nfsv4-idmapd-plugin], [AC_HELP_STRING([--with-nfsv4-idmapd-plugin], diff --git a/src/sss_client/common.c b/src/sss_client/common.c index ebe783aba9cfa3dc11a529e7875966f139eea1af..7c4bb7ab8769a72f943158366f358b108bfc3bdc 100644 --- a/src/sss_client/common.c +++ b/src/sss_client/common.c @@ -724,7 +724,12 @@ enum nss_status sss_nss_make_request(enum sss_cli_command cmd, ret = sss_cli_check_socket(errnop, SSS_NSS_SOCKET_NAME); if (ret != SSS_STATUS_SUCCESS) { +#ifdef NONSTANDARD_SSS_NSS_BEHAVIOUR + errno = 0; + return NSS_STATUS_NOTFOUND; +#else return NSS_STATUS_UNAVAIL; +#endif } ret = sss_cli_make_request_nochecks(cmd, rd, repbuf, replen, errnop); @@ -735,7 +740,12 @@ enum nss_status sss_nss_make_request(enum sss_cli_command cmd, return NSS_STATUS_SUCCESS; case SSS_STATUS_UNAVAIL: default: +#ifdef NONSTANDARD_SSS_NSS_BEHAVIOUR + errno = 0; + return NSS_STATUS_NOTFOUND; +#else return NSS_STATUS_UNAVAIL; +#endif } } -- 2.1.0
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
