[SSSD] [PATCH] Monitor and sbus changes for running SSSD as a non-privileged user

Simo Sorce ssorce at redhat.com
Fri Oct 17 17:29:42 UTC 2014


On Fri, 17 Oct 2014 18:47:28 +0200
Jakub Hrozek <jhrozek at redhat.com> wrote:

> On Thu, Oct 16, 2014 at 10:25:12AM +0200, Jakub Hrozek wrote:
> > On Wed, Oct 15, 2014 at 06:19:49PM -0400, Simo Sorce wrote:
> > > On Wed, 15 Oct 2014 22:24:04 +0200
> > > Jakub Hrozek <jhrozek at redhat.com> wrote:
> > > 
> > > > Attached are patches that perform changes in the monitor
> > > > process and the low-level sbus and sysdb code required to run
> > > > the NSS responder as a non-privileged user. Some of the patches
> > > > call chmod/chown on files owned by the SSSD, so I'd like to
> > > > request a very careful review.
> > > 
> > > Aside from the points raised in the emails already sent the rest
> > > looks good to me.
> > 
> > Thank you very much for the review. I'll send out updated patches.
> 
> Attached are patches that implement the corrections Simo and Pavel
> asked for with the exception of confdb being read-only for sssd
> processes, Michal is still investigating that one.
> 
> I also merged some more Michal's patches that help spawn the PAM
> privileged pipe before dropping privileges and also dropping
> privileges in the other responders.
> 
> With this patchset, all responders are able to run as the SSSD user.

Great!

> I have one question:
> is it wise to also set the permissions of directories we create when
> we "install -d" them? Or is this something typically done by the
> downstream? Previously, we would just rely on the default system umask
> when running "make install", maybe we should tighten the permissions
> in Makefile.am as part of this effort?

+1

> And one comment:
> One of Michal's patches moves creating the socket into a separate
> function. This function also changes the umask, which I don't think
> belongs into a utility function. I realize this is how the code worked
> even before Michal's change, but I think it would be better to move
> the umask setting into function like sss_process_init().

+1

> Thanks again for the review.


FYI the commit message for patch 2 still mentions the group option you
just removed :-)

Same for patch 4


The rest looks good.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York



More information about the sssd-devel mailing list