* Patch to make libselinux shut up when SELinux is disabled.
@ 2008-07-18 17:16 Daniel J Walsh
2008-07-18 17:27 ` Stephen Smalley
0 siblings, 1 reply; 19+ messages in thread
From: Daniel J Walsh @ 2008-07-18 17:16 UTC (permalink / raw)
To: SE Linux
[-- Attachment #1: Type: text/plain, Size: 462 bytes --]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
SELinux complains about things like restorecon or rpm when conflicts
exist in file_context file even when SELinux is disabled.
It should just shut up....
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iEYEARECAAYFAkiAz9AACgkQrlYvE4MpobMO6wCdGC7kvnHsss5gBkQGy1s10MzS
WgIAn2Ryd93w9zfVihfDKyGa8M78E21W
=hwEi
-----END PGP SIGNATURE-----
[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 402 bytes --]
diff --exclude-from=exclude -N -u -r nsalibselinux/src/callbacks.c libselinux-2.0.67/src/callbacks.c
--- nsalibselinux/src/callbacks.c 2008-06-12 23:25:14.000000000 -0400
+++ libselinux-2.0.67/src/callbacks.c 2008-07-18 11:15:56.000000000 -0400
@@ -16,6 +16,7 @@
{
int rc;
va_list ap;
+ if (is_selinux_enabled() == 0) return 0;
va_start(ap, fmt);
rc = vfprintf(stderr, fmt, ap);
va_end(ap);
[-- Attachment #3: diff.sig --]
[-- Type: application/octet-stream, Size: 72 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Patch to make libselinux shut up when SELinux is disabled. 2008-07-18 17:16 Patch to make libselinux shut up when SELinux is disabled Daniel J Walsh @ 2008-07-18 17:27 ` Stephen Smalley 2008-07-18 17:40 ` Daniel J Walsh 0 siblings, 1 reply; 19+ messages in thread From: Stephen Smalley @ 2008-07-18 17:27 UTC (permalink / raw) To: Daniel J Walsh; +Cc: SE Linux, Christopher J. PeBenito On Fri, 2008-07-18 at 13:16 -0400, Daniel J Walsh wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > SELinux complains about things like restorecon or rpm when conflicts > exist in file_context file even when SELinux is disabled. > > It should just shut up.... I think that's the wrong place to do it: - the fact that the application called libselinux at all except to test is_selinux_enabled() when SELinux is disabled is either a bug in the application or an indication that the application wants SELinux behavior regardless, - silencing all log messages coming from libselinux is too broad. And of course, file_contexts conflicts should be caught during policy build time aside from local customizations; if not, then we need to change the policy build process to do that even for modular policy builds. > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.9 (GNU/Linux) > Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org > > iEYEARECAAYFAkiAz9AACgkQrlYvE4MpobMO6wCdGC7kvnHsss5gBkQGy1s10MzS > WgIAn2Ryd93w9zfVihfDKyGa8M78E21W > =hwEi > -----END PGP SIGNATURE----- > plain text document attachment (diff) > diff --exclude-from=exclude -N -u -r nsalibselinux/src/callbacks.c libselinux-2.0.67/src/callbacks.c > --- nsalibselinux/src/callbacks.c 2008-06-12 23:25:14.000000000 -0400 > +++ libselinux-2.0.67/src/callbacks.c 2008-07-18 11:15:56.000000000 -0400 > @@ -16,6 +16,7 @@ > { > int rc; > va_list ap; > + if (is_selinux_enabled() == 0) return 0; > va_start(ap, fmt); > rc = vfprintf(stderr, fmt, ap); > va_end(ap); -- 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] 19+ messages in thread
* Re: Patch to make libselinux shut up when SELinux is disabled. 2008-07-18 17:27 ` Stephen Smalley @ 2008-07-18 17:40 ` Daniel J Walsh 2008-07-18 17:54 ` Stephen Smalley 0 siblings, 1 reply; 19+ messages in thread From: Daniel J Walsh @ 2008-07-18 17:40 UTC (permalink / raw) To: Stephen Smalley; +Cc: SE Linux, Christopher J. PeBenito -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Stephen Smalley wrote: > On Fri, 2008-07-18 at 13:16 -0400, Daniel J Walsh wrote: > SELinux complains about things like restorecon or rpm when conflicts > exist in file_context file even when SELinux is disabled. > > It should just shut up.... > >> I think that's the wrong place to do it: >> - the fact that the application called libselinux at all except to test >> is_selinux_enabled() when SELinux is disabled is either a bug in the >> application or an indication that the application wants SELinux behavior >> regardless, >> - silencing all log messages coming from libselinux is too broad. > >> And of course, file_contexts conflicts should be caught during policy >> build time aside from local customizations; if not, then we need to >> change the policy build process to do that even for modular policy >> builds. > Well it could be argued that libraries should never write to the terminal... Try explaining this to the users we are telling selinux disabled does not effect their machines. They just come away with the opinion that SELinux Sucks and is broken. Besides we are even complaining when they are warnings and SELinux is disabled. The problem seems to be a broken genhomedircon, but we don't currently prevent users (rpm post install scripts) from adding conflicting file context in file context.local plain text document attachment (diff) The tools just complain about it after the fact. # semanage fcontext -a -t httpd_sys_content_t /tmp # matchpathcon /etc # matchpathcon /etc/ /etc/selinux/targeted/contexts/files/file_contexts: Multiple different specifications for /tmp (system_u:object_r:httpd_sys_content_t:s0 and system_u:object_r:tmp_t:s0). /etc/ system_u:object_r:etc_t:s0 Nice. diff --exclude-from=exclude -N -u -r nsalibselinux/src/callbacks.c libselinux-2.0.67/src/callbacks.c - --- nsalibselinux/src/callbacks.c 2008-06-12 23:25:14.000000000 -0400 +++ libselinux-2.0.67/src/callbacks.c 2008-07-18 11:15:56.000000000 -0400 @@ -16,6 +16,7 @@ { int rc; va_list ap; + if (is_selinux_enabled() == 0) return 0; va_start(ap, fmt); rc = vfprintf(stderr, fmt, ap); va_end(ap); -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkiA1YUACgkQrlYvE4MpobOXawCgqA+eAWSiytqSXvdmAXSOboMU 7BMAoOh5vRzyK8u8sWC/fIIaEuiK+I0F =XAen -----END PGP SIGNATURE----- -- 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] 19+ messages in thread
* Re: Patch to make libselinux shut up when SELinux is disabled. 2008-07-18 17:40 ` Daniel J Walsh @ 2008-07-18 17:54 ` Stephen Smalley 2008-07-18 18:34 ` [rfc][patch] setfiles: validate all file_contexts files when using -c Stephen Smalley 2008-07-18 18:50 ` Patch to make libselinux shut up when SELinux is disabled Daniel J Walsh 0 siblings, 2 replies; 19+ messages in thread From: Stephen Smalley @ 2008-07-18 17:54 UTC (permalink / raw) To: Daniel J Walsh; +Cc: SE Linux, Christopher J. PeBenito, Joshua Brindle On Fri, 2008-07-18 at 13:40 -0400, Daniel J Walsh wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Stephen Smalley wrote: > > On Fri, 2008-07-18 at 13:16 -0400, Daniel J Walsh wrote: > > SELinux complains about things like restorecon or rpm when conflicts > > exist in file_context file even when SELinux is disabled. > > > > It should just shut up.... > > > >> I think that's the wrong place to do it: > >> - the fact that the application called libselinux at all except to test > >> is_selinux_enabled() when SELinux is disabled is either a bug in the > >> application or an indication that the application wants SELinux behavior > >> regardless, > >> - silencing all log messages coming from libselinux is too broad. > > > >> And of course, file_contexts conflicts should be caught during policy > >> build time aside from local customizations; if not, then we need to > >> change the policy build process to do that even for modular policy > >> builds. > > > Well it could be argued that libraries should never write to the terminal... selinux_set_callback() lets the application control that behavior. > Try explaining this to the users we are telling selinux disabled does > not effect their machines. They just come away with the opinion that > SELinux Sucks and is broken. Besides we are even complaining when they > are warnings and SELinux is disabled. Which reflects a bug in the application - why is it calling matchpathcon if is_selinux_enabled() <= 0? As far as the warning goes, the log callback does get a type parameter so it can distinguish warnings vs. errors. But we don't want to hide these kinds of duplications or conflicts - we just want them to be caught earlier. > The problem seems to be a broken genhomedircon, but we don't currently > prevent users (rpm post install scripts) from adding conflicting file > context in file context.local > plain text document attachment (diff) > > The tools just complain about it after the fact. > > # semanage fcontext -a -t httpd_sys_content_t /tmp Ah, that's interesting. So the setfiles -c execution by libsemanage only validates the base file contexts. I'm trying to remember why we did that. Seems like that could be changed easily enough. > # matchpathcon /etc > # matchpathcon /etc/ > /etc/selinux/targeted/contexts/files/file_contexts: Multiple different > specifications for /tmp (system_u:object_r:httpd_sys_content_t:s0 and > system_u:object_r:tmp_t:s0). > /etc/ system_u:object_r:etc_t:s0 > > Nice. That's a real bug in the file_contexts file that shouldn't be hidden from the user. I do agree that a) the application shouldn't be calling matchpathcon if is_selinux_enabled() <= 0, and b) we ought to catch these kinds of errors at policy build time for the base policy, and c) we ought to catch these kinds of errors at semodule/semanage invocation time for local customizations. That is what we should be fixing. Not silencing the messenger. -- 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] 19+ messages in thread
* [rfc][patch] setfiles: validate all file_contexts files when using -c 2008-07-18 17:54 ` Stephen Smalley @ 2008-07-18 18:34 ` Stephen Smalley 2008-07-18 18:37 ` [rfc][patch] setfiles: validate all file_contexts files whenusing -c Joshua Brindle 2008-07-18 19:01 ` [patch] libselinux: handle conflicting file contexts as a fatal error Stephen Smalley 2008-07-18 18:50 ` Patch to make libselinux shut up when SELinux is disabled Daniel J Walsh 1 sibling, 2 replies; 19+ messages in thread From: Stephen Smalley @ 2008-07-18 18:34 UTC (permalink / raw) To: Daniel J Walsh; +Cc: SE Linux, Christopher J. PeBenito, Joshua Brindle In ancient days of yore, setfiles could only validate the base file_contexts configuration because the .homedirs or .local configurations might include local users that weren't defined by the base policy since those definitions were brought in at policy load time. These days the policy.N file contains all of the definitions required to validate all file_contexts files and thus setfiles can and should validate them. Before: # /usr/sbin/semanage fcontext -a -t httpd_sys_content_t /tmp (no warnings) After: # /usr/sbin/semanage fcontext -a -t httpd_sys_content_t /tmp /etc/selinux/targeted/contexts/files/file_contexts: Multiple different specifications for /tmp (system_u:object_r:httpd_sys_content_t:s0 and system_u:object_r:tmp_t:s0). If we want it to be a fatal error, then libselinux should be patched to return an error in this case on the selabel_open() (requires making nodups_specs return an error to the caller and propagating it up). Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> --- Index: trunk/policycoreutils/setfiles/setfiles.c =================================================================== --- trunk/policycoreutils/setfiles/setfiles.c (revision 2927) +++ trunk/policycoreutils/setfiles/setfiles.c (working copy) @@ -72,7 +72,6 @@ static int abort_on_error; /* Abort the file tree walk upon an error. */ static int add_assoc; /* Track inode associations for conflict detection. */ static int nftw_flags; /* Flags to nftw, e.g. follow links, follow mounts */ -static int base_only; /* Don't use local file_contexts customizations */ static int ctx_validate; /* Validate contexts */ static const char *altpath; /* Alternate path to file_contexts */ @@ -748,7 +747,6 @@ char *base; struct selinux_opt opts[] = { { SELABEL_OPT_VALIDATE, NULL }, - { SELABEL_OPT_BASEONLY, NULL }, { SELABEL_OPT_PATH, NULL } }; @@ -836,10 +834,6 @@ } fclose(policystream); - /* Only process the specified file_contexts file, not - any .homedirs or .local files, and do not perform - context translations. */ - base_only = 1; ctx_validate = 1; break; @@ -972,10 +966,9 @@ /* Load the file contexts configuration and check it. */ opts[0].value = (ctx_validate ? (char*)1 : NULL); - opts[1].value = (base_only ? (char *)1 : NULL); - opts[2].value = altpath; + opts[1].value = altpath; - hnd = selabel_open(SELABEL_CTX_FILE, opts, 3); + hnd = selabel_open(SELABEL_CTX_FILE, opts, 2); if (!hnd) { perror(altpath); exit(1); -- 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] 19+ messages in thread
* RE: [rfc][patch] setfiles: validate all file_contexts files whenusing -c 2008-07-18 18:34 ` [rfc][patch] setfiles: validate all file_contexts files when using -c Stephen Smalley @ 2008-07-18 18:37 ` Joshua Brindle 2008-07-18 18:47 ` Stephen Smalley 2008-07-18 19:01 ` [patch] libselinux: handle conflicting file contexts as a fatal error Stephen Smalley 1 sibling, 1 reply; 19+ messages in thread From: Joshua Brindle @ 2008-07-18 18:37 UTC (permalink / raw) To: Stephen Smalley, Daniel J Walsh; +Cc: SE Linux, Christopher J. PeBenito Stephen Smalley wrote: > In ancient days of yore, setfiles could only validate the > base file_contexts configuration because the .homedirs or > .local configurations might include local users that weren't > defined by the base policy since those definitions were > brought in at policy load time. > These days the policy.N file contains all of the definitions > required to validate all file_contexts files and thus > setfiles can and should validate them. > I don't think that was the motivation. We want to explicitely support a user overriding a context specified in the policy with semanage and I think giving this cryptic warning is going to confuse people. For all other semanage objects we allow overriding the main policy with no warning, why would this be different? > Before: > # /usr/sbin/semanage fcontext -a -t httpd_sys_content_t /tmp (no > warnings) After: > # /usr/sbin/semanage fcontext -a -t httpd_sys_content_t /tmp > /etc/selinux/targeted/contexts/files/file_contexts: Multiple > different specifications for /tmp > (system_u:object_r:httpd_sys_content_t:s0 and > system_u:object_r:tmp_t:s0). > > If we want it to be a fatal error, then libselinux should be > patched to return an error in this case on the selabel_open() > (requires making nodups_specs return an error to the caller and > propagating it up). > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > > --- > > Index: trunk/policycoreutils/setfiles/setfiles.c > =================================================================== > --- trunk/policycoreutils/setfiles/setfiles.c (revision 2927) > +++ trunk/policycoreutils/setfiles/setfiles.c (working copy) @@ -72,7 > +72,6 @@ static int abort_on_error; /* Abort the file tree walk upon > an error. */ static int add_assoc; /* Track inode > associations for conflict detection. */ static int > nftw_flags; /* Flags to nftw, e.g. follow links, follow > mounts */ -static int base_only; /* Don't use local > file_contexts customizations */ static int ctx_validate; /* > Validate contexts */ static const char *altpath; /* > Alternate path to file_contexts */ > > @@ -748,7 +747,6 @@ > char *base; > struct selinux_opt opts[] = { > { SELABEL_OPT_VALIDATE, NULL }, > - { SELABEL_OPT_BASEONLY, NULL }, > { SELABEL_OPT_PATH, NULL } > }; > > @@ -836,10 +834,6 @@ > } > fclose(policystream); > > - /* Only process the specified > file_contexts file, not > - any .homedirs or .local > files, and do not perform > - context translations. */ > - base_only = 1; > ctx_validate = 1; > > break; > @@ -972,10 +966,9 @@ > > /* Load the file contexts configuration and check it. */ > opts[0].value = (ctx_validate ? (char*)1 : NULL); > - opts[1].value = (base_only ? (char *)1 : NULL); > - opts[2].value = altpath; > + opts[1].value = altpath; > > - hnd = selabel_open(SELABEL_CTX_FILE, opts, 3); > + hnd = selabel_open(SELABEL_CTX_FILE, opts, 2); > if (!hnd) { > perror(altpath); > exit(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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [rfc][patch] setfiles: validate all file_contexts files whenusing -c 2008-07-18 18:37 ` [rfc][patch] setfiles: validate all file_contexts files whenusing -c Joshua Brindle @ 2008-07-18 18:47 ` Stephen Smalley 2008-07-18 19:09 ` [rfc][patch] setfiles: validate all file_contexts fileswhenusing -c Joshua Brindle 0 siblings, 1 reply; 19+ messages in thread From: Stephen Smalley @ 2008-07-18 18:47 UTC (permalink / raw) To: Joshua Brindle; +Cc: Daniel J Walsh, SE Linux, Christopher J. PeBenito On Fri, 2008-07-18 at 14:37 -0400, Joshua Brindle wrote: > Stephen Smalley wrote: > > In ancient days of yore, setfiles could only validate the > > base file_contexts configuration because the .homedirs or > > .local configurations might include local users that weren't > > defined by the base policy since those definitions were > > brought in at policy load time. > > These days the policy.N file contains all of the definitions > > required to validate all file_contexts files and thus > > setfiles can and should validate them. > > > > I don't think that was the motivation. We want to explicitely support a > user overriding a context specified in the policy with semanage and I > think giving this cryptic warning is going to confuse people. For all > other semanage objects we allow overriding the main policy with no > warning, why would this be different? I actually went back through the mailing list archives, and the reason I stated (inability to validate entries using localusers) was the the motivation for baseonly validation in setfiles -c. We've never supported users creating conflicting file contexts entries with the base policy as far as I know. We've always warned on this condition. I suppose we could alter that behavior but I'm not sure it will work as expected at present; the logic in libselinux to move all exact specs to the end could for example interfere with it. Or if we did a full fcsort of file_contexts.local as has been suggested elsewhere. And one could have dups within file_contexts.local itself, which we can't presently distinguish from dups between base and .local with the current logic. > > Before: > > # /usr/sbin/semanage fcontext -a -t httpd_sys_content_t /tmp (no > > warnings) After: > > # /usr/sbin/semanage fcontext -a -t httpd_sys_content_t /tmp > > /etc/selinux/targeted/contexts/files/file_contexts: Multiple > > different specifications for /tmp > > (system_u:object_r:httpd_sys_content_t:s0 and > > system_u:object_r:tmp_t:s0). > > > > If we want it to be a fatal error, then libselinux should be > > patched to return an error in this case on the selabel_open() > > (requires making nodups_specs return an error to the caller and > > propagating it up). > > > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > > > > --- > > > > Index: trunk/policycoreutils/setfiles/setfiles.c > > =================================================================== > > --- trunk/policycoreutils/setfiles/setfiles.c (revision 2927) > > +++ trunk/policycoreutils/setfiles/setfiles.c (working copy) @@ -72,7 > > +72,6 @@ static int abort_on_error; /* Abort the file tree walk upon > > an error. */ static int add_assoc; /* Track inode > > associations for conflict detection. */ static int > > nftw_flags; /* Flags to nftw, e.g. follow links, follow > > mounts */ -static int base_only; /* Don't use local > > file_contexts customizations */ static int ctx_validate; /* > > Validate contexts */ static const char *altpath; /* > > Alternate path to file_contexts */ > > > > @@ -748,7 +747,6 @@ > > char *base; > > struct selinux_opt opts[] = { > > { SELABEL_OPT_VALIDATE, NULL }, > > - { SELABEL_OPT_BASEONLY, NULL }, > > { SELABEL_OPT_PATH, NULL } > > }; > > > > @@ -836,10 +834,6 @@ > > } > > fclose(policystream); > > > > - /* Only process the specified > > file_contexts file, not > > - any .homedirs or .local > > files, and do not perform > > - context translations. */ > > - base_only = 1; > > ctx_validate = 1; > > > > break; > > @@ -972,10 +966,9 @@ > > > > /* Load the file contexts configuration and check it. */ > > opts[0].value = (ctx_validate ? (char*)1 : NULL); > > - opts[1].value = (base_only ? (char *)1 : NULL); > > - opts[2].value = altpath; > > + opts[1].value = altpath; > > > > - hnd = selabel_open(SELABEL_CTX_FILE, opts, 3); > > + hnd = selabel_open(SELABEL_CTX_FILE, opts, 2); > > if (!hnd) { > > perror(altpath); > > exit(1); > -- 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] 19+ messages in thread
* RE: [rfc][patch] setfiles: validate all file_contexts fileswhenusing -c 2008-07-18 18:47 ` Stephen Smalley @ 2008-07-18 19:09 ` Joshua Brindle 2008-07-18 19:10 ` Stephen Smalley 0 siblings, 1 reply; 19+ messages in thread From: Joshua Brindle @ 2008-07-18 19:09 UTC (permalink / raw) To: Stephen Smalley; +Cc: Daniel J Walsh, SE Linux, Christopher J. PeBenito Stephen Smalley wrote: > On Fri, 2008-07-18 at 14:37 -0400, Joshua Brindle wrote: >> Stephen Smalley wrote: >>> In ancient days of yore, setfiles could only validate the base >>> file_contexts configuration because the .homedirs or .local >>> configurations might include local users that weren't defined by the >>> base policy since those definitions were brought in at policy load >>> time. These days the policy.N file contains all of the definitions >>> required to validate all file_contexts files and thus setfiles can >>> and should validate them. >>> >> >> I don't think that was the motivation. We want to explicitely support >> a user overriding a context specified in the policy with semanage and >> I think giving this cryptic warning is going to confuse people. For >> all other semanage objects we allow overriding the main policy with >> no warning, why would this be different? > > I actually went back through the mailing list archives, and > the reason I stated (inability to validate entries using > localusers) was the the motivation for baseonly validation in > setfiles -c. > > We've never supported users creating conflicting file > contexts entries with the base policy as far as I know. > We've always warned on this condition. > > I suppose we could alter that behavior but I'm not sure it > will work as expected at present; the logic in libselinux to > move all exact specs to the end could for example interfere > with it. Or if we did a full fcsort of file_contexts.local > as has been suggested elsewhere. And one could have dups > within file_contexts.local itself, which we can't presently > distinguish from dups between base and .local with the current logic. > All the same I think that a user overriding a policy file context should be explicitely supported. -- 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] 19+ messages in thread
* RE: [rfc][patch] setfiles: validate all file_contexts fileswhenusing -c 2008-07-18 19:09 ` [rfc][patch] setfiles: validate all file_contexts fileswhenusing -c Joshua Brindle @ 2008-07-18 19:10 ` Stephen Smalley 2008-07-18 19:12 ` [rfc][patch] setfiles: validate all file_contextsfileswhenusing -c Joshua Brindle 0 siblings, 1 reply; 19+ messages in thread From: Stephen Smalley @ 2008-07-18 19:10 UTC (permalink / raw) To: Joshua Brindle; +Cc: Daniel J Walsh, SE Linux, Christopher J. PeBenito On Fri, 2008-07-18 at 15:09 -0400, Joshua Brindle wrote: > Stephen Smalley wrote: > > On Fri, 2008-07-18 at 14:37 -0400, Joshua Brindle wrote: > >> Stephen Smalley wrote: > >>> In ancient days of yore, setfiles could only validate the base > >>> file_contexts configuration because the .homedirs or .local > >>> configurations might include local users that weren't defined by the > >>> base policy since those definitions were brought in at policy load > >>> time. These days the policy.N file contains all of the definitions > >>> required to validate all file_contexts files and thus setfiles can > >>> and should validate them. > >>> > >> > >> I don't think that was the motivation. We want to explicitely support > >> a user overriding a context specified in the policy with semanage and > >> I think giving this cryptic warning is going to confuse people. For > >> all other semanage objects we allow overriding the main policy with > >> no warning, why would this be different? > > > > I actually went back through the mailing list archives, and > > the reason I stated (inability to validate entries using > > localusers) was the the motivation for baseonly validation in > > setfiles -c. > > > > We've never supported users creating conflicting file > > contexts entries with the base policy as far as I know. > > We've always warned on this condition. > > > > I suppose we could alter that behavior but I'm not sure it > > will work as expected at present; the logic in libselinux to > > move all exact specs to the end could for example interfere > > with it. Or if we did a full fcsort of file_contexts.local > > as has been suggested elsewhere. And one could have dups > > within file_contexts.local itself, which we can't presently > > distinguish from dups between base and .local with the current logic. > > > > All the same I think that a user overriding a policy file context should > be explicitely supported. Patches accepted ;) -- 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] 19+ messages in thread
* RE: [rfc][patch] setfiles: validate all file_contextsfileswhenusing -c 2008-07-18 19:10 ` Stephen Smalley @ 2008-07-18 19:12 ` Joshua Brindle 2008-07-18 19:23 ` Stephen Smalley 0 siblings, 1 reply; 19+ messages in thread From: Joshua Brindle @ 2008-07-18 19:12 UTC (permalink / raw) To: Stephen Smalley; +Cc: Daniel J Walsh, SE Linux, Christopher J. PeBenito Stephen Smalley wrote: > On Fri, 2008-07-18 at 15:09 -0400, Joshua Brindle wrote: >> Stephen Smalley wrote: >>> On Fri, 2008-07-18 at 14:37 -0400, Joshua Brindle wrote: >>>> Stephen Smalley wrote: >>>>> In ancient days of yore, setfiles could only validate the base >>>>> file_contexts configuration because the .homedirs or .local >>>>> configurations might include local users that weren't defined by >>>>> the base policy since those definitions were brought in at policy >>>>> load time. These days the policy.N file contains all of the >>>>> definitions required to validate all file_contexts files and thus >>>>> setfiles can and should validate them. >>>>> >>>> >>>> I don't think that was the motivation. We want to explicitely >>>> support a user overriding a context specified in the policy with >>>> semanage and I think giving this cryptic warning is going to >>>> confuse people. For all other semanage objects we allow overriding >>>> the main policy with no warning, why would this be different? >>> >>> I actually went back through the mailing list archives, and the >>> reason I stated (inability to validate entries using >>> localusers) was the the motivation for baseonly validation in >>> setfiles -c. >>> >>> We've never supported users creating conflicting file contexts >>> entries with the base policy as far as I know. >>> We've always warned on this condition. >>> >>> I suppose we could alter that behavior but I'm not sure it will work >>> as expected at present; the logic in libselinux to move all exact >>> specs to the end could for example interfere with it. Or if we did >>> a full fcsort of file_contexts.local as has been suggested >>> elsewhere. And one could have dups within file_contexts.local >>> itself, which we can't presently distinguish from dups between base >>> and .local with the current logic. >>> >> >> All the same I think that a user overriding a policy file context >> should be explicitely supported. > > Patches accepted ;) So, your current patch is disabling this support altogether, right? -- 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] 19+ messages in thread
* RE: [rfc][patch] setfiles: validate all file_contextsfileswhenusing -c 2008-07-18 19:12 ` [rfc][patch] setfiles: validate all file_contextsfileswhenusing -c Joshua Brindle @ 2008-07-18 19:23 ` Stephen Smalley 0 siblings, 0 replies; 19+ messages in thread From: Stephen Smalley @ 2008-07-18 19:23 UTC (permalink / raw) To: Joshua Brindle; +Cc: Daniel J Walsh, SE Linux, Christopher J. PeBenito On Fri, 2008-07-18 at 15:12 -0400, Joshua Brindle wrote: > Stephen Smalley wrote: > > On Fri, 2008-07-18 at 15:09 -0400, Joshua Brindle wrote: > >> Stephen Smalley wrote: > >>> On Fri, 2008-07-18 at 14:37 -0400, Joshua Brindle wrote: > >>>> Stephen Smalley wrote: > >>>>> In ancient days of yore, setfiles could only validate the base > >>>>> file_contexts configuration because the .homedirs or .local > >>>>> configurations might include local users that weren't defined by > >>>>> the base policy since those definitions were brought in at policy > >>>>> load time. These days the policy.N file contains all of the > >>>>> definitions required to validate all file_contexts files and thus > >>>>> setfiles can and should validate them. > >>>>> > >>>> > >>>> I don't think that was the motivation. We want to explicitely > >>>> support a user overriding a context specified in the policy with > >>>> semanage and I think giving this cryptic warning is going to > >>>> confuse people. For all other semanage objects we allow overriding > >>>> the main policy with no warning, why would this be different? > >>> > >>> I actually went back through the mailing list archives, and the > >>> reason I stated (inability to validate entries using > >>> localusers) was the the motivation for baseonly validation in > >>> setfiles -c. > >>> > >>> We've never supported users creating conflicting file contexts > >>> entries with the base policy as far as I know. > >>> We've always warned on this condition. > >>> > >>> I suppose we could alter that behavior but I'm not sure it will work > >>> as expected at present; the logic in libselinux to move all exact > >>> specs to the end could for example interfere with it. Or if we did > >>> a full fcsort of file_contexts.local as has been suggested > >>> elsewhere. And one could have dups within file_contexts.local > >>> itself, which we can't presently distinguish from dups between base > >>> and .local with the current logic. > >>> > >> > >> All the same I think that a user overriding a policy file context > >> should be explicitely supported. > > > > Patches accepted ;) > > So, your current patch is disabling this support altogether, right? There is no support for duplicate pathname regexes presently (although obviously you can always add more specific ones that overlap), and letting the user add duplicates via semanage just means constant spew from rpm, udev, restorecon, etc. If we want to support duplicates in file_contexts.local that take precedence over file_contexts, then we still want nodups checking within the base file_contexts at least, and likely within file_contexts.local (i.e. no duplication among the local customizations). We just want to support duplication between file_contexts and file_contexts.local, with guaranteed precedence/ordering. I'm not taking any functionality away with these patches. -- 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] 19+ messages in thread
* [patch] libselinux: handle conflicting file contexts as a fatal error 2008-07-18 18:34 ` [rfc][patch] setfiles: validate all file_contexts files when using -c Stephen Smalley 2008-07-18 18:37 ` [rfc][patch] setfiles: validate all file_contexts files whenusing -c Joshua Brindle @ 2008-07-18 19:01 ` Stephen Smalley 2008-07-18 19:04 ` Daniel J Walsh 1 sibling, 1 reply; 19+ messages in thread From: Stephen Smalley @ 2008-07-18 19:01 UTC (permalink / raw) To: Daniel J Walsh; +Cc: SE Linux, Christopher J. PeBenito, Joshua Brindle Ensure that conflicting file context errors are propagated to the caller. This causes setfiles -c to exit with an error status, which in turn causes libsemanage to roll back the transaction and prevents such entries from being added by semanage. Duplicate same entries are left as warnings-only since they don't create any ambiguity for file labeling. Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> --- Index: trunk/libselinux/src/label_file.c =================================================================== --- trunk/libselinux/src/label_file.c (revision 2927) +++ trunk/libselinux/src/label_file.c (working copy) @@ -146,8 +146,9 @@ /* * Warn about duplicate specifications. */ -static void nodups_specs(struct saved_data *data, const char *path) +static int nodups_specs(struct saved_data *data, const char *path) { + int rc = 0; unsigned int ii, jj; struct spec *curr_spec, *spec_arr = data->spec_arr; @@ -161,8 +162,10 @@ if (strcmp (spec_arr[jj].lr.ctx_raw, curr_spec->lr.ctx_raw)) { + rc = -1; + errno = EINVAL; COMPAT_LOG - (SELINUX_WARNING, + (SELINUX_ERROR, "%s: Multiple different specifications for %s (%s and %s).\n", path, curr_spec->regex_str, spec_arr[jj].lr.ctx_raw, @@ -176,6 +179,7 @@ } } } + return rc; } /* Determine if the regular expression specification has any meta characters. */ @@ -503,6 +507,10 @@ } free(line_buf); + status = nodups_specs(data, path); + if (status) + goto finish; + /* Move exact pathname specifications to the end. */ spec_copy = malloc(sizeof(spec_t) * data->nspec); if (!spec_copy) @@ -519,8 +527,6 @@ free(data->spec_arr); data->spec_arr = spec_copy; - nodups_specs(data, path); - status = 0; finish: fclose(fp); -- 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] 19+ messages in thread
* Re: [patch] libselinux: handle conflicting file contexts as a fatal error 2008-07-18 19:01 ` [patch] libselinux: handle conflicting file contexts as a fatal error Stephen Smalley @ 2008-07-18 19:04 ` Daniel J Walsh 2008-07-18 19:09 ` [patch v2] libselinux: handle duplicate file context entries " Stephen Smalley 0 siblings, 1 reply; 19+ messages in thread From: Daniel J Walsh @ 2008-07-18 19:04 UTC (permalink / raw) To: Stephen Smalley; +Cc: SE Linux, Christopher J. PeBenito, Joshua Brindle Stephen Smalley wrote: > Ensure that conflicting file context errors are propagated to the > caller. This causes setfiles -c to exit with an error status, which in > turn causes libsemanage to roll back the transaction and prevents such > entries from being added by semanage. > > Duplicate same entries are left as warnings-only since they don't create > any ambiguity for file labeling. > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > > --- > > Index: trunk/libselinux/src/label_file.c > =================================================================== > --- trunk/libselinux/src/label_file.c (revision 2927) > +++ trunk/libselinux/src/label_file.c (working copy) > @@ -146,8 +146,9 @@ > /* > * Warn about duplicate specifications. > */ > -static void nodups_specs(struct saved_data *data, const char *path) > +static int nodups_specs(struct saved_data *data, const char *path) > { > + int rc = 0; > unsigned int ii, jj; > struct spec *curr_spec, *spec_arr = data->spec_arr; > > @@ -161,8 +162,10 @@ > if (strcmp > (spec_arr[jj].lr.ctx_raw, > curr_spec->lr.ctx_raw)) { > + rc = -1; > + errno = EINVAL; > COMPAT_LOG > - (SELINUX_WARNING, > + (SELINUX_ERROR, > "%s: Multiple different specifications for %s (%s and %s).\n", > path, curr_spec->regex_str, > spec_arr[jj].lr.ctx_raw, > @@ -176,6 +179,7 @@ > } > } > } > + return rc; > } > > /* Determine if the regular expression specification has any meta characters. */ > @@ -503,6 +507,10 @@ > } > free(line_buf); > > + status = nodups_specs(data, path); > + if (status) > + goto finish; > + > /* Move exact pathname specifications to the end. */ > spec_copy = malloc(sizeof(spec_t) * data->nspec); > if (!spec_copy) > @@ -519,8 +527,6 @@ > free(data->spec_arr); > data->spec_arr = spec_copy; > > - nodups_specs(data, path); > - > status = 0; > finish: > fclose(fp); > What about the other half. why are we allowing you to add dups that match? If we are going to blab on about it being a problem then we should not allow it in the first place. -- 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] 19+ messages in thread
* [patch v2] libselinux: handle duplicate file context entries as a fatal error 2008-07-18 19:04 ` Daniel J Walsh @ 2008-07-18 19:09 ` Stephen Smalley 0 siblings, 0 replies; 19+ messages in thread From: Stephen Smalley @ 2008-07-18 19:09 UTC (permalink / raw) To: Daniel J Walsh; +Cc: SE Linux, Christopher J. PeBenito, Joshua Brindle Take two. Ensure that duplicate file context entry errors are propagated to the caller, causing setfiles -c to exit with an error status and libsemanage to roll back the transaction. Do it for both duplicate same entries and for duplicate conflicting entries. Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> --- Index: trunk/libselinux/src/label_file.c =================================================================== --- trunk/libselinux/src/label_file.c (revision 2927) +++ trunk/libselinux/src/label_file.c (working copy) @@ -146,8 +146,9 @@ /* * Warn about duplicate specifications. */ -static void nodups_specs(struct saved_data *data, const char *path) +static int nodups_specs(struct saved_data *data, const char *path) { + int rc = 0; unsigned int ii, jj; struct spec *curr_spec, *spec_arr = data->spec_arr; @@ -158,24 +159,27 @@ (spec_arr[jj].regex_str, curr_spec->regex_str)) && (!spec_arr[jj].mode || !curr_spec->mode || spec_arr[jj].mode == curr_spec->mode)) { + rc = -1; + errno = EINVAL; if (strcmp (spec_arr[jj].lr.ctx_raw, curr_spec->lr.ctx_raw)) { COMPAT_LOG - (SELINUX_WARNING, + (SELINUX_ERROR, "%s: Multiple different specifications for %s (%s and %s).\n", path, curr_spec->regex_str, spec_arr[jj].lr.ctx_raw, curr_spec->lr.ctx_raw); } else { COMPAT_LOG - (SELINUX_WARNING, + (SELINUX_ERROR, "%s: Multiple same specifications for %s.\n", path, curr_spec->regex_str); } } } } + return rc; } /* Determine if the regular expression specification has any meta characters. */ @@ -503,6 +507,10 @@ } free(line_buf); + status = nodups_specs(data, path); + if (status) + goto finish; + /* Move exact pathname specifications to the end. */ spec_copy = malloc(sizeof(spec_t) * data->nspec); if (!spec_copy) @@ -519,8 +527,6 @@ free(data->spec_arr); data->spec_arr = spec_copy; - nodups_specs(data, path); - status = 0; finish: fclose(fp); -- 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] 19+ messages in thread
* Re: Patch to make libselinux shut up when SELinux is disabled. 2008-07-18 17:54 ` Stephen Smalley 2008-07-18 18:34 ` [rfc][patch] setfiles: validate all file_contexts files when using -c Stephen Smalley @ 2008-07-18 18:50 ` Daniel J Walsh 2008-08-04 15:06 ` Paul Howarth 1 sibling, 1 reply; 19+ messages in thread From: Daniel J Walsh @ 2008-07-18 18:50 UTC (permalink / raw) To: Stephen Smalley; +Cc: SE Linux, Christopher J. PeBenito, Joshua Brindle Stephen Smalley wrote: > On Fri, 2008-07-18 at 13:40 -0400, Daniel J Walsh wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> Stephen Smalley wrote: >>> On Fri, 2008-07-18 at 13:16 -0400, Daniel J Walsh wrote: >>> SELinux complains about things like restorecon or rpm when conflicts >>> exist in file_context file even when SELinux is disabled. >>> >>> It should just shut up.... >>> >>>> I think that's the wrong place to do it: >>>> - the fact that the application called libselinux at all except to test >>>> is_selinux_enabled() when SELinux is disabled is either a bug in the >>>> application or an indication that the application wants SELinux behavior >>>> regardless, >>>> - silencing all log messages coming from libselinux is too broad. >>>> And of course, file_contexts conflicts should be caught during policy >>>> build time aside from local customizations; if not, then we need to >>>> change the policy build process to do that even for modular policy >>>> builds. >> Well it could be argued that libraries should never write to the terminal... > > selinux_set_callback() lets the application control that behavior. > >> Try explaining this to the users we are telling selinux disabled does >> not effect their machines. They just come away with the opinion that >> SELinux Sucks and is broken. Besides we are even complaining when they >> are warnings and SELinux is disabled. > > Which reflects a bug in the application - why is it calling matchpathcon > if is_selinux_enabled() <= 0? > > As far as the warning goes, the log callback does get a type parameter > so it can distinguish warnings vs. errors. But we don't want to hide > these kinds of duplications or conflicts - we just want them to be > caught earlier. > >> The problem seems to be a broken genhomedircon, but we don't currently >> prevent users (rpm post install scripts) from adding conflicting file >> context in file context.local >> plain text document attachment (diff) >> >> The tools just complain about it after the fact. >> >> # semanage fcontext -a -t httpd_sys_content_t /tmp > > Ah, that's interesting. So the setfiles -c execution by libsemanage > only validates the base file contexts. I'm trying to remember why we > did that. Seems like that could be changed easily enough. > >> # matchpathcon /etc >> # matchpathcon /etc/ >> /etc/selinux/targeted/contexts/files/file_contexts: Multiple different >> specifications for /tmp (system_u:object_r:httpd_sys_content_t:s0 and >> system_u:object_r:tmp_t:s0). >> /etc/ system_u:object_r:etc_t:s0 >> >> Nice. > > That's a real bug in the file_contexts file that shouldn't be hidden > from the user. > > I do agree that a) the application shouldn't be calling matchpathcon if > is_selinux_enabled() <= 0, and b) we ought to catch these kinds of > errors at policy build time for the base policy, and c) we ought to > catch these kinds of errors at semodule/semanage invocation time for > local customizations. That is what we should be fixing. Not silencing > the messenger. > setfiles works while selinux is disabled. It could be the kerberos libraries, or udev. I still think warnings on a disabled system should be quietly ignored or thrown in syslog at most. Having the machine scream at boot, is a mistake. kerberos libraries can not call selinux_set_callback() since they do not know if the process has set it already. Yes I agree semanage should stop this from happening at compile time. -- 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] 19+ messages in thread
* Re: Patch to make libselinux shut up when SELinux is disabled. 2008-07-18 18:50 ` Patch to make libselinux shut up when SELinux is disabled Daniel J Walsh @ 2008-08-04 15:06 ` Paul Howarth 2008-08-04 17:42 ` Daniel J Walsh 2008-08-04 17:51 ` Stephen Smalley 0 siblings, 2 replies; 19+ messages in thread From: Paul Howarth @ 2008-08-04 15:06 UTC (permalink / raw) To: Daniel J Walsh Cc: Stephen Smalley, SE Linux, Christopher J. PeBenito, Joshua Brindle Daniel J Walsh wrote: > Stephen Smalley wrote: >> On Fri, 2008-07-18 at 13:40 -0400, Daniel J Walsh wrote: >>> -----BEGIN PGP SIGNED MESSAGE----- >>> Hash: SHA1 >>> >>> Stephen Smalley wrote: >>>> On Fri, 2008-07-18 at 13:16 -0400, Daniel J Walsh wrote: >>>> SELinux complains about things like restorecon or rpm when conflicts >>>> exist in file_context file even when SELinux is disabled. >>>> >>>> It should just shut up.... >>>> >>>>> I think that's the wrong place to do it: >>>>> - the fact that the application called libselinux at all except to test >>>>> is_selinux_enabled() when SELinux is disabled is either a bug in the >>>>> application or an indication that the application wants SELinux behavior >>>>> regardless, >>>>> - silencing all log messages coming from libselinux is too broad. >>>>> And of course, file_contexts conflicts should be caught during policy >>>>> build time aside from local customizations; if not, then we need to >>>>> change the policy build process to do that even for modular policy >>>>> builds. >>> Well it could be argued that libraries should never write to the terminal... >> selinux_set_callback() lets the application control that behavior. >> >>> Try explaining this to the users we are telling selinux disabled does >>> not effect their machines. They just come away with the opinion that >>> SELinux Sucks and is broken. Besides we are even complaining when they >>> are warnings and SELinux is disabled. >> Which reflects a bug in the application - why is it calling matchpathcon >> if is_selinux_enabled() <= 0? >> >> As far as the warning goes, the log callback does get a type parameter >> so it can distinguish warnings vs. errors. But we don't want to hide >> these kinds of duplications or conflicts - we just want them to be >> caught earlier. >> >>> The problem seems to be a broken genhomedircon, but we don't currently >>> prevent users (rpm post install scripts) from adding conflicting file >>> context in file context.local >>> plain text document attachment (diff) >>> >>> The tools just complain about it after the fact. >>> >>> # semanage fcontext -a -t httpd_sys_content_t /tmp >> Ah, that's interesting. So the setfiles -c execution by libsemanage >> only validates the base file contexts. I'm trying to remember why we >> did that. Seems like that could be changed easily enough. >> >>> # matchpathcon /etc >>> # matchpathcon /etc/ >>> /etc/selinux/targeted/contexts/files/file_contexts: Multiple different >>> specifications for /tmp (system_u:object_r:httpd_sys_content_t:s0 and >>> system_u:object_r:tmp_t:s0). >>> /etc/ system_u:object_r:etc_t:s0 >>> >>> Nice. >> That's a real bug in the file_contexts file that shouldn't be hidden >> from the user. >> >> I do agree that a) the application shouldn't be calling matchpathcon if >> is_selinux_enabled() <= 0, and b) we ought to catch these kinds of >> errors at policy build time for the base policy, and c) we ought to >> catch these kinds of errors at semodule/semanage invocation time for >> local customizations. That is what we should be fixing. Not silencing >> the messenger. >> > setfiles works while selinux is disabled. It could be the kerberos > libraries, or udev. I still think warnings on a disabled system should > be quietly ignored or thrown in syslog at most. Having the machine > scream at boot, is a mistake. > > kerberos libraries can not call selinux_set_callback() since they do not > know if the process has set it already. > > Yes I agree semanage should stop this from happening at compile time. This may present a problem for policy developers. For instance, I am writing new policy for spamass-milter, which currently shares spamd_t with spamassassin. I need spamass-milter to transition into a different domain, so I need to specify a new context for /usr/bin/spamass-milter in my policy module. This conflicts with the existing context for the same file (spamd_exec_t) in the main selinux-policy-targeted package and I get warnings like this on most rpm/selinux operations: /etc/selinux/targeted/contexts/files/file_contexts: Multiple different specifications for /usr/sbin/spamass-milter (system_u:object_r:milter_spamass_exec_t:s0 and system_u:object_r:spamd_exec_t:s0). For whatever reason, the context from my local module "wins" and I get the desired result. However, if semanage didn't allow this, I believe I'd need to fork the selinux-policy package for the duration of my development to prevent the unwanted context specification from being used. Or is there some other way around this? Paul. -- 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] 19+ messages in thread
* Re: Patch to make libselinux shut up when SELinux is disabled. 2008-08-04 15:06 ` Paul Howarth @ 2008-08-04 17:42 ` Daniel J Walsh 2008-08-05 13:50 ` Paul Howarth 2008-08-04 17:51 ` Stephen Smalley 1 sibling, 1 reply; 19+ messages in thread From: Daniel J Walsh @ 2008-08-04 17:42 UTC (permalink / raw) To: Paul Howarth Cc: Stephen Smalley, SE Linux, Christopher J. PeBenito, Joshua Brindle -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Paul Howarth wrote: > Daniel J Walsh wrote: >> Stephen Smalley wrote: >>> On Fri, 2008-07-18 at 13:40 -0400, Daniel J Walsh wrote: >>>> -----BEGIN PGP SIGNED MESSAGE----- >>>> Hash: SHA1 >>>> >>>> Stephen Smalley wrote: >>>>> On Fri, 2008-07-18 at 13:16 -0400, Daniel J Walsh wrote: >>>>> SELinux complains about things like restorecon or rpm when conflicts >>>>> exist in file_context file even when SELinux is disabled. >>>>> >>>>> It should just shut up.... >>>>> >>>>>> I think that's the wrong place to do it: >>>>>> - the fact that the application called libselinux at all except to >>>>>> test >>>>>> is_selinux_enabled() when SELinux is disabled is either a bug in the >>>>>> application or an indication that the application wants SELinux >>>>>> behavior >>>>>> regardless, >>>>>> - silencing all log messages coming from libselinux is too broad. >>>>>> And of course, file_contexts conflicts should be caught during policy >>>>>> build time aside from local customizations; if not, then we need to >>>>>> change the policy build process to do that even for modular policy >>>>>> builds. >>>> Well it could be argued that libraries should never write to the >>>> terminal... >>> selinux_set_callback() lets the application control that behavior. >>> >>>> Try explaining this to the users we are telling selinux disabled does >>>> not effect their machines. They just come away with the opinion that >>>> SELinux Sucks and is broken. Besides we are even complaining when they >>>> are warnings and SELinux is disabled. >>> Which reflects a bug in the application - why is it calling matchpathcon >>> if is_selinux_enabled() <= 0? >>> >>> As far as the warning goes, the log callback does get a type parameter >>> so it can distinguish warnings vs. errors. But we don't want to hide >>> these kinds of duplications or conflicts - we just want them to be >>> caught earlier. >>> >>>> The problem seems to be a broken genhomedircon, but we don't currently >>>> prevent users (rpm post install scripts) from adding conflicting file >>>> context in file context.local >>>> plain text document attachment (diff) >>>> >>>> The tools just complain about it after the fact. >>>> >>>> # semanage fcontext -a -t httpd_sys_content_t /tmp >>> Ah, that's interesting. So the setfiles -c execution by libsemanage >>> only validates the base file contexts. I'm trying to remember why we >>> did that. Seems like that could be changed easily enough. >>> >>>> # matchpathcon /etc >>>> # matchpathcon /etc/ >>>> /etc/selinux/targeted/contexts/files/file_contexts: Multiple different >>>> specifications for /tmp (system_u:object_r:httpd_sys_content_t:s0 and >>>> system_u:object_r:tmp_t:s0). >>>> /etc/ system_u:object_r:etc_t:s0 >>>> >>>> Nice. >>> That's a real bug in the file_contexts file that shouldn't be hidden >>> from the user. >>> >>> I do agree that a) the application shouldn't be calling matchpathcon if >>> is_selinux_enabled() <= 0, and b) we ought to catch these kinds of >>> errors at policy build time for the base policy, and c) we ought to >>> catch these kinds of errors at semodule/semanage invocation time for >>> local customizations. That is what we should be fixing. Not silencing >>> the messenger. >>> >> setfiles works while selinux is disabled. It could be the kerberos >> libraries, or udev. I still think warnings on a disabled system should >> be quietly ignored or thrown in syslog at most. Having the machine >> scream at boot, is a mistake. >> >> kerberos libraries can not call selinux_set_callback() since they do not >> know if the process has set it already. >> >> Yes I agree semanage should stop this from happening at compile time. > > This may present a problem for policy developers. For instance, I am > writing new policy for spamass-milter, which currently shares spamd_t > with spamassassin. I need spamass-milter to transition into a different > domain, so I need to specify a new context for /usr/bin/spamass-milter > in my policy module. This conflicts with the existing context for the > same file (spamd_exec_t) in the main selinux-policy-targeted package and > I get warnings like this on most rpm/selinux operations: > > /etc/selinux/targeted/contexts/files/file_contexts: Multiple different > specifications for /usr/sbin/spamass-milter > (system_u:object_r:milter_spamass_exec_t:s0 and > system_u:object_r:spamd_exec_t:s0). > > For whatever reason, the context from my local module "wins" and I get > the desired result. However, if semanage didn't allow this, I believe > I'd need to fork the selinux-policy package for the duration of my > development to prevent the unwanted context specification from being > used. Or is there some other way around this? > > Paul. Paul after discussions at the OLS/SELinux mini summit. Karl MacMillan had an interesting talk about how we are perhaps going to far with Least Priv. And the idea of common security goals came up. He was talking about grouping applications with common security constraints into a greater policy, this would eliminate lots of potential bugs without decreasing security that greatly. The best example of this in my opinion has been our treatment of spam. We have multiple packages within Fedora/Debian whose main goals are the prevention of spam email reaching the user. Spamassassin, razor, pyzor/spamass-milter probably more. Each one of these has a written policy for it and interacts with the multiple email clients. These have caused a huge amount of AVCs/Bugzilla's. And they all pretty much have the same security problems. We want to prevent either a daemon or a user app from reading untrusted mail and then doing something bad. So in Rawhide I have worked to consolodate these policies into two types spamd_t and a spamc_t policy. Where the difference is a spam daemon or spam client application. Then we can begin collapsing the rules and labeling down to the directories that all of the similar apps need to use. Hopefully this will simplify the policy and eliminate the breakage we currently have in handling spam. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkiXP3AACgkQrlYvE4MpobOLsACfe0oxR0yX7+eEnBruoUsjjkif dnkAn1bbZkPT7FT0kUYR6OO7haN8C4Va =dcuN -----END PGP SIGNATURE----- -- 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] 19+ messages in thread
* Re: Patch to make libselinux shut up when SELinux is disabled. 2008-08-04 17:42 ` Daniel J Walsh @ 2008-08-05 13:50 ` Paul Howarth 0 siblings, 0 replies; 19+ messages in thread From: Paul Howarth @ 2008-08-05 13:50 UTC (permalink / raw) To: Daniel J Walsh Cc: Stephen Smalley, SE Linux, Christopher J. PeBenito, Joshua Brindle Daniel J Walsh wrote: > Paul after discussions at the OLS/SELinux mini summit. Karl MacMillan > had an interesting talk about how we are perhaps going to far with Least > Priv. And the idea of common security goals came up. He was talking > about grouping applications with common security constraints into a > greater policy, this would eliminate lots of potential bugs without > decreasing security that greatly. The best example of this in my > opinion has been our treatment of spam. We have multiple packages > within Fedora/Debian whose main goals are the prevention of spam email > reaching the user. Spamassassin, razor, pyzor/spamass-milter probably > more. Each one of these has a written policy for it and interacts with > the multiple email clients. These have caused a huge amount of > AVCs/Bugzilla's. And they all pretty much have the same security > problems. We want to prevent either a daemon or a user app from reading > untrusted mail and then doing something bad. So in Rawhide I have > worked to consolodate these policies into two types spamd_t and a > spamc_t policy. Where the difference is a spam daemon or spam client > application. Then we can begin collapsing the rules and labeling down > to the directories that all of the similar apps need to use. Hopefully > this will simplify the policy and eliminate the breakage we currently > have in handling spam. This is interesting in that the DAC world seems to have been moving in the opposite direction in recent years, such as the split of sendmail into separate daemon (not suid root) and client with 8.12, openssh adding privilege separation in a small helper etc. Anyway, what I'm doing with spamass-milter probably ties in reasonably well with the Rawhide approach. The milter sits in a sort of middle ground between being a daemon and a client. It's a daemon in that it's directly involved in SMTP processing with the MTA, but as far as spamassassin is concerned, it's a client, calling spamc just like any other client. The only thing directly spam-related in the spamass-milter policy is the call to the spamassassin_domtrans_spamc() interface, so I'm leveraging the work already done for regular SA usage. All of the other stuff is milter-specific and not generally useful to other spam-attacking applications I think, though there's a lot of commonality with other milter applications, hence the template approach I'm using. Paul. -- 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] 19+ messages in thread
* Re: Patch to make libselinux shut up when SELinux is disabled. 2008-08-04 15:06 ` Paul Howarth 2008-08-04 17:42 ` Daniel J Walsh @ 2008-08-04 17:51 ` Stephen Smalley 1 sibling, 0 replies; 19+ messages in thread From: Stephen Smalley @ 2008-08-04 17:51 UTC (permalink / raw) To: Paul Howarth Cc: Daniel J Walsh, SE Linux, Christopher J. PeBenito, Joshua Brindle On Mon, 2008-08-04 at 16:06 +0100, Paul Howarth wrote: > This may present a problem for policy developers. For instance, I am > writing new policy for spamass-milter, which currently shares spamd_t > with spamassassin. I need spamass-milter to transition into a different > domain, so I need to specify a new context for /usr/bin/spamass-milter > in my policy module. This conflicts with the existing context for the > same file (spamd_exec_t) in the main selinux-policy-targeted package and > I get warnings like this on most rpm/selinux operations: > > /etc/selinux/targeted/contexts/files/file_contexts: Multiple different > specifications for /usr/sbin/spamass-milter > (system_u:object_r:milter_spamass_exec_t:s0 and > system_u:object_r:spamd_exec_t:s0). > > For whatever reason, the context from my local module "wins" and I get > the desired result. However, if semanage didn't allow this, I believe > I'd need to fork the selinux-policy package for the duration of my > development to prevent the unwanted context specification from being > used. Or is there some other way around this? If that is the desired behavior, then I suppose we want to move up the nodups_specs checking. Like this: Index: trunk/libselinux/src/label_file.c =================================================================== --- trunk/libselinux/src/label_file.c (revision 2938) +++ trunk/libselinux/src/label_file.c (working copy) @@ -468,6 +468,11 @@ pass, ++lineno) != 0) goto finish; } + if (pass == 1) { + status = nodups_specs(data, path); + if (status) + goto finish; + } lineno = 0; if (homedirfp) while (getline(&line_buf, &line_len, homedirfp) > 0 @@ -507,10 +512,6 @@ } free(line_buf); - status = nodups_specs(data, path); - if (status) - goto finish; - /* Move exact pathname specifications to the end. */ spec_copy = malloc(sizeof(spec_t) * data->nspec); if (!spec_copy) -- 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] 19+ messages in thread
end of thread, other threads:[~2008-08-05 13:50 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-07-18 17:16 Patch to make libselinux shut up when SELinux is disabled Daniel J Walsh 2008-07-18 17:27 ` Stephen Smalley 2008-07-18 17:40 ` Daniel J Walsh 2008-07-18 17:54 ` Stephen Smalley 2008-07-18 18:34 ` [rfc][patch] setfiles: validate all file_contexts files when using -c Stephen Smalley 2008-07-18 18:37 ` [rfc][patch] setfiles: validate all file_contexts files whenusing -c Joshua Brindle 2008-07-18 18:47 ` Stephen Smalley 2008-07-18 19:09 ` [rfc][patch] setfiles: validate all file_contexts fileswhenusing -c Joshua Brindle 2008-07-18 19:10 ` Stephen Smalley 2008-07-18 19:12 ` [rfc][patch] setfiles: validate all file_contextsfileswhenusing -c Joshua Brindle 2008-07-18 19:23 ` Stephen Smalley 2008-07-18 19:01 ` [patch] libselinux: handle conflicting file contexts as a fatal error Stephen Smalley 2008-07-18 19:04 ` Daniel J Walsh 2008-07-18 19:09 ` [patch v2] libselinux: handle duplicate file context entries " Stephen Smalley 2008-07-18 18:50 ` Patch to make libselinux shut up when SELinux is disabled Daniel J Walsh 2008-08-04 15:06 ` Paul Howarth 2008-08-04 17:42 ` Daniel J Walsh 2008-08-05 13:50 ` Paul Howarth 2008-08-04 17:51 ` Stephen Smalley
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.