On Thu, Oct 16, 2014 at 09:05:24AM -0400, Simo Sorce wrote:
On Thu, 16 Oct 2014 10:23:35 +0200
Jakub Hrozek <jhrozek(a)redhat.com> wrote:
> On Wed, Oct 15, 2014 at 06:17:55PM -0400, Simo Sorce wrote:
> > On Wed, 15 Oct 2014 22:24:04 +0200
> > Jakub Hrozek <jhrozek(a)redhat.com> wrote:
> >
> > > From c0385561ee5e9d050d2222aa43ebf46514f37dad Mon Sep 17 00:00:00
> > > 2001 From: Michal Zidek <mzidek(a)redhat.com>
> > > Date: Thu, 9 Oct 2014 17:15:56 +0200
> > > Subject: [PATCH 5/7] MONITOR: Allow confdb to be accessed by
> > > nonroot user
> > >
> > > ---
> > > src/monitor/monitor.c | 11 ++++++++++-
> > > 1 file changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
> > > index
> > >
44614be173325aa5b6f7ed03f00b6d4489ddf522..bd2c373008ef75ab46cf7dccdefd12468894f1ba
> > > 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c
> > > @@ -1718,7 +1718,6 @@ static errno_t load_configuration(TALLOC_CTX
> > > *mem_ctx, DEBUG(SSSDBG_FATAL_FAILURE, "Fatal error initializing
> > > confdb\n"); goto done;
> > > }
> > > - talloc_zfree(cdb_file);
> > >
> > > ret = confdb_init_db(config_file, ctx->cdb);
> > > if (ret != EOK) {
> > > @@ -1734,6 +1733,16 @@ static errno_t
> > > load_configuration(TALLOC_CTX *mem_ctx, goto done;
> > > }
> > >
> > > + /* Allow configuration database to be accessible
> > > + * when SSSD runs as nonroot */
> > > + ret = chown(cdb_file, ctx->uid, ctx->gid);
> > > + if (ret != EOK) {
> > > + DEBUG(SSSDBG_FATAL_FAILURE,
> > > + "chown failed for [%s]: [%d][%s].\n",
> > > + cdb_file, ret, sss_strerror(ret));
> > > + goto done;
> > > + }
> > > +
> > > *monitor = ctx;
> > >
> > > ret = EOK;
> >
> >
> > I wonder if we shouldn't be more cautious here.
> > Do we need to give the sssd user write access ?
> > I think probably not, sounds like a great way to prevent
> > "accidental" changes would be to chown to (0, gid) and chmod so
> > that the group can only read, while root can read/write.
> >
> > This way non-root process will be allowed to read but not change the
> > database.
>
> But the sssd_be processes should also run as non-root if possible and
> it needs to write to the db.
To the configuration db ?
Why ?
Aah, this is confdb.
Sorry, I was thinking sysdb. Then you are completely right.
> In general, the current plan is that sssd_be would start as root, do
> whatever privileged initialization it needs and drop to sssd user,
> too. Then, any operations that need to be done with different
> privileges will be done either in setuid helper (ldap_child,
> krb5_child with validation, setting the SELinux context) or in a
> helper running as the user on whose behalf we are authenticating
> (krb5_child when keytab validation is not needed).
>
> This way, only the monitor and the proxy_child will run privileged..
Yes, and the children usually do not read the confdb at all as they get
their parameters from the invoking process.
In what case does anything need to write to the confdb after the file
has been generated by the monitor ?
None, I confused sysdb and confdb. Sorry about that.
When on the topic of setuid child processes, we currently pass quite a
lot of info through environment variables. Even though the variables are
private and namespaced (SSS_DEFAULT_REALM for example), I still think
we should get rid of the variables and either use a parameter
(--default-realm) or just pass the info through the pipe..