infelicity in context_user_set; new syscalls: setfileconat, etc.?

infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Jim Meyering]

Hello,

I've just discovered what may be a documentation error
for context_user_set, context_role_set, etc.

Those functions are documented as setting errno upon failure,
yet the following program prints "failed with errno=0":

  #include <stdio.h>
  #include <errno.h>
  #include <selinux/context.h>

  int
  main ()
  {
    context_t c = context_new ("a:b:c");
    if (!c)
      return 1;
    errno = 0;
    if (context_user_set (c, ":") != 0)
      printf ("failed with errno=%d\n", errno);
    return 0;
  }

FYI, as part of merging the coreutils SE Linux changes "upstream",
I've just rewritten chcon to use fts (and new openat-style functions,
getfileconat, lgetfileconat, setfileconat, lsetfileconat[*]).  Now,
chcon -R can operate on an arbitrarily deep hierarchy.
One test was to process a 100K-level hierarchy -- finished in just 19sec.
The old version chokes on PATH_MAX at a depth of 2k.

Can any of you point me to a test suite for chcon.
That might save me the trouble of writing my own.

Jim

[*] With linux and /proc, I can emulate functions like openat and
setfileconat, but not everyone has linux, and some linux environments
lack access to /proc, so...

Has anyone considered adding these system calls?
  getfileconat
  lgetfileconat
  setfileconat
  lsetfileconat

They'd be useful for the same reasons openat, lstatat, etc.
are useful (and are now in the Linux kernel).

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

On Sat, 29 Jul 2006 16:50:16 +0200, Jim Meyering said:

> FYI, as part of merging the coreutils SE Linux changes "upstream",
> I've just rewritten chcon to use fts (and new openat-style functions,
> getfileconat, lgetfileconat, setfileconat, lsetfileconat[*]).

OK...

> [*] With linux and /proc, I can emulate functions like openat and
> setfileconat, but not everyone has linux, and some linux environments
> lack access to /proc, so...

Everybody who cares about SELinux changes for coreutils presumably has Linux. ;)

And I suspect that nobody's running an SELinux system with no /proc mounted
(except on some *really* Martian-logic design for a Really Secure embedded
system or something...)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Jim Meyering]

Valdis.Kletnieks@vt.edu wrote:

> On Sat, 29 Jul 2006 16:50:16 +0200, Jim Meyering said:
>
>> FYI, as part of merging the coreutils SE Linux changes "upstream",
>> I've just rewritten chcon to use fts (and new openat-style functions,
>> getfileconat, lgetfileconat, setfileconat, lsetfileconat[*]).
>
> OK...
>
>> [*] With linux and /proc, I can emulate functions like openat and
>> setfileconat, but not everyone has linux, and some linux environments
>> lack access to /proc, so...
>
> Everybody who cares about SELinux changes for coreutils presumably has Linux. ;)

Of course :)

> And I suspect that nobody's running an SELinux system with no /proc mounted
> (except on some *really* Martian-logic design for a Really Secure embedded
> system or something...)

Let's assume that all properly-configured environments do mount /proc.
Are the required features[*] of /proc usable even in the most restrictive
environments?  If so, then the only remaining argument for adding syscalls
is one of efficiency -- not very compelling.

[*] The ability to access any FILE via /proc/self/fd/N/FILE,
where the directory containing FILE is open on file descriptor N.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Russell Coker]

On Monday 31 July 2006 18:39, Jim Meyering <jim@meyering.net> wrote:
> > And I suspect that nobody's running an SELinux system with no /proc
> > mounted (except on some *really* Martian-logic design for a Really Secure
> > embedded system or something...)
>
> Let's assume that all properly-configured environments do mount /proc.

Let's not, think of chroot environments.

> Are the required features[*] of /proc usable even in the most restrictive
> environments?

I doubt it at the moment.  But it wouldn't be difficult to enable these things 
for the rare cases where they are needed.

> If so, then the only remaining argument for adding syscalls 
> is one of efficiency -- not very compelling.
>
> [*] The ability to access any FILE via /proc/self/fd/N/FILE,
> where the directory containing FILE is open on file descriptor N.

I believe that the real question is whether processes in restrictive 
environments such as chroot's need to do chcon -R operations anyway, and if 
they do whether they need to go to great depth.

I have set up many chroot environments, many of which were so restrictive that 
they would not permit what you desire.  But I can't think of any of them 
having a need for deep chcon -R operations.

Would it be possible to use the current functionality and only skip to the 
other type when the path depth is exceeded?

-- 
http://www.coker.com.au/selinux/   My NSA Security Enhanced Linux packages
http://www.coker.com.au/bonnie++/  Bonnie++ hard drive benchmark
http://www.coker.com.au/postal/    Postal SMTP/POP benchmark
http://www.coker.com.au/~russell/  My home page

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Jim Meyering]

Russell Coker <russell@coker.com.au> wrote:

> On Monday 31 July 2006 18:39, Jim Meyering <jim@meyering.net> wrote:
>> > And I suspect that nobody's running an SELinux system with no /proc
>> > mounted (except on some *really* Martian-logic design for a Really Secure
>> > embedded system or something...)
>>
>> Let's assume that all properly-configured environments do mount /proc.
>
> Let's not, think of chroot environments.

These days, chroot environments seem more and more likely to require /proc.
That said, allowing openat et al to work even in a chroot without /proc
was one of the arguments for adding those functions to the kernel.

>> Are the required features[*] of /proc usable even in the most restrictive
>> environments?
>
> I doubt it at the moment.  But it wouldn't be difficult to enable these things
> for the rare cases where they are needed.

Is needing /proc so unusual, these days?  Even with glibc-2.4?
Quite a few glibc primitives are implemented to rely on those same
/proc features.

>> If so, then the only remaining argument for adding syscalls
>> is one of efficiency -- not very compelling.
>>
>> [*] The ability to access any FILE via /proc/self/fd/N/FILE,
>> where the directory containing FILE is open on file descriptor N.
>
> I believe that the real question is whether processes in restrictive
> environments such as chroot's need to do chcon -R operations anyway, and if
> they do whether they need to go to great depth.

In general, operating on deep hierarchies is only one benefit of using
the *at functions.  That paradigm also permits efficient (no O(N^2)
perf. hit) file system traversal (a la fts(1)) that is also thread safe.
Though of course, chcon doesn't care about thread safety.

> I have set up many chroot environments, many of which were so restrictive that
> they would not permit what you desire.  But I can't think of any of them
> having a need for deep chcon -R operations.
>
> Would it be possible to use the current functionality and only skip to the
> other type when the path depth is exceeded?

My working implementation degrades gracefully in all but some unusually
pathological cases.  I.e., when emulating a FOO*at function via /proc
fails, the implementation resorts to simulating it via
save_cwd/fchdir/FOO/restore_cwd, where save_cwd and restore_cwd are
in gnulib/coreutils' lib/save-cwd.c.  That emulation fails only when
save_cwd or restore_cwd fails, neither of which is likely in practice.
The classic failure case is when "." cannot be opened and getcwd fails.
As I said, unlikely.

If you want to see actual code before I check things in,
there are very similar bits in upstream CVS coreutils/lib/openat.c.
E.g., the fstatat function in that file:

  http://cvs.savannah.gnu.org/viewcvs/coreutils/lib/openat.c?root=coreutils&view=markup

In the upcoming coreutils-6.0, so far, the following tools already
use this sort of infrastructure: rm, du, chown, chmod, chgrp.
It makes them robust and bulletproof on systems with /proc or syscall
support for openat-style functions.  On deficient systems, the only
penalty is failure in unusual cases, as mentioned above.
So if you see a problem, please let me know.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Karl MacMillan]

On Sat, 2006-07-29 at 16:50 +0200, Jim Meyering wrote:
> Hello,
> 
> I've just discovered what may be a documentation error
> for context_user_set, context_role_set, etc.
> 
> Those functions are documented as setting errno upon failure,
> yet the following program prints "failed with errno=0":
> 
>   #include <stdio.h>
>   #include <errno.h>
>   #include <selinux/context.h>
> 
>   int
>   main ()
>   {
>     context_t c = context_new ("a:b:c");
>     if (!c)
>       return 1;
>     errno = 0;
>     if (context_user_set (c, ":") != 0)
>       printf ("failed with errno=%d\n", errno);
>     return 0;
>   }
> 

Looks like a documentation error to me - there is no attempt to set
errno in the code. In fact, most of libselinux looks like errno will
only get set as a result of a call to a lower level library function
(e.g., open or getxattr).

Can you create a patch to correct the documentation?

<snip>

> 
> Has anyone considered adding these system calls?
>   getfileconat
>   lgetfileconat
>   setfileconat
>   lsetfileconat
> 
> They'd be useful for the same reasons openat, lstatat, etc.
> are useful (and are now in the Linux kernel).
> 

Seems much simpler to use fgetfilecon, etc. that take a fd instead of a
path. They can be used in conjunction with openat and do not require a
new system call.

Currently there are no system calls for SELinux and I doubt that there
would be much enthusiasm for them upstream.

Karl

> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
> 

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Jim Meyering]

Karl MacMillan <kmacmillan@mentalrootkit.com> wrote:
> Looks like a documentation error to me - there is no attempt to set
> errno in the code. In fact, most of libselinux looks like errno will
> only get set as a result of a call to a lower level library function
> (e.g., open or getxattr).
>
> Can you create a patch to correct the documentation?

I'm new to selinux, but will be happy to take a shot.
What's the recipe for checking out the preferred libselinux sources?
Are there any patch submission guidelines?

>> Has anyone considered adding these system calls?
>>   getfileconat
>>   lgetfileconat
>>   setfileconat
>>   lsetfileconat
>>
>> They'd be useful for the same reasons openat, lstatat, etc.
>> are useful (and are now in the Linux kernel).
>
> Seems much simpler to use fgetfilecon, etc. that take a fd instead of a
> path. They can be used in conjunction with openat and do not require a
> new system call.

It would be simpler, indeed.
Unfortunately, that approach fails for any file that cannot be opened,
while the *fileconat approach imposes no extra restrictions.

> Currently there are no system calls for SELinux and I doubt that there
> would be much enthusiasm for them upstream.

I'm not holding my breath.
Besides, I was never really convinced it was worthwhile.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Stephen Smalley]

On Mon, 2006-07-31 at 15:48 +0200, Jim Meyering wrote:
> I'm new to selinux, but will be happy to take a shot.
> What's the recipe for checking out the preferred libselinux sources?
> Are there any patch submission guidelines?

svn co https://svn.sourceforge.net/svnroot/selinux/trunk/libselinux

kernel coding style preferred.

> I'm not holding my breath.
> Besides, I was never really convinced it was worthwhile.

Perhaps, but it would seem to be consistent with having fstatat() and
fchmodat(), and setxattr/getxattr are more general than just SELinux, so
it would benefit ACLs and other xattr users.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Jim Meyering]

Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Mon, 2006-07-31 at 15:48 +0200, Jim Meyering wrote:
>> I'm new to selinux, but will be happy to take a shot.
>> What's the recipe for checking out the preferred libselinux sources?
>> Are there any patch submission guidelines?
>
> svn co https://svn.sourceforge.net/svnroot/selinux/trunk/libselinux
>
> kernel coding style preferred.

Thanks.

>> I'm not holding my breath.
>> Besides, I was never really convinced it was worthwhile.
>
> Perhaps, but it would seem to be consistent with having fstatat() and
> fchmodat(), and setxattr/getxattr are more general than just SELinux, so
> it would benefit ACLs and other xattr users.

That makes sense.
Adding lsetfileconat et al as syscalls didn't feel right.
Now that I've actually seen the implementations (and that
they're based on setxattr/getxattr), adding getxattrat/setxattrat
syscalls seems much more palatable.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Karl MacMillan]

On Mon, 2006-07-31 at 15:48 +0200, Jim Meyering wrote:
> Karl MacMillan <kmacmillan@mentalrootkit.com> wrote:
> > Looks like a documentation error to me - there is no attempt to set
> > errno in the code. In fact, most of libselinux looks like errno will
> > only get set as a result of a call to a lower level library function
> > (e.g., open or getxattr).
> >
> > Can you create a patch to correct the documentation?
> 
> I'm new to selinux, but will be happy to take a shot.
> What's the recipe for checking out the preferred libselinux sources?
> Are there any patch submission guidelines?
> 

The source is at http://selinux.sourceforge.net/ - specifically at
http://sourceforge.net/svn/?group_id=21266.

Just post a patch to the list.

> >> Has anyone considered adding these system calls?
> >>   getfileconat
> >>   lgetfileconat
> >>   setfileconat
> >>   lsetfileconat
> >>
> >> They'd be useful for the same reasons openat, lstatat, etc.
> >> are useful (and are now in the Linux kernel).
> >
> > Seems much simpler to use fgetfilecon, etc. that take a fd instead of a
> > path. They can be used in conjunction with openat and do not require a
> > new system call.
> 
> It would be simpler, indeed.
> Unfortunately, that approach fails for any file that cannot be opened,
> while the *fileconat approach imposes no extra restrictions.
> 

Got it - Steve has the better solution in his mail (as usual).

Karl



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Jim Meyering]

Karl MacMillan <kmacmillan@mentalrootkit.com> wrote:

> On Sat, 2006-07-29 at 16:50 +0200, Jim Meyering wrote:
>> Hello,
>>
>> I've just discovered what may be a documentation error
>> for context_user_set, context_role_set, etc.
>>
>> Those functions are documented as setting errno upon failure,
>> yet the following program prints "failed with errno=0":
>>
>>   #include <stdio.h>
>>   #include <errno.h>
>>   #include <selinux/context.h>
>>
>>   int
>>   main ()
>>   {
>>     context_t c = context_new ("a:b:c");
>>     if (!c)
>>       return 1;
>>     errno = 0;
>>     if (context_user_set (c, ":") != 0)
>>       printf ("failed with errno=%d\n", errno);
>>     return 0;
>>   }
>>
>
> Looks like a documentation error to me - there is no attempt to set
> errno in the code. In fact, most of libselinux looks like errno will
> only get set as a result of a call to a lower level library function
> (e.g., open or getxattr).
>
> Can you create a patch to correct the documentation?

I've looked at the code and now feel that the documentation
should continue to say those functions set errno on failure.
There are many ways in which context_user_set can fail and
set errno to a useful value.  Callers should be able to use
errno in all cases.

The solution is to ensure that whenever those functions
fail they set errno to *something*.  In the case above,
a tiny change to src/context.c would accomplish this,
using EINVAL to tell the caller that ":" is invalid:

--- src/context.c.~1~	2006-07-31 18:29:01.000000000 +0200
+++ src/context.c	2006-07-31 18:30:57.000000000 +0200
@@ -2,6 +2,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <errno.h>
 
 #define COMP_USER  0
 #define COMP_ROLE  1
@@ -157,6 +158,7 @@ static int set_comp(context_private_t *
 			if (*p == '\t' || *p == '\n' || *p == '\r' ||
 			    ((*p == ':' || *p == ' ') && idx != COMP_RANGE)) {
 				free(t);
+				errno = EINVAL;
 				return 1;
 			}
 		}

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Stephen Smalley]

On Mon, 2006-07-31 at 18:37 +0200, Jim Meyering wrote:
> Karl MacMillan <kmacmillan@mentalrootkit.com> wrote:
> 
> > On Sat, 2006-07-29 at 16:50 +0200, Jim Meyering wrote:
> >> Hello,
> >>
> >> I've just discovered what may be a documentation error
> >> for context_user_set, context_role_set, etc.
> >>
> >> Those functions are documented as setting errno upon failure,
> >> yet the following program prints "failed with errno=0":
> >>
> >>   #include <stdio.h>
> >>   #include <errno.h>
> >>   #include <selinux/context.h>
> >>
> >>   int
> >>   main ()
> >>   {
> >>     context_t c = context_new ("a:b:c");
> >>     if (!c)
> >>       return 1;
> >>     errno = 0;
> >>     if (context_user_set (c, ":") != 0)
> >>       printf ("failed with errno=%d\n", errno);
> >>     return 0;
> >>   }
> >>
> >
> > Looks like a documentation error to me - there is no attempt to set
> > errno in the code. In fact, most of libselinux looks like errno will
> > only get set as a result of a call to a lower level library function
> > (e.g., open or getxattr).
> >
> > Can you create a patch to correct the documentation?
> 
> I've looked at the code and now feel that the documentation
> should continue to say those functions set errno on failure.
> There are many ways in which context_user_set can fail and
> set errno to a useful value.  Callers should be able to use
> errno in all cases.
> 
> The solution is to ensure that whenever those functions
> fail they set errno to *something*.  In the case above,
> a tiny change to src/context.c would accomplish this,
> using EINVAL to tell the caller that ":" is invalid:
> 
> --- src/context.c.~1~	2006-07-31 18:29:01.000000000 +0200
> +++ src/context.c	2006-07-31 18:30:57.000000000 +0200
> @@ -2,6 +2,7 @@
>  #include <string.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <errno.h>
>  
>  #define COMP_USER  0
>  #define COMP_ROLE  1
> @@ -157,6 +158,7 @@ static int set_comp(context_private_t *
>  			if (*p == '\t' || *p == '\n' || *p == '\r' ||
>  			    ((*p == ':' || *p == ' ') && idx != COMP_RANGE)) {
>  				free(t);
> +				errno = EINVAL;
>  				return 1;
>  			}
>  		}

Thanks, merged.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: infelicity in context_user_set; new syscalls: setfileconat, etc.?

[Stephen Smalley]

On Sat, 2006-07-29 at 16:50 +0200, Jim Meyering wrote:
> Hello,
> 
> I've just discovered what may be a documentation error
> for context_user_set, context_role_set, etc.
> 
> Those functions are documented as setting errno upon failure,
> yet the following program prints "failed with errno=0":
> 
>   #include <stdio.h>
>   #include <errno.h>
>   #include <selinux/context.h>
> 
>   int
>   main ()
>   {
>     context_t c = context_new ("a:b:c");
>     if (!c)
>       return 1;
>     errno = 0;
>     if (context_user_set (c, ":") != 0)
>       printf ("failed with errno=%d\n", errno);
>     return 0;
>   }

Yes, that is a documentation error.

> FYI, as part of merging the coreutils SE Linux changes "upstream",
> I've just rewritten chcon to use fts (and new openat-style functions,
> getfileconat, lgetfileconat, setfileconat, lsetfileconat[*]).  Now,
> chcon -R can operate on an arbitrarily deep hierarchy.
> One test was to process a 100K-level hierarchy -- finished in just 19sec.
> The old version chokes on PATH_MAX at a depth of 2k.
> 
> Can any of you point me to a test suite for chcon.
> That might save me the trouble of writing my own.

I don't think that there is one; the selinux kernel testsuite (included
in LTP) does make use of chcon as part of its tests of the kernel access
checks, but there isn't a separate testsuite that fully exercises chcon
at present.

> Jim
> 
> [*] With linux and /proc, I can emulate functions like openat and
> setfileconat, but not everyone has linux, and some linux environments
> lack access to /proc, so...
> 
> Has anyone considered adding these system calls?
>   getfileconat
>   lgetfileconat
>   setfileconat
>   lsetfileconat
> 
> They'd be useful for the same reasons openat, lstatat, etc.
> are useful (and are now in the Linux kernel).

Looks like we would only need fgetxattrat() and fsetxattrat() system
calls in the kernel, analogous to the fstatat() and fchmodat() calls,
and then libselinux could implement the *fileconat() functions on top of
them.  cc'd Ulrich since he introduced the *at functions into Linux.
  
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.