On Mon, Oct 20, 2014 at 10:44:48AM +0200, Pavel Reichl wrote:
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(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.
>
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
You also need to apply the patches from the thread "[PATCH] SSSD: Add
the options to specify a UID and GID to run as" shall I also include
them here?
btw my whole branch is here (the top patches are not 100% tested yet,
although it works in general..)
https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/log/?h=nonroot