On (10/05/16 17:06), Jakub Hrozek wrote:
>On Tue, May 10, 2016 at 09:51:18AM -0400, Stephen Gallagher wrote:
>> On 05/10/2016 09:45 AM, Jakub Hrozek wrote:
>> > On Tue, Apr 19, 2016 at 02:09:14PM -0400, Stephen Gallagher wrote:
>> >> These patches provide support for shipping a default configuration file
>> >> that the
>> >> monitor will automatically copy to /etc/sssd/sssd.conf if none already
>> >> exists.
>> >> The idea is for distributions to be able to provide a default (and
>> >> resettable)
>> >> configuration for out-of-the-box behavior.
>> >>
>> >> I considered writing the patch to check /etc/sssd and then check
>> >> /usr/lib*/sssd
>> >> in turn, but I realized that this would be too complicated with the
>> >> infopipe
>> >> interactions (which would need to be updated to do a copy-on-write the
>> >> first
>> >> time they changed something). It was simpler to just always create the
>> >> /etc
>> >> version and use that.
>> >>
>> >>
>> >> Patch 0001: Create a secure copy function that can be used to duplicate
>> >> the
>> >> default configuration
>> >>
>> >> Patch 0002: Cosmetic patch; changes the name of an internal macro
>> >> variable to
>> >> make it clear that it's the active configuration file, not the default
>> >> one.
>> >>
>> >> Patch 0003: Add the logic to confdb_setup.c to copy over the default
>> >> configuration if and only if our attempt to load the configuration came
>> >> up with
>> >> ERR_MISSING_CONF. It will then try to load it again and proceed or fail
>> >> from there.
>> >>
>> >> The default configuration provided here is to load the SSSD with a single
>> >> proxy
>> >> provider that reads from nss_files (and supports authentication through
>> >> pam_unix). This does not have to be shipped with any downstream package;
>> >> the
>> >> idea is that downstreams would be expected to modify this configuration
>> >> to their
>> >> own needs. This would need to be called out in the release announcement
>> >> for
>> >> whatever version of SSSD incorporates this change.
>> >
>> > Wow, it took me long to get back to the review :-(
>> >
>> > I had to slightly fix the unit test otherwise it was failing for me. The
>> > follow up patch is at:
>> > https://github.com/jhrozek/sssd/tree/conf-review
>> > if you agree with squashing the patch into your patchset, I can ACK the
>> > patches.
>> >
>>
>> LGTM
>
>OK, for posterity, attached are the patches (RB: me) that I would like
>to commit.
>
>CI passed as well:
>http://sssd-ci.duckdns.org/logs/job/43/08/summary.html
>(The failure on debian is in dyndns-tests, which is unrelated)
>From 209457ccd2cd90623833b72ba6df15c1d3a42955 Mon Sep 17 00:00:00 2001
>From: Stephen Gallagher <sgall...@redhat.com>
>Date: Tue, 19 Apr 2016 11:58:35 -0400
>Subject: [PATCH 3/3] CONFIG: Use default config when none provided
>
>This patch makes SSSD possibly useful "out of the box" by allowing
>packagers to provide a default config file located in $LIBDIR/sssd/conf
>that will be copied by the monitor to /etc/sssd if no file already
>exists in that location. This will make it possible to have SSSD set up
>to have distribution-specific default configuration, such as enabling
>the proxy provider to cache /etc/passwd (such as in the provided
>example in this patch).
>
>Reviewed-by: Jakub Hrozek <jhro...@redhat.com>
>---
> Makefile.am | 12 +++++++++++-
> contrib/sssd.spec.in | 3 +++
> src/confdb/confdb.h | 1 +
> src/confdb/confdb_setup.c | 40 ++++++++++++++++++++++++++++++++++++----
> src/examples/sssd-shadowutils | 6 ++++++
> src/examples/sssd.conf | 17 +++++++++++++++++
> 6 files changed, 74 insertions(+), 5 deletions(-)
> create mode 100644 src/examples/sssd-shadowutils
> create mode 100644 src/examples/sssd.conf
>
>diff --git a/Makefile.am b/Makefile.am
>index
>7161bef3c9b47db92a390220e3f285c7b5d2d812..d23913b0f667c132d96290495b8d4c9b42aaad27
> 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -33,6 +33,7 @@ endif
>
> sssdlibexecdir = $(libexecdir)/sssd
> sssdlibdir = $(libdir)/sssd
>+sssddefaultconfdir = $(sssdlibdir)/conf
> ldblibdir = @ldblibdir@
> if BUILD_KRB5_LOCATOR_PLUGIN
> krb5plugindir = @krb5pluginpath@
>@@ -77,6 +78,7 @@ pkgconfigdir = $(libdir)/pkgconfig
> krb5rcachedir = @krb5rcachedir@
> sudolibdir = @sudolibpath@
> polkitdir = @polkitdir@
>+pamconfdir = $(sysconfdir)/pam.d
>
> UNICODE_LIBS=@UNICODE_LIBS@
>
>@@ -434,6 +436,7 @@ AM_CPPFLAGS = \
> -DSHLIBEXT=\"$(SHLIBEXT)\" \
> -DSSSD_LIBEXEC_PATH=\"$(sssdlibexecdir)\" \
> -DSSSD_CONF_DIR=\"$(sssdconfdir)\" \
>+ -DSSSD_DEFAULT_CONF_DIR=\"$(sssddefaultconfdir)\" \
> -DSSS_NSS_MCACHE_DIR=\"$(mcpath)\" \
> -DSSS_NSS_SOCKET_NAME=\"$(pipepath)/nss\" \
> -DSSS_PAM_SOCKET_NAME=\"$(pipepath)/pam\" \
>@@ -1104,8 +1107,8 @@ sssd_SOURCES = \
> src/monitor/monitor.c \
> src/monitor/monitor_netlink.c \
> src/confdb/confdb_setup.c \
>- src/util/nscd.c \
> src/monitor/monitor_iface_generated.c \
>+ $(SSSD_TOOLS_OBJ) \
> $(NULL)
> sssd_LDADD = \
> $(SSSD_LIBS) \
>@@ -1268,6 +1271,12 @@ dist_noinst_DATA += \
> src/sss_client/COPYING.LESSER \
> src/m4
>
>+dist_sssddefaultconf_DATA = \
>+ src/examples/sssd.conf
>+
>+dist_pamconf_DATA = \
>+ src/examples/sssd-shadowutils
>+
> ######################
> # Command-line Tools #
> ######################
>@@ -3567,6 +3576,7 @@ SSSD_USER_DIRS = \
> $(DESTDIR)$(pubconfpath)/krb5.include.d \
> $(DESTDIR)$(gpocachepath) \
> $(DESTDIR)$(sssdconfdir) \
>+ $(DESTDIR)$(sssddefaultconfdir) \
> $(DESTDIR)$(logpath) \
> $(NULL)
>
>diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>index
>2ba6a4d4c919a0697b18c4293f5e33e12b996cac..355b9510994b2f5ea470febca670d8982ad4bfce
> 100644
>--- a/contrib/sssd.spec.in
>+++ b/contrib/sssd.spec.in
>@@ -766,6 +766,9 @@ done
> %dir %{_sysconfdir}/rwtab.d
> %config(noreplace) %{_sysconfdir}/rwtab.d/sssd
> %dir %{_datadir}/sssd
>+%{_sysconfdir}/pam.d/sssd-shadowutils
>+%{_libdir}/%{name}/conf/sssd.conf
>+
> %{_datadir}/sssd/sssd.api.conf
> %{_datadir}/sssd/sssd.api.d
> %{_mandir}/man1/sss_ssh_authorizedkeys.1*
>diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
>index
>b90ced2bb3c7ded76950ce2b16586c995cda798d..a9b1c4362b5c0c6b158830b1bf2ef68db09d8d06
> 100644
>--- a/src/confdb/confdb.h
>+++ b/src/confdb/confdb.h
>@@ -40,6 +40,7 @@
>
> #define CONFDB_DEFAULT_CFG_FILE_VER 2
> #define CONFDB_FILE "config.ldb"
>+#define SSSD_DEFAULT_CONFIG_FILE SSSD_DEFAULT_CONF_DIR"/sssd.conf"
> #define SSSD_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf"
> #define SSSD_MIN_ID 1
> #define SSSD_LOCAL_MINID 1000
>diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c
>index
>694a7f0161304f3c7ac94bb9307181f56ca25f05..dfdcae56697123c414968cfaaabe3e1cd68ca21f
> 100644
>--- a/src/confdb/confdb_setup.c
>+++ b/src/confdb/confdb_setup.c
>@@ -21,12 +21,14 @@
>
> #include "config.h"
> #include <sys/stat.h>
>+#include <unistd.h>
> #include "util/util.h"
> #include "db/sysdb.h"
> #include "confdb.h"
> #include "confdb_private.h"
> #include "confdb_setup.h"
> #include "util/sss_ini.h"
>+#include "tools/tools_util.h"
>
>
> int confdb_test(struct confdb_ctx *cdb)
>@@ -159,11 +161,41 @@ int confdb_init_db(const char *config_file, struct
>confdb_ctx *cdb)
> DEBUG(SSSDBG_TRACE_FUNC,
> "sss_ini_config_file_open failed: %s [%d]\n", strerror(ret),
> ret);
>- if (ret == ENOENT) {
>- /* sss specific error denoting missing configuration file */
>- ret = ERR_MISSING_CONF;
>+ if (ret != ENOENT) {
>+ /* Anything other than ENOENT is unrecoverable */
>+ goto done;
>+ } else {
>+ /* Copy the default configuration file to the standard location
>+ * and then retry
>+ */
>+ ret = copy_file_secure(SSSD_DEFAULT_CONFIG_FILE,
>+ SSSD_CONFIG_FILE,
>+ 0600,
>+ getuid(),
>+ getgid(),
>+ false);
>+ if (ret != EOK) {
>+ DEBUG(SSSDBG_FATAL_FAILURE,
>+ "Could not copy default configuration: %s",
>+ sss_strerror(ret));
>+ /* sss specific error denoting missing configuration file */
>+ ret = ERR_MISSING_CONF;
>+ goto done;
>+ }
>+
>+ /* Try again */
>+ ret = sss_ini_config_file_open(init_data, config_file);
>+ if (ret != EOK) {
>+ DEBUG(SSSDBG_TRACE_FUNC,
>+ "sss_ini_config_file_open(default) failed: %s [%d]\n",
>+ strerror(ret), ret);
>+ if (ret == ENOENT) {
>+ /* sss specific error denoting missing configuration file
>*/
>+ ret = ERR_MISSING_CONF;
>+ }
>+ goto done;
>+ }
> }
>- goto done;
> }
>
> ret = sss_ini_config_access_check(init_data);
>diff --git a/src/examples/sssd-shadowutils b/src/examples/sssd-shadowutils
>new file mode 100644
>index
>0000000000000000000000000000000000000000..626c7d075dfbf97dd91e259f94c6061689c83e9e
>--- /dev/null
>+++ b/src/examples/sssd-shadowutils
>@@ -0,0 +1,6 @@
>+#%PAM-1.0
>+auth [success=done ignore=ignore default=die] pam_unix.so nullok
>try_first_pass
>+auth required pam_deny.so
>+
>+account required pam_unix.so
>+account required pam_permit.so
>diff --git a/src/examples/sssd.conf b/src/examples/sssd.conf
>new file mode 100644
>index
>0000000000000000000000000000000000000000..a851dbb7ecd5c3220fbd6a946a6c7be2822dbd27
>--- /dev/null
>+++ b/src/examples/sssd.conf
>@@ -0,0 +1,17 @@
>+[sssd]
>+config_file_version = 2
>+services = nss, pam
>+domains = shadowutils
>+
>+[nss]
>+
>+[pam]
>+
>+[domain/shadowutils]
>+id_provider = proxy
>+proxy_lib_name = files
>+
>+auth_provider = proxy
>+proxy_pam_target = sssd-shadowutils
>+
>+proxy_fast_alias = True
This basic sssd configuration expects that proxy provider will
be installed by default. And that's not reality.
It's only installed with meta-package sssd.
But It will not work with minimal installation of sssd.
"yum install -y sssd-ldap"
or even just "sssd-common"
If should start without failures and with default configuration
then we should change pacakging as well and recommend similar packaging change
to other downstreams (debian, opensuse ...)
LS
BTW There was introduced warning in the function copy_file_contents.
Patch is already on the list
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
