On Fri, Oct 31, 2014 at 10:48:09PM +0100, Michal Židek wrote:
On 10/31/2014 05:59 PM, Jakub Hrozek wrote:
>On Fri, Oct 24, 2014 at 10:57:02PM +0200, Jakub Hrozek wrote:
>>>Hi,
>>>
>>>this patchset moves setting the SELinux context from the sssd_be process
>>>to a privileges child process in preparation for making the sssd_be process
>>>running as the sssd user.
>>>
>>>The first two patches just reduce code duplication between the child
>>>processes we already have, the last one implements setting the context.
>>>
>>>Please note that the Fedora and RHEL SELinux policy must be tweaked, so
>>>currently the patches must be tested with SELinux set to Permissive.
>>>
>>>Additionally, I wasn't able to run Coverity scan on these patches,
because
>>>our Coverity server was returning errors again for me. I'll run the
tests
>>>later, or, if the reviewer would find any issues, I'll be happy to fix
them.
>Hi,
>
>attached patches change the packaging a bit -- make distcheck didn't
>pass previously and I forgot to package selinux_child in the RPM.
>
>
>0001-UTIL-Remove-code-duplication-of-struct-io.patch
Ack.
>
>0002-UTIL-Remove-more-code-duplication-setting-up-child-p.patch
Ack.
>
>0003-IPA-Move-setting-the-SELinux-context-to-a-child-proc.patch
The chgrp and chmod in Makefile.am should probably be called
only if the semanage is available:
--- a/Makefile.am
+++ b/Makefile.am
@@ -2852,10 +2852,12 @@ endif
if SSSD_USER
chgrp $(SSSD_USER) $(sssdlibexecdir)/ldap_child
- chgrp $(SSSD_USER) $(sssdlibexecdir)/selinux_child
chmod 4750 $(sssdlibexecdir)/ldap_child
+if BUILD_SEMANAGE
+ chgrp $(SSSD_USER) $(sssdlibexecdir)/selinux_child
chmod 4750 $(sssdlibexecdir)/selinux_child
endif
+endif
Fixed
---
In src/providers/ipa/ipa_selinux.c, the line 405 is too long:
405 /* Update the SELinux context ...
406 * unprivileged
407 */
---
Fixed
Also in src/providers/ipa/ipa_selinux.c, the indentation
on line 872 should be aligned with the line above:
871 if (sci->seuser == NULL || sci->mls_range == NULL
872 || sci->username == NULL) {
Should be:
871 if (sci->seuser == NULL || sci->mls_range == NULL
872 || sci->username == NULL) {
(Or at least I am doing it like this, and it is style that
was proposed IIRC by Stephen in some old discussion).
Fixed. I thought the leading tabs were also proposed in that thread..
---
Otherwise the patch looks good to me and the selinux
context is retrieved properly.
Michal
Thank you for the review, new patches are attached.