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(a)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.
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?
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().
Thanks again for the review.