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