Re: is_selinux_enabled() after chroot()

2018-06-19 Thread Petr Lautrbach
On Mon, Jun 18, 2018 at 04:06:11PM -0400, Stephen Smalley wrote:
> On 06/18/2018 03:24 PM, Petr Lautrbach wrote:
> > Hello,
> > 
> > libselinux sets selinut_mnt and has_selinux_config only in its constructor 
> > and
> > is_selinux_enabled() and others just use selinux_mnt to check if SELinux is
> > enabled. But it doesn't work correctly when you use chroot() to a directory 
> > without /proc
> > and /sys/fs/selinux mounted as it was discovered in
> > https://bugzilla.redhat.com/show_bug.cgi?id=1321375 
> > 
> > In this case, is_selinux_enabled() after chroot() returns true while in a 
> > new
> > program run from chrooted process it returns false. It can be demonstrated 
> > by
> > the steps below.
> > 
> > The solution could be to check if selinux_mnt still exists whenever a 
> > function
> > depending on this is called. Would this be acceptable?
> 
> You want to call stat() or access(F_OK) on selinux_mnt and/or SELINUXCONFIG 
> in is_selinux_enabled()?

Yes. I was thinking about something like this:

@@ -16,7 +16,7 @@ int is_selinux_enabled(void)
 #ifdef ANDROID
return (selinux_mnt ? 1 : 0);
 #else
-   return (selinux_mnt && has_selinux_config);
+   return (selinux_mnt && (access(selinux_mnt, F_OK) == 0) && 
has_selinux_config);
 #endif
 }

But the problem seems to be more complex and it would probably be better to fix
it on a callers side - mount /sys/fs/selinux and /proc into chroots or do all
SELinux checks before chroot().

> Could potentially trigger a permission check that wasn't previously required, 
> thereby breaking existing policies.
> Caller might just be checking to see if SELinux is enabled before using 
> interfaces other than selinuxfs (e.g. setexeccon, setfilecon, etc) and 
> therefore didn't previously need permissions to selinuxfs or 
> /etc/selinux/config.
> So, possible but you'd need to make sure you don't break anything.  
> Definitely don't want that changed in Android.
> > 
> > 
> > 
> > 
> > $ sudo dnf --nogpg --installroot=/var/lib/machines/example  install systemd
> > 
> > $ cat > test_libselinux.c < > #include 
> > #include 
> > #include 
> > #include 
> > #include 
> > 
> > int main(int argc, char *argv[]) {
> >   pid_t pid;
> >   int wstatus;
> > 
> >   if (argc > 1) {
> > printf("SELinux in chrooted process: %d\n", is_selinux_enabled());
> > return 0;
> >   }
> >   if (chroot("/var/lib/machines/example") != 0)
> > return -1;
> > 
> >   printf("SELinux in process after chroot(): %d\n", is_selinux_enabled());
> >   printf("/sys/fs/selinux exists: %d\n", access("/sys/fs/selinux", F_OK));
> >   printf("/etc/selinux/config exists: %d\n\n", 
> > access("/etc/selinux/config", F_OK));
> > 
> >   if ((pid = fork()) == 0 ) {
> > execv("./test_is_selinux_enabled", (char *[]){ 
> > "./test_is_selinux_enabled", "chrooted", NULL});
> >   }
> > 
> >   wait();
> >   return 0;
> > }
> > EOF
> > 
> > $ gcc -o test_is_selinux_enabled test_libselinux.c -lselinux
> > 
> > $ sudo ./test_is_selinux_enabled
> > SELinux in process after chroot(): 1
> > /sys/fs/selinux exists: -1
> > /etc/selinux/config exists: -1
> > 
> > SELinux in chrooted process: 0
> > 
> > 
> > 
> > ___
> > Selinux mailing list
> > Selinux@tycho.nsa.gov
> > To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> > To get help, send an email containing "help" to 
> > selinux-requ...@tycho.nsa.gov.
> > 
> 


signature.asc
Description: PGP signature
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: is_selinux_enabled() after chroot()

2018-06-18 Thread Stephen Smalley
On 06/18/2018 03:24 PM, Petr Lautrbach wrote:
> Hello,
> 
> libselinux sets selinut_mnt and has_selinux_config only in its constructor and
> is_selinux_enabled() and others just use selinux_mnt to check if SELinux is
> enabled. But it doesn't work correctly when you use chroot() to a directory 
> without /proc
> and /sys/fs/selinux mounted as it was discovered in
> https://bugzilla.redhat.com/show_bug.cgi?id=1321375 
> 
> In this case, is_selinux_enabled() after chroot() returns true while in a new
> program run from chrooted process it returns false. It can be demonstrated by
> the steps below.
> 
> The solution could be to check if selinux_mnt still exists whenever a function
> depending on this is called. Would this be acceptable?

You want to call stat() or access(F_OK) on selinux_mnt and/or SELINUXCONFIG in 
is_selinux_enabled()?
Could potentially trigger a permission check that wasn't previously required, 
thereby breaking existing policies.
Caller might just be checking to see if SELinux is enabled before using 
interfaces other than selinuxfs (e.g. setexeccon, setfilecon, etc) and 
therefore didn't previously need permissions to selinuxfs or 
/etc/selinux/config.
So, possible but you'd need to make sure you don't break anything.  Definitely 
don't want that changed in Android.

> 
> 
> 
> 
> $ sudo dnf --nogpg --installroot=/var/lib/machines/example  install systemd
> 
> $ cat > test_libselinux.c < #include 
> #include 
> #include 
> #include 
> #include 
> 
> int main(int argc, char *argv[]) {
>   pid_t pid;
>   int wstatus;
> 
>   if (argc > 1) {
> printf("SELinux in chrooted process: %d\n", is_selinux_enabled());
> return 0;
>   }
>   if (chroot("/var/lib/machines/example") != 0)
> return -1;
> 
>   printf("SELinux in process after chroot(): %d\n", is_selinux_enabled());
>   printf("/sys/fs/selinux exists: %d\n", access("/sys/fs/selinux", F_OK));
>   printf("/etc/selinux/config exists: %d\n\n", access("/etc/selinux/config", 
> F_OK));
> 
>   if ((pid = fork()) == 0 ) {
> execv("./test_is_selinux_enabled", (char *[]){ 
> "./test_is_selinux_enabled", "chrooted", NULL});
>   }
> 
>   wait();
>   return 0;
> }
> EOF
> 
> $ gcc -o test_is_selinux_enabled test_libselinux.c -lselinux
> 
> $ sudo ./test_is_selinux_enabled
> SELinux in process after chroot(): 1
> /sys/fs/selinux exists: -1
> /etc/selinux/config exists: -1
> 
> SELinux in chrooted process: 0
> 
> 
> 
> ___
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


is_selinux_enabled() after chroot()

2018-06-18 Thread Petr Lautrbach
Hello,

libselinux sets selinut_mnt and has_selinux_config only in its constructor and
is_selinux_enabled() and others just use selinux_mnt to check if SELinux is
enabled. But it doesn't work correctly when you use chroot() to a directory 
without /proc
and /sys/fs/selinux mounted as it was discovered in
https://bugzilla.redhat.com/show_bug.cgi?id=1321375 

In this case, is_selinux_enabled() after chroot() returns true while in a new
program run from chrooted process it returns false. It can be demonstrated by
the steps below.

The solution could be to check if selinux_mnt still exists whenever a function
depending on this is called. Would this be acceptable?




$ sudo dnf --nogpg --installroot=/var/lib/machines/example  install systemd

$ cat > test_libselinux.c <
#include 
#include 
#include 
#include 

int main(int argc, char *argv[]) {
  pid_t pid;
  int wstatus;

  if (argc > 1) {
printf("SELinux in chrooted process: %d\n", is_selinux_enabled());
return 0;
  }
  if (chroot("/var/lib/machines/example") != 0)
return -1;

  printf("SELinux in process after chroot(): %d\n", is_selinux_enabled());
  printf("/sys/fs/selinux exists: %d\n", access("/sys/fs/selinux", F_OK));
  printf("/etc/selinux/config exists: %d\n\n", access("/etc/selinux/config", 
F_OK));

  if ((pid = fork()) == 0 ) {
execv("./test_is_selinux_enabled", (char *[]){ "./test_is_selinux_enabled", 
"chrooted", NULL});
  }

  wait();
  return 0;
}
EOF

$ gcc -o test_is_selinux_enabled test_libselinux.c -lselinux

$ sudo ./test_is_selinux_enabled
SELinux in process after chroot(): 1
/sys/fs/selinux exists: -1
/etc/selinux/config exists: -1

SELinux in chrooted process: 0



signature.asc
Description: PGP signature
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.