On 10/17/2014 06:47 PM, Jakub Hrozek 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@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.

Sorry, but I can't apply these patches on top of the master.
I think you have to move the first patch.

git am patch/0001-UTIL-Add-a-function-to-convert-id_t-from-a-number-or.patch

Applying: UTIL: Add a function to convert id_t from a number or a name
error: patch failed: src/tests/cwrap/Makefile.am:45
error: src/tests/cwrap/Makefile.am: patch does not apply
Patch failed at 0001 UTIL: Add a function to convert id_t from a number or a name


_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel