On (11/05/16 17:35), Lukas Slebodnik wrote:
>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
And there seems to be an AVC as well :-)
time->Thu May 12 14:36:37 2016
type=SYSCALL msg=audit(1463078197.932:202): arch=c000003e syscall=2 success=no
exit=-13 a0=7f34ad1a831d a1=200c1 a2=180 a3=30733a745f666e6f items=0 ppid=1
pid=25124 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0
fsgid=0 tty=(none) ses=4294967295 comm="sssd" exe="/usr/sbin/sssd"
subj=system_u:system_r:sssd_t:s0 key=(null)
type=AVC msg=audit(1463078197.932:202): avc: denied { write } for pid=25124
comm="sssd" name="sssd" dev="dm-0" ino=69228705
scontext=system_u:system_r:sssd_t:s0 tcontext=system_u:object_r:sssd_conf_t:s0
tclass=dir
LS
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
