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
---
In src/providers/ipa/ipa_selinux.c, the line 405 is too long:
405 /* Update the SELinux context ...
406 * unprivileged
407 */
---
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).
---
Otherwise the patch looks good to me and the selinux
context is retrieved properly.
Michal