All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
       [not found] <1254727932.3849.8.camel@dhcp-lab-219.englab.brq.redhat.com>
@ 2009-10-05 18:44 ` Jim Meyering
  2009-10-05 19:17   ` Stephen Smalley
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Meyering @ 2009-10-05 18:44 UTC (permalink / raw)
  To: Ondřej Vašík, yaneti; +Cc: CoreutilsBugs, SE Linux

Ondřej Vašík wrote:
> as reported in https://bugzilla.redhat.com/show_bug.cgi?id=527142 by
> Yanko Kaneti, chcon aborts on SELinux disabled kernel due to missing
> check for SELinux enabled kernel. Attached patch is fixing the issue.
>
> Additionally - for consistency - error message of this check in runcon
> was changed to not hardcode "runcon" program name.

[Cc'ing the SELinux list for the question at the end
 Summary: chcon(1) aborts when a successful getfilecon
 result is passed to context_new and makes it return NULL. ]

Thanks, Ondřej.
At first, I planned to use that patch, mostly as-is, but
moving the chcon paragraph in NEWS "up" so that it's alphabetized.

  chcon no longer aborts on SELinux disabled system.
  [the bug dates back to the initial implementation]

However, since I have so far been unable to reproduce the failure,
(neither in mock, nor on bare-metal x86_64 with SELinux disabled)
and know that merely running chcon with SELinux disabled is not
enough to trigger the abort, what's written above is misleading.
What if the only way to trigger the abort is with a strangely-
or improperly-configured system?
In fact, until I understand how/why the offending code is being
reached, I hesitate to call this a bug or say where it originated.

If the getfilecon call succeeds, then why would context_new fail?
Right after I wrote the line above, it clicked.
Some versions of getfilecon can succeed (and return >= 0)
yet set the context string to "unlabeled".  *That* is what
causes trouble if you pass it to context_new.

This makes me want to write a getfilecon wrapper
that would convert that surprising result into a return
value of -1 with errno of ENOTSUP.  A wrapper would also
protect us from the small risk of folks using the older
libselinux versions that can return 0 and a NULL context.

Can the SELinux folks tell us under what circumstances
getfilecon will return 10 and set *context to "unlabeled"?

Thanks,

Jim


--
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
  2009-10-05 18:44 ` [PATCH]: chcon: no longer abort on SELinux disabled kernel Jim Meyering
@ 2009-10-05 19:17   ` Stephen Smalley
  2009-10-05 20:02     ` Jim Meyering
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Smalley @ 2009-10-05 19:17 UTC (permalink / raw)
  To: Jim Meyering; +Cc: Ondřej Vašík, yaneti, CoreutilsBugs, SE Linux

On Mon, 2009-10-05 at 20:44 +0200, Jim Meyering wrote:
> Ondřej Vašík wrote:
> > as reported in https://bugzilla.redhat.com/show_bug.cgi?id=527142 by
> > Yanko Kaneti, chcon aborts on SELinux disabled kernel due to missing
> > check for SELinux enabled kernel. Attached patch is fixing the issue.
> >
> > Additionally - for consistency - error message of this check in runcon
> > was changed to not hardcode "runcon" program name.
> 
> [Cc'ing the SELinux list for the question at the end
>  Summary: chcon(1) aborts when a successful getfilecon
>  result is passed to context_new and makes it return NULL. ]
> 
> Thanks, Ondřej.
> At first, I planned to use that patch, mostly as-is, but
> moving the chcon paragraph in NEWS "up" so that it's alphabetized.
> 
>   chcon no longer aborts on SELinux disabled system.
>   [the bug dates back to the initial implementation]
> 
> However, since I have so far been unable to reproduce the failure,
> (neither in mock, nor on bare-metal x86_64 with SELinux disabled)
> and know that merely running chcon with SELinux disabled is not
> enough to trigger the abort, what's written above is misleading.
> What if the only way to trigger the abort is with a strangely-
> or improperly-configured system?
> In fact, until I understand how/why the offending code is being
> reached, I hesitate to call this a bug or say where it originated.
> 
> If the getfilecon call succeeds, then why would context_new fail?
> Right after I wrote the line above, it clicked.
> Some versions of getfilecon can succeed (and return >= 0)
> yet set the context string to "unlabeled".  *That* is what
> causes trouble if you pass it to context_new.
> 
> This makes me want to write a getfilecon wrapper
> that would convert that surprising result into a return
> value of -1 with errno of ENOTSUP.  A wrapper would also
> protect us from the small risk of folks using the older
> libselinux versions that can return 0 and a NULL context.
> 
> Can the SELinux folks tell us under what circumstances
> getfilecon will return 10 and set *context to "unlabeled"?

Must have previously booted an ancient kernel with SELinux permissive
and no policy loaded.  Kernel was fixed by the commit below in 2006.
I'd recommend that he run the following to clean up the droppings in his
filesystem:
find / \( -fstype ext2 -o -fstype ext3 -o -fstype ext4 \) -exec setfattr -x security.selinux {} \;

commit 8aad38752e81d1d4de67e3d8e2524618ce7c9276
Author: Stephen Smalley <sds@tycho.nsa.gov>
Date:   Wed Mar 22 00:09:13 2006 -0800

    [PATCH] selinux: Disable automatic labeling of new inodes when no policy is loaded
    
    This patch disables the automatic labeling of new inodes on disk
    when no policy is loaded.
    
    Discussion is here:
    https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=180296
    
    In short, we're changing the behavior so that when no policy is loaded,
    SELinux does not label files at all.  Currently it does add an 'unlabeled'
    label in this case, which we've found causes problems later.



-- 
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
  2009-10-05 19:17   ` Stephen Smalley
@ 2009-10-05 20:02     ` Jim Meyering
  2009-10-06  8:14       ` Jim Meyering
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Meyering @ 2009-10-05 20:02 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Ondřej Vašík, yaneti, CoreutilsBugs, SE Linux

Stephen Smalley wrote:
...
> Must have previously booted an ancient kernel with SELinux permissive
> and no policy loaded.  Kernel was fixed by the commit below in 2006.
> I'd recommend that he run the following to clean up the droppings in his
> filesystem:
> find / \( -fstype ext2 -o -fstype ext3 -o -fstype ext4 \) -exec setfattr -x security.selinux {} \;
>
> commit 8aad38752e81d1d4de67e3d8e2524618ce7c9276
> Author: Stephen Smalley <sds@tycho.nsa.gov>
> Date:   Wed Mar 22 00:09:13 2006 -0800
>
>     [PATCH] selinux: Disable automatic labeling of new inodes when no policy is loaded

Thanks for the quick explanation!

--
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
  2009-10-05 20:02     ` Jim Meyering
@ 2009-10-06  8:14       ` Jim Meyering
  2009-10-07 12:37         ` Stephen Smalley
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Meyering @ 2009-10-06  8:14 UTC (permalink / raw)
  To: CoreutilsBugs, Ondřej Vašík
  Cc: SE Linux, yaneti, Stephen Smalley

Jim Meyering wrote:
> Stephen Smalley wrote:
> ...
>> Must have previously booted an ancient kernel with SELinux permissive
>> and no policy loaded.  Kernel was fixed by the commit below in 2006.
>> I'd recommend that he run the following to clean up the droppings in his
>> filesystem:
>> find / \( -fstype ext2 -o -fstype ext3 -o -fstype ext4 \) -exec setfattr -x security.selinux {} \;
>>
>> commit 8aad38752e81d1d4de67e3d8e2524618ce7c9276
>> Author: Stephen Smalley <sds@tycho.nsa.gov>
>> Date:   Wed Mar 22 00:09:13 2006 -0800
>>
>>     [PATCH] selinux: Disable automatic labeling of new inodes when no policy is loaded
>
> Thanks for the quick explanation!

I've revised the commit not to say anything in NEWS
and to expand the log message.  While the exit-early
change doesn't solve the problem in all cases, it is useful
and does make chcon consistent with runcon in that respect.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
  2009-10-06  8:14       ` Jim Meyering
@ 2009-10-07 12:37         ` Stephen Smalley
  2009-10-07 12:48           ` Jim Meyering
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Smalley @ 2009-10-07 12:37 UTC (permalink / raw)
  To: Jim Meyering; +Cc: CoreutilsBugs, Ondřej Vašík, SE Linux, yaneti

On Tue, 2009-10-06 at 10:14 +0200, Jim Meyering wrote:
> Jim Meyering wrote:
> > Stephen Smalley wrote:
> > ...
> >> Must have previously booted an ancient kernel with SELinux permissive
> >> and no policy loaded.  Kernel was fixed by the commit below in 2006.
> >> I'd recommend that he run the following to clean up the droppings in his
> >> filesystem:
> >> find / \( -fstype ext2 -o -fstype ext3 -o -fstype ext4 \) -exec setfattr -x security.selinux {} \;
> >>
> >> commit 8aad38752e81d1d4de67e3d8e2524618ce7c9276
> >> Author: Stephen Smalley <sds@tycho.nsa.gov>
> >> Date:   Wed Mar 22 00:09:13 2006 -0800
> >>
> >>     [PATCH] selinux: Disable automatic labeling of new inodes when no policy is loaded
> >
> > Thanks for the quick explanation!
> 
> I've revised the commit not to say anything in NEWS
> and to expand the log message.  While the exit-early
> change doesn't solve the problem in all cases, it is useful
> and does make chcon consistent with runcon in that respect.

FWIW, there is a subtle difference here:
- chcon can in fact work on a SELinux-disabled kernel, as you can still
set the security.* extended attributes as long as the filesystem
provides handlers for the security.* namespace.
- runcon cannot work without a SELinux-enabled kernel, as only a
SELinux-enabled kernel allows you to set the security context of a
running process.

So by preventing chcon from running in the SELinux-disabled case, you
are imposing a restriction above and beyond what is strictly required.
The user can of course still use setfattr -n security.selinux -v
<context> <path> to set a SELinux security context on a file when
SELinux is disabled, or can run the setfiles program to set SELinux
security contexts on an entire file tree even when SELinux is disabled.

> 
> >From 3a97d664b9f639fddb5a245775f47d27bfbb56c9 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <ovasik@redhat.com>
> Date: Mon, 5 Oct 2009 09:20:48 +0200
> Subject: [PATCH] chcon: exit immediately if SELinux is disabled
> 
> This change happens to avoid an abort in chcon when SELinux is
> disabled while operating on a file with an "unlabeled" context from
> back in 2006.  However, that same abort can still be triggered by the
> same file when running chcon with SELinux enabled.  This bug in chcon
> will be fixed in a subsequent commit via a getfilecon wrapper.  See
> http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/18378/focus=18384
> for how to correct your disk attributes to avoid triggering this bug.
> * src/chcon.c (main): Exit immediately if SELinux is disabled.
> Reported in http://bugzilla.redhat.com/527142 by Yanko Kaneti.
> * src/runcon.c (main): Do not hardcode program name in error message.
> * THANKS: Update.
> ---
>  THANKS       |    1 +
>  src/chcon.c  |    4 ++++
>  src/runcon.c |    2 +-
>  3 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/THANKS b/THANKS
> index e0e14e5..65ac1bb 100644
> --- a/THANKS
> +++ b/THANKS
> @@ -612,6 +612,7 @@ Wis Macomson                        wis.macomson@intel.com
>  Wojciech Purczynski                 cliph@isec.pl
>  Wolfram Kleff                       kleff@cs.uni-bonn.de
>  Won-kyu Park                        wkpark@chem.skku.ac.kr
> +Yanko Kaneti                        yaneti@declera.com
>  Yann Dirson                         dirson@debian.org
>  Zvi Har'El                          rl@math.technion.ac.il
> 
> diff --git a/src/chcon.c b/src/chcon.c
> index fbfdb4d..c0da694 100644
> --- a/src/chcon.c
> +++ b/src/chcon.c
> @@ -519,6 +519,10 @@ main (int argc, char **argv)
>        usage (EXIT_FAILURE);
>      }
> 
> +  if (is_selinux_enabled () != 1)
> +    error (EXIT_FAILURE, 0,
> +           _("%s may be used only on a SELinux kernel"), program_name);
> +
>    if (reference_file)
>      {
>        if (getfilecon (reference_file, &ref_context) < 0)
> diff --git a/src/runcon.c b/src/runcon.c
> index e0019da..f87eada 100644
> --- a/src/runcon.c
> +++ b/src/runcon.c
> @@ -195,7 +195,7 @@ main (int argc, char **argv)
> 
>    if (is_selinux_enabled () != 1)
>      error (EXIT_FAILURE, 0,
> -           _("runcon may be used only on a SELinux kernel"));
> +           _("%s may be used only on a SELinux kernel"), program_name);
> 
>    if (context)
>      {
> --
> 1.6.5.rc2.204.g8ea19
-- 
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
  2009-10-07 12:37         ` Stephen Smalley
@ 2009-10-07 12:48           ` Jim Meyering
  2009-10-07 12:54             ` Stephen Smalley
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Meyering @ 2009-10-07 12:48 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: CoreutilsBugs, Ondřej Vašík, SE Linux, yaneti

Stephen Smalley wrote:
...
> FWIW, there is a subtle difference here:
> - chcon can in fact work on a SELinux-disabled kernel, as you can still
> set the security.* extended attributes as long as the filesystem
> provides handlers for the security.* namespace.
> - runcon cannot work without a SELinux-enabled kernel, as only a
> SELinux-enabled kernel allows you to set the security context of a
> running process.
>
> So by preventing chcon from running in the SELinux-disabled case, you
> are imposing a restriction above and beyond what is strictly required.
> The user can of course still use setfattr -n security.selinux -v
> <context> <path> to set a SELinux security context on a file when
> SELinux is disabled, or can run the setfiles program to set SELinux
> security contexts on an entire file tree even when SELinux is disabled.
...
>> diff --git a/src/chcon.c b/src/chcon.c
>> index fbfdb4d..c0da694 100644
>> --- a/src/chcon.c
>> +++ b/src/chcon.c
>> @@ -519,6 +519,10 @@ main (int argc, char **argv)
>>        usage (EXIT_FAILURE);
>>      }
>>
>> +  if (is_selinux_enabled () != 1)
>> +    error (EXIT_FAILURE, 0,
>> +           _("%s may be used only on a SELinux kernel"), program_name);
>> +

Thanks for the tip.
I'll revert that part of the patch.

I'll address the original problem by adding
getfilecon and lgetfilecon wrappers that
map those unusual cases (10,"unlabeled" and 0,NULL)
to a return value of -1 with errno == ENOTSUPP.

--
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
  2009-10-07 12:48           ` Jim Meyering
@ 2009-10-07 12:54             ` Stephen Smalley
  2009-10-07 13:34               ` Jim Meyering
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Smalley @ 2009-10-07 12:54 UTC (permalink / raw)
  To: Jim Meyering; +Cc: CoreutilsBugs, Ondřej Vašík, SE Linux, yaneti

On Wed, 2009-10-07 at 14:48 +0200, Jim Meyering wrote:
> Stephen Smalley wrote:
> ...
> > FWIW, there is a subtle difference here:
> > - chcon can in fact work on a SELinux-disabled kernel, as you can still
> > set the security.* extended attributes as long as the filesystem
> > provides handlers for the security.* namespace.
> > - runcon cannot work without a SELinux-enabled kernel, as only a
> > SELinux-enabled kernel allows you to set the security context of a
> > running process.
> >
> > So by preventing chcon from running in the SELinux-disabled case, you
> > are imposing a restriction above and beyond what is strictly required.
> > The user can of course still use setfattr -n security.selinux -v
> > <context> <path> to set a SELinux security context on a file when
> > SELinux is disabled, or can run the setfiles program to set SELinux
> > security contexts on an entire file tree even when SELinux is disabled.
> ...
> >> diff --git a/src/chcon.c b/src/chcon.c
> >> index fbfdb4d..c0da694 100644
> >> --- a/src/chcon.c
> >> +++ b/src/chcon.c
> >> @@ -519,6 +519,10 @@ main (int argc, char **argv)
> >>        usage (EXIT_FAILURE);
> >>      }
> >>
> >> +  if (is_selinux_enabled () != 1)
> >> +    error (EXIT_FAILURE, 0,
> >> +           _("%s may be used only on a SELinux kernel"), program_name);
> >> +
> 
> Thanks for the tip.
> I'll revert that part of the patch.
> 
> I'll address the original problem by adding
> getfilecon and lgetfilecon wrappers that
> map those unusual cases (10,"unlabeled" and 0,NULL)
> to a return value of -1 with errno == ENOTSUPP.

I'd suggest ENODATA instead - that means that the filesystem supports
attributes but there was no value set for the particular file.

-- 
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
  2009-10-07 12:54             ` Stephen Smalley
@ 2009-10-07 13:34               ` Jim Meyering
  2009-10-07 15:30                 ` Stephen Smalley
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Meyering @ 2009-10-07 13:34 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: CoreutilsBugs, Ondřej Vašík, SE Linux, yaneti

Stephen Smalley wrote:

> On Wed, 2009-10-07 at 14:48 +0200, Jim Meyering wrote:
>> Stephen Smalley wrote:
>> ...
>> > FWIW, there is a subtle difference here:
>> > - chcon can in fact work on a SELinux-disabled kernel, as you can still
>> > set the security.* extended attributes as long as the filesystem
>> > provides handlers for the security.* namespace.
>> > - runcon cannot work without a SELinux-enabled kernel, as only a
>> > SELinux-enabled kernel allows you to set the security context of a
>> > running process.
>> >
>> > So by preventing chcon from running in the SELinux-disabled case, you
>> > are imposing a restriction above and beyond what is strictly required.
>> > The user can of course still use setfattr -n security.selinux -v
>> > <context> <path> to set a SELinux security context on a file when
>> > SELinux is disabled, or can run the setfiles program to set SELinux
>> > security contexts on an entire file tree even when SELinux is disabled.
>> ...
>> >> diff --git a/src/chcon.c b/src/chcon.c
>> >> index fbfdb4d..c0da694 100644
>> >> --- a/src/chcon.c
>> >> +++ b/src/chcon.c
>> >> @@ -519,6 +519,10 @@ main (int argc, char **argv)
>> >>        usage (EXIT_FAILURE);
>> >>      }
>> >>
>> >> +  if (is_selinux_enabled () != 1)
>> >> +    error (EXIT_FAILURE, 0,
>> >> +           _("%s may be used only on a SELinux kernel"), program_name);
>> >> +
>>
>> Thanks for the tip.
>> I'll revert that part of the patch.
>>
>> I'll address the original problem by adding
>> getfilecon and lgetfilecon wrappers that
>> map those unusual cases (10,"unlabeled" and 0,NULL)
>> to a return value of -1 with errno == ENOTSUPP.
>
> I'd suggest ENODATA instead - that means that the filesystem supports
> attributes but there was no value set for the particular file.

ENODATA makes sense for the 10,"unlabeled" case.
I viewed "using a library so old that its getfilecon
can return 0 and set context to NULL" as lacking support (ENOTSUPP).
But I'll do whatever is more consistent with the rest of SELinux.

Here's the first part: revert the above:

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
  2009-10-07 13:34               ` Jim Meyering
@ 2009-10-07 15:30                 ` Stephen Smalley
  2009-10-10  9:59                   ` Jim Meyering
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Smalley @ 2009-10-07 15:30 UTC (permalink / raw)
  To: Jim Meyering
  Cc: CoreutilsBugs, Ondřej Vašík, SE Linux, yaneti,
	Eric Paris

On Wed, 2009-10-07 at 15:34 +0200, Jim Meyering wrote:
> Stephen Smalley wrote:
> 
> > On Wed, 2009-10-07 at 14:48 +0200, Jim Meyering wrote:
> >> Stephen Smalley wrote:
> >> ...
> >> > FWIW, there is a subtle difference here:
> >> > - chcon can in fact work on a SELinux-disabled kernel, as you can still
> >> > set the security.* extended attributes as long as the filesystem
> >> > provides handlers for the security.* namespace.
> >> > - runcon cannot work without a SELinux-enabled kernel, as only a
> >> > SELinux-enabled kernel allows you to set the security context of a
> >> > running process.
> >> >
> >> > So by preventing chcon from running in the SELinux-disabled case, you
> >> > are imposing a restriction above and beyond what is strictly required.
> >> > The user can of course still use setfattr -n security.selinux -v
> >> > <context> <path> to set a SELinux security context on a file when
> >> > SELinux is disabled, or can run the setfiles program to set SELinux
> >> > security contexts on an entire file tree even when SELinux is disabled.
> >> ...
> >> >> diff --git a/src/chcon.c b/src/chcon.c
> >> >> index fbfdb4d..c0da694 100644
> >> >> --- a/src/chcon.c
> >> >> +++ b/src/chcon.c
> >> >> @@ -519,6 +519,10 @@ main (int argc, char **argv)
> >> >>        usage (EXIT_FAILURE);
> >> >>      }
> >> >>
> >> >> +  if (is_selinux_enabled () != 1)
> >> >> +    error (EXIT_FAILURE, 0,
> >> >> +           _("%s may be used only on a SELinux kernel"), program_name);
> >> >> +
> >>
> >> Thanks for the tip.
> >> I'll revert that part of the patch.
> >>
> >> I'll address the original problem by adding
> >> getfilecon and lgetfilecon wrappers that
> >> map those unusual cases (10,"unlabeled" and 0,NULL)
> >> to a return value of -1 with errno == ENOTSUPP.
> >
> > I'd suggest ENODATA instead - that means that the filesystem supports
> > attributes but there was no value set for the particular file.
> 
> ENODATA makes sense for the 10,"unlabeled" case.
> I viewed "using a library so old that its getfilecon
> can return 0 and set context to NULL" as lacking support (ENOTSUPP).
> But I'll do whatever is more consistent with the rest of SELinux.

Ah, I see - we did change getfilecon() in libselinux in 2007 to check
for a return value of zero from getxattr() and map that to -1 with errno
EOPNOTSUPP.  That was to address a change/regression in the kernel that
caused getxattr() on /proc/sys nodes to return 0.  So I guess EOPNOTSUPP
aka ENOTSUP is fine for the zero-return case.

-- 
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH]: chcon: no longer abort on SELinux disabled kernel
  2009-10-07 15:30                 ` Stephen Smalley
@ 2009-10-10  9:59                   ` Jim Meyering
  0 siblings, 0 replies; 10+ messages in thread
From: Jim Meyering @ 2009-10-10  9:59 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: CoreutilsBugs, Ondřej Vašík, SE Linux, yaneti,
	Eric Paris

Stephen Smalley wrote:
> On Wed, 2009-10-07 at 15:34 +0200, Jim Meyering wrote:
>> Stephen Smalley wrote:
>>
>> > On Wed, 2009-10-07 at 14:48 +0200, Jim Meyering wrote:
>> >> Stephen Smalley wrote:
>> >> ...
>> >> > FWIW, there is a subtle difference here:
>> >> > - chcon can in fact work on a SELinux-disabled kernel, as you can still
>> >> > set the security.* extended attributes as long as the filesystem
>> >> > provides handlers for the security.* namespace.
>> >> > - runcon cannot work without a SELinux-enabled kernel, as only a
>> >> > SELinux-enabled kernel allows you to set the security context of a
>> >> > running process.
>> >> >
>> >> > So by preventing chcon from running in the SELinux-disabled case, you
>> >> > are imposing a restriction above and beyond what is strictly required.
>> >> > The user can of course still use setfattr -n security.selinux -v
>> >> > <context> <path> to set a SELinux security context on a file when
>> >> > SELinux is disabled, or can run the setfiles program to set SELinux
>> >> > security contexts on an entire file tree even when SELinux is disabled.
>> >> ...
>> >> >> diff --git a/src/chcon.c b/src/chcon.c
>> >> >> index fbfdb4d..c0da694 100644
>> >> >> --- a/src/chcon.c
>> >> >> +++ b/src/chcon.c
>> >> >> @@ -519,6 +519,10 @@ main (int argc, char **argv)
>> >> >>        usage (EXIT_FAILURE);
>> >> >>      }
>> >> >>
>> >> >> +  if (is_selinux_enabled () != 1)
>> >> >> +    error (EXIT_FAILURE, 0,
>> >> >> +           _("%s may be used only on a SELinux kernel"), program_name);
>> >> >> +
>> >>
>> >> Thanks for the tip.
>> >> I'll revert that part of the patch.
>> >>
>> >> I'll address the original problem by adding
>> >> getfilecon and lgetfilecon wrappers that
>> >> map those unusual cases (10,"unlabeled" and 0,NULL)
>> >> to a return value of -1 with errno == ENOTSUPP.
>> >
>> > I'd suggest ENODATA instead - that means that the filesystem supports
>> > attributes but there was no value set for the particular file.
>>
>> ENODATA makes sense for the 10,"unlabeled" case.
>> I viewed "using a library so old that its getfilecon
>> can return 0 and set context to NULL" as lacking support (ENOTSUPP).
>> But I'll do whatever is more consistent with the rest of SELinux.
>
> Ah, I see - we did change getfilecon() in libselinux in 2007 to check
> for a return value of zero from getxattr() and map that to -1 with errno
> EOPNOTSUPP.  That was to address a change/regression in the kernel that
> caused getxattr() on /proc/sys nodes to return 0.  So I guess EOPNOTSUPP
> aka ENOTSUP is fine for the zero-return case.

I've resolved this via two changes:
One to add *getfilecon wrappers to gnulib:

    http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/19037

and another to clean up coreutils, now that it can depend on
the wrappers' API-conforming behavior:

    http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/18454

--
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-10-10  9:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1254727932.3849.8.camel@dhcp-lab-219.englab.brq.redhat.com>
2009-10-05 18:44 ` [PATCH]: chcon: no longer abort on SELinux disabled kernel Jim Meyering
2009-10-05 19:17   ` Stephen Smalley
2009-10-05 20:02     ` Jim Meyering
2009-10-06  8:14       ` Jim Meyering
2009-10-07 12:37         ` Stephen Smalley
2009-10-07 12:48           ` Jim Meyering
2009-10-07 12:54             ` Stephen Smalley
2009-10-07 13:34               ` Jim Meyering
2009-10-07 15:30                 ` Stephen Smalley
2009-10-10  9:59                   ` Jim Meyering

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.