<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <div class="moz-cite-prefix">On 10/17/2014 06:47 PM, Jakub Hrozek
      wrote:<br>
    </div>
    <blockquote cite="mid:20141017164728.GH3803@hendrix.redhat.com"
      type="cite">
      <pre wrap="">On Thu, Oct 16, 2014 at 10:25:12AM +0200, Jakub Hrozek wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="">On Wed, Oct 15, 2014 at 06:19:49PM -0400, Simo Sorce wrote:
</pre>
        <blockquote type="cite">
          <pre wrap="">On Wed, 15 Oct 2014 22:24:04 +0200
Jakub Hrozek <a class="moz-txt-link-rfc2396E" href="mailto:jhrozek@redhat.com">&lt;jhrozek@redhat.com&gt;</a> wrote:

</pre>
          <blockquote type="cite">
            <pre wrap="">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.
</pre>
          </blockquote>
          <pre wrap="">
Aside from the points raised in the emails already sent the rest looks
good to me.
</pre>
        </blockquote>
        <pre wrap="">
Thank you very much for the review. I'll send out updated patches.
</pre>
      </blockquote>
      <pre wrap="">
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.
</pre>
      <br>
    </blockquote>
    Sorry, but I can't apply these patches on top of the master. <br>
    I think you have to move the first patch.<br>
    <br>
    git am
    patch/0001-UTIL-Add-a-function-to-convert-id_t-from-a-number-or.patch<br>
    <br>
    Applying: UTIL: Add a function to convert id_t from a number or a
    name<br>
    error: patch failed: src/tests/cwrap/Makefile.am:45<br>
    error: src/tests/cwrap/Makefile.am: patch does not apply<br>
    Patch failed at 0001 UTIL: Add a function to convert id_t from a
    number or a name<br>
    <br>
    <blockquote cite="mid:20141017164728.GH3803@hendrix.redhat.com"
      type="cite">
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <br>
      <pre wrap="">_______________________________________________
sssd-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a>
<a class="moz-txt-link-freetext" href="https://lists.fedorahosted.org/mailman/listinfo/sssd-devel">https://lists.fedorahosted.org/mailman/listinfo/sssd-devel</a>
</pre>
    </blockquote>
    <br>
  </body>
</html>