On Wed, Aug 31, 2011 at 12:04:37PM -0400, Stephen Gallagher wrote:
On Wed, 2011-08-31 at 12:47 +0200, Sumit Bose wrote:
> On Tue, Aug 30, 2011 at 04:07:22PM -0400, Stephen Gallagher wrote:
> > On Tue, 2011-08-30 at 13:10 -0400, Stephen Gallagher wrote:
> > > On Tue, 2011-08-30 at 11:23 -0400, Stephen Gallagher wrote:
> > > > On Tue, 2011-08-30 at 10:52 -0400, Stephen Gallagher wrote:
> > > > > Adds a configure option to set the distribution default as well
as
> > > > > an sssd.conf option to override it.
> > > > >
> > > > > Resolves:
https://fedorahosted.org/sssd/ticket/980
> > > >
> > > > Sumit pointed out on IRC that I forgot to include the option in the
> > > > SSSDConfig API. New patch fixes that.
> > >
> > > Sumit also discovered that my configure script did not properly handle
> > > the default case (where --with-krb5-rcache-dir was not specified).
> > >
> > > Fixed in the attached patch.
> >
> > New patch guarantees the existence of the rcache directory.
> >
>
> The patch is working as expected, but I'm sorry but I think I have
> changed my mind about the default for KRB5RCACHEDIR. From a Fedora/Red
> Hat perspective using /var/cache/krb5rcache to solve the SELinux issue
> mentioned in #980 and the related bugzilla entry make sense. But while
> testing the patch I realized that currently it is not possible to not
> set KRB5RCACHEDIR to a value. So it is not possible to just use the
> libkrb5 defaults.
>
> I would like to suggest to change the default to not set KRB5RCACHEDIR
> and use "--with-krb5-rcache-dir=/var/cache/krb5rcache" in Fedora and
> RHEL spec files. This would allow other users and distributions to use
> the libkrb5 defaults.
>
> Additionally I wasn't able to overwrite the path given by the configure
> option with an empty path with krb5_rcache_dir = "" or similar. Maybe we
> need a special keyword here to allow unsetting KRB5RCACHEDIR via
> sssd.conf?
>
> Finally a higher level log message if the rcache directory does not
> exists might be useful even if it is in the log of the monitor.
New patch attached makes the following changes:
The special option __LIBKRB5_DEFAULTS__ has been added which will not
set the environment variable. I opted not to have it explicitly unset
the variable, since proper default behavior would be to honor whatever
was in the environment at startup.
I have made the default configure option be __LIBKRB5_DEFAULTS__ but
have changed the example spec file to specify
%{_localstatedir}/cache/krb5rcache
I have updated the manpage to mention __LIBKRB5_DEFAULTS__ and the new
default behavior.
Works great. I found two minor issues:
diff --git a/Makefile.am b/Makefile.am
index 4e2e26bae9ba9b388998d3c1af84f8a7aa2e51ed..1d381612eb1c43c4494331cb205d0d88a5f3a477
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -34,6 +34,7 @@ systemdunitdir = @systemdunitdir@
logpath = @logpath@
pubconfpath = @pubconfpath@
pkgconfigdir = $(libdir)/pkgconfig
+krb5rcachedir = @krb5rcachedir@
AM_CFLAGS =
if WANT_AUX_INFO
@@ -1141,6 +1142,7 @@ install-data-hook:
rm $(DESTDIR)/$(nsslibdir)/libnss_sss.so.2 \
$(DESTDIR)/$(nsslibdir)/libnss_sss.so
mv $(DESTDIR)/$(nsslibdir)/libnss_sss.so.2.0.0 $(DESTDIR)/$(nsslibdir)/libnss_sss.so.2
+ mkdir -p $(krb5rcachedir)
$(DESTDIR)/ is missing and mkdir shouldn't be called if $(krb5rcachedir)
is __LIBKRB5_DEFAULTS__
uninstall-hook:
if [ -f $(abs_builddir)/src/config/.files ]; then \
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 5e50991f3014019b0c8fd065f4728827d959f06b..e982d17983eb68b3cde4d58b4289a447c723f90e
100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -63,6 +63,7 @@
#define CONFDB_MONITOR_NAME_REGEX "re_expression"
#define CONFDB_MONITOR_FULL_NAME_FORMAT "full_name_format"
#define CONFDB_MONITOR_TRY_INOTIFY "try_inotify"
+#define CONFDB_MONITOR_KRB5_RCACHEDIR "krb5_rcachedir"
_ missing
/* NSS */
#define CONFDB_NSS_CONF_ENTRY "config/nss"
bye,
Sumit