[PATCHES] Support starting SSSD from a default configuration

[Stephen Gallagher]

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.




These patches will require a change to the SELinux policy, since the monitor
needs to be able to write to the /etc/sssd directory.

type=AVC msg=audit(1461088081.353:550): avc:  denied  { write } for  pid=3721
comm="sssd" name="sssd" dev="dm-0" ino=4600013
scontext=system_u:system_r:sssd_t:s0 tcontext=system_u:object_r:sssd_conf_t:s0
tclass=dir permissive=0
	Was caused by:
		Missing type enforcement (TE) allow rule.

		You can use audit2allow to generate a loadable module to allow this access.

[Jakub Hrozek]

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.

[Stephen Gallagher]

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

[Jakub Hrozek]

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)

[Jakub Hrozek]

On Tue, May 10, 2016 at 05:06:41PM +0200, Jakub Hrozek wrote:
>  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) 
master:
    * 760a62e4aea13e3d1a428be35c4ae1547ae63afc
    * 98dbaea0a00c60972b991755a44c51964dfb7877
    * 59744cff6edb106ae799b2321cb8731edadf409a

This is a 'SUCCESS' CI link:
http://sssd-ci.duckdns.org/logs/job/43/17/summary.html

[Lukas Slebodnik]

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 <sgallagh(a)redhat.com&gt;
> 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 <jhrozek(a)redhat.com&gt;
> ---
>  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

[Lukas Slebodnik]

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 <sgallagh(a)redhat.com&gt;
> >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 <jhrozek(a)redhat.com&gt;
> >---
> > 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

[Stephen Gallagher]

On 05/13/2016 10:29 AM, Lukas Slebodnik wrote:
>  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: 
<snip>
> > 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 ...) 

Sorry it took me so long to reply on this. I should have examined this more
closely. It was probably a bad idea to have the proxy provided be the default
setup, since it might or might not be installed (and we probably don't want to
force its inclusion). I guess it would make more sense for the native 'local'
provider to be the default.

As for the AVC, I spotted that in my testing and I think I reported it to the
selinux-policy package, but I'm not certain (I can't find it right now).

[Lukas Slebodnik]

On (03/06/16 09:07), Stephen Gallagher wrote:
> On 05/13/2016 10:29 AM, Lukas Slebodnik wrote:
> > 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:
> <snip>
> >> 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 ...)
>
>
> Sorry it took me so long to reply on this. I should have examined this more
> closely. It was probably a bad idea to have the proxy provided be the default 
It
wasn't a bad idea per se.
I realized this issue later when I ran downstream tests and noticed AVCs.

> setup, since it might or might not be installed (and we probably
> don't want to
> force its inclusion). I guess it would make more sense for the native 'local'
> provider to be the default.
>  
maybe.


> As for the AVC, I spotted that in my testing and I think I reported it
> to the
> selinux-policy package, but I'm not certain (I can't find it right now).
>  
If you cannot find BZ then let us know whether we should file a bug
or you will.

LS

[Jakub Hrozek]

On Fri, Jun 03, 2016 at 05:55:24PM +0200, Lukas Slebodnik wrote:
>  On (03/06/16 09:07), Stephen Gallagher wrote:
>  >On 05/13/2016 10:29 AM, Lukas Slebodnik wrote:
>  >> 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:
>  ><snip>
>  >>> 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 ...)
>  >
>  >
>  >Sorry it took me so long to reply on this. I should have examined this more
>  >closely. It was probably a bad idea to have the proxy provided be the default
>  It wasn't a bad idea per se.
>  I realized this issue later when I ran downstream tests and noticed AVCs. 
Yeah, sorry, I didn't realize this either because I build from source
during my review :-/

>  
>  >setup, since it might or might not be installed (and we probably don't want to
>  >force its inclusion). I guess it would make more sense for the native
> 'local'
>  >provider to be the default.
>  >
>  maybe. 
FWIW, I already started on the 'files' provider and have some WIP
patches in my branches, but I don't think I will finish them before we
release 1.14 (~3 weeks), so I would also vote to include a provider that
already exists for the time being and switch to the files provider in
the next 1.14 release.