[PATCHES] IPA: Use libsemanage

[Michal Židek]

Hi,

these 3 patches remove the old approach where we directly
wrote the selinux login file. Here, the same approach is used
as in the tools (sss_useradd/sss_usermod) where we already
use libsemanage to manage these tasks.

The advantage is that we remove some lines of code (yay!)
and get the auditing of selinux user changes for free.

1st patch just moves stuff around, so that set_seuser
function can be used from IPA provider.

2nd patch is a very simple change that adds new attribute
to the set_seuser function (mls_range).

The main functionality is in the 3rd patch. Note that to
test the patch, you must run in permissive mode.
$ setenforce Permissive

Otherwise you will get AVC denials (because sssd_be is
not allowed to use libsemanage).

So we will have to change the selinux policy in fedora/rhel
to make it work this way. I  will ping the selinux policy maintainer
after the patches are reviewed.

Patches are attached.

Thanks,
Michal

[Michal Židek]

On 09/24/2014 06:52 PM, Michal Židek wrote:
> Hi,
>
> these 3 patches remove the old approach where we directly
> wrote the selinux login file. Here, the same approach is used
> as in the tools (sss_useradd/sss_usermod) where we already
> use libsemanage to manage these tasks.
>
> The advantage is that we remove some lines of code (yay!)
> and get the auditing of selinux user changes for free.
>
> 1st patch just moves stuff around, so that set_seuser
> function can be used from IPA provider.
>
> 2nd patch is a very simple change that adds new attribute
> to the set_seuser function (mls_range).
>
> The main functionality is in the 3rd patch. Note that to
> test the patch, you must run in permissive mode.
> $ setenforce Permissive
>
> Otherwise you will get AVC denials (because sssd_be is
> not allowed to use libsemanage).
>
> So we will have to change the selinux policy in fedora/rhel
> to make it work this way. I  will ping the selinux policy maintainer
> after the patches are reviewed.
>
> Patches are attached.
>
> Thanks,
> Michal

I forgot to remove one unused variable. New
patches are attached.

Michal

[Jakub Hrozek]

On Wed, Sep 24, 2014 at 06:52:15PM +0200, Michal Židek wrote:
> Hi,
>
> these 3 patches remove the old approach where we directly
> wrote the selinux login file. Here, the same approach is used
> as in the tools (sss_useradd/sss_usermod) where we already
> use libsemanage to manage these tasks.

Thanks a lot for the patches! I'm afraid we won't have the capacity to
review them until Tuesday and the next release, though..
> The advantage is that we remove some lines of code (yay!)
> and get the auditing of selinux user changes for free.
>
> 1st patch just moves stuff around, so that set_seuser
> function can be used from IPA provider.
>
> 2nd patch is a very simple change that adds new attribute
> to the set_seuser function (mls_range).
>
> The main functionality is in the 3rd patch. Note that to
> test the patch, you must run in permissive mode.
> $ setenforce Permissive
>
> Otherwise you will get AVC denials (because sssd_be is
> not allowed to use libsemanage).
>
> So we will have to change the selinux policy in fedora/rhel
> to make it work this way. I  will ping the selinux policy maintainer
> after the patches are reviewed.

Thanks, please do. Can you also in general ask him to help us review the
patches? Just to make sure we're not misusing something in the library..
> Patches are attached.
>
> Thanks,
> Michal

[Michal Židek]

On 09/25/2014 08:04 AM, Jakub Hrozek wrote:
> On Wed, Sep 24, 2014 at 06:52:15PM +0200, Michal Židek wrote:
> > Hi,
> >
> > these 3 patches remove the old approach where we directly
> > wrote the selinux login file. Here, the same approach is used
> > as in the tools (sss_useradd/sss_usermod) where we already
> > use libsemanage to manage these tasks.
>
> Thanks a lot for the patches! I'm afraid we won't have the capacity to
> review them until Tuesday and the next release, though..
> > The advantage is that we remove some lines of code (yay!)
> > and get the auditing of selinux user changes for free.
> >
> > 1st patch just moves stuff around, so that set_seuser
> > function can be used from IPA provider.
> >
> > 2nd patch is a very simple change that adds new attribute
> > to the set_seuser function (mls_range).
> >
> > The main functionality is in the 3rd patch. Note that to
> > test the patch, you must run in permissive mode.
> > $ setenforce Permissive
> >
> > Otherwise you will get AVC denials (because sssd_be is
> > not allowed to use libsemanage).
> >
> > So we will have to change the selinux policy in fedora/rhel
> > to make it work this way. I  will ping the selinux policy maintainer
> > after the patches are reviewed.
>
> Thanks, please do. Can you also in general ask him to help us review the
> patches? Just to make sure we're not misusing something in the library..

I pinged mgrepl and he took a look at the patches. He did not see any
problems with the patches, but he mostly look at how we use the
selinux  specific functions (libesemanage) so it still needs
review from SSSD developer.
> > Patches are attached.
> >
> > Thanks,
> > Michal

[Michal Židek]

Lukas requested some changes to Makefile.am
and dlopen-test offline.

New patches are attached.

Michal

[Lukas Slebodnik]

On (16/10/14 16:19), Michal Židek wrote:
> Lukas requested some changes to Makefile.am
> and dlopen-test offline.
>
> New patches are attached.
>
> Michal
> From e1af11faf7ba7320e90bddc855632154894b313a Mon Sep 17 00:00:00 2001
> From: Michal Zidek 
> Date: Wed, 24 Sep 2014 16:03:04 +0200
> Subject: [PATCH 2/3] sss_semanage: Add mlsrange parameter to set_seuser
>
> mlsrange parameter will be needed in IPA provider
> and probably at some point in the tools as well.

compilation will fail if sssd is built without semanage.

CC       src/util/libsss_config_la-sss_config.lo
./src/util/sss_semanage.c:358:5: error: conflicting types for 'set_seuser'
 int set_seuser(const char *login_name, const char *seuser_name)
     ^
In file included from ../src/util/sss_semanage.c:30:0:
./src/util/util.h:593:5: note: previous declaration of 'set_seuser' was here
 int set_seuser(const char *login_name, const char *seuser_name,
     ^
Makefile:9946: recipe for target 'src/util/sss_semanage.lo' failed

LS

[Michal Židek]

On 10/16/2014 05:25 PM, Lukas Slebodnik wrote:
> On (16/10/14 16:19), Michal Židek wrote:
> > Lukas requested some changes to Makefile.am
> > and dlopen-test offline.
> >
> > New patches are attached.
> >
> > Michal
> > From e1af11faf7ba7320e90bddc855632154894b313a Mon Sep 17 00:00:00 2001
> > From: Michal Zidek 
> > Date: Wed, 24 Sep 2014 16:03:04 +0200
> > Subject: [PATCH 2/3] sss_semanage: Add mlsrange parameter to set_seuser
> >
> > mlsrange parameter will be needed in IPA provider
> > and probably at some point in the tools as well.
>
> compilation will fail if sssd is built without semanage.
>
> CC       src/util/libsss_config_la-sss_config.lo
> ./src/util/sss_semanage.c:358:5: error: conflicting types for 'set_seuser'
>   int set_seuser(const char *login_name, const char *seuser_name)
>       ^
> In file included from ../src/util/sss_semanage.c:30:0:
> ./src/util/util.h:593:5: note: previous declaration of 'set_seuser' was here
>   int set_seuser(const char *login_name, const char *seuser_name,
>       ^
> Makefile:9946: recipe for target 'src/util/sss_semanage.lo' failed
>
> LS

Sorry for that.

New patches are attached.

Michal

[Lukas Slebodnik]

On (16/10/14 19:02), Michal Židek wrote:
> On 10/16/2014 05:25 PM, Lukas Slebodnik wrote:
> > On (16/10/14 16:19), Michal Židek wrote:
> > > Lukas requested some changes to Makefile.am
> > > and dlopen-test offline.
> > >
> > > New patches are attached.
> > >
> > > Michal
> > > From e1af11faf7ba7320e90bddc855632154894b313a Mon Sep 17 00:00:00 2001
> > > From: Michal Zidek 
> > > Date: Wed, 24 Sep 2014 16:03:04 +0200
> > > Subject: [PATCH 2/3] sss_semanage: Add mlsrange parameter to set_seuser
> > >
> > > mlsrange parameter will be needed in IPA provider
> > > and probably at some point in the tools as well.
> >
> > compilation will fail if sssd is built without semanage.
> >
> > CC       src/util/libsss_config_la-sss_config.lo
> > ./src/util/sss_semanage.c:358:5: error: conflicting types for 'set_seuser'
> >  int set_seuser(const char *login_name, const char *seuser_name)
> >      ^
> > In file included from ../src/util/sss_semanage.c:30:0:
> > ./src/util/util.h:593:5: note: previous declaration of 'set_seuser' was here
> >  int set_seuser(const char *login_name, const char *seuser_name,
> >      ^
> > Makefile:9946: recipe for target 'src/util/sss_semanage.lo' failed
> >
> > LS
>
> Sorry for that.
>
> New patches are attached.
>
> Michal

works fine with libsemanage and without.

ACK to all

LS

[Jakub Hrozek]

On Thu, Oct 16, 2014 at 07:45:32PM +0200, Lukas Slebodnik wrote:
> On (16/10/14 19:02), Michal Židek wrote:
> > On 10/16/2014 05:25 PM, Lukas Slebodnik wrote:
> > > On (16/10/14 16:19), Michal Židek wrote:
> > > > Lukas requested some changes to Makefile.am
> > > > and dlopen-test offline.
> > > >
> > > > New patches are attached.
> > > >
> > > > Michal
> > > > From e1af11faf7ba7320e90bddc855632154894b313a Mon Sep 17 00:00:00 2001
> > > > From: Michal Zidek 
> > > > Date: Wed, 24 Sep 2014 16:03:04 +0200
> > > > Subject: [PATCH 2/3] sss_semanage: Add mlsrange parameter to set_seuser
> > > >
> > > > mlsrange parameter will be needed in IPA provider
> > > > and probably at some point in the tools as well.
> > >
> > > compilation will fail if sssd is built without semanage.
> > >
> > > CC       src/util/libsss_config_la-sss_config.lo
> > > ./src/util/sss_semanage.c:358:5: error: conflicting types for 'set_seuser'
> > >  int set_seuser(const char *login_name, const char *seuser_name)
> > >      ^
> > > In file included from ../src/util/sss_semanage.c:30:0:
> > > ./src/util/util.h:593:5: note: previous declaration of 'set_seuser' was here
> > >  int set_seuser(const char *login_name, const char *seuser_name,
> > >      ^
> > > Makefile:9946: recipe for target 'src/util/sss_semanage.lo' failed
> > >
> > > LS
> >
> > Sorry for that.
> >
> > New patches are attached.
> >
> > Michal
>
> works fine with libsemanage and without.
>
> ACK to all
>
> LS

Thank you for the review. Similar to the root-drop patches, I'm going to
postpone pushing these to master until after we release 1.12.2.

[Jakub Hrozek]

On Thu, Oct 16, 2014 at 07:45:32PM +0200, Lukas Slebodnik wrote:
> works fine with libsemanage and without.
>
> ACK to all
>
> LS

* master:
    * 9c47c8c59b5c9078f342f82367cd0ad7857acef8
    * 180c7a75ee8507d459c7de21882dc714c59c3cc9
    * 42ec8af02ecf1937e4db9b1ecc6216022634f0f9