Patch to make libselinux shut up when SELinux is disabled.

Patch to make libselinux shut up when SELinux is disabled.

[Daniel J Walsh]

[-- 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 --]

Re: Patch to make libselinux shut up when SELinux is disabled.

[Stephen Smalley]

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.

Re: Patch to make libselinux shut up when SELinux is disabled.

[Daniel J Walsh]

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

Re: Patch to make libselinux shut up when SELinux is disabled.

[Stephen Smalley]

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.

[rfc][patch] setfiles: validate all file_contexts files when using -c

[Stephen Smalley]

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.

RE: [rfc][patch] setfiles: validate all file_contexts files whenusing -c

[Joshua Brindle]

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.

RE: [rfc][patch] setfiles: validate all file_contexts files whenusing -c

[Stephen Smalley]

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.

Re: Patch to make libselinux shut up when SELinux is disabled.

[Daniel J Walsh]

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.

[patch] libselinux: handle conflicting file contexts as a fatal error

[Stephen Smalley]

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.

Re: [patch] libselinux: handle conflicting file contexts as a fatal error

[Daniel J Walsh]

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.

[patch v2] libselinux: handle duplicate file context entries as a fatal error

[Stephen Smalley]

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.

RE: [rfc][patch] setfiles: validate all file_contexts fileswhenusing -c

[Joshua Brindle]

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.

RE: [rfc][patch] setfiles: validate all file_contexts fileswhenusing -c

[Stephen Smalley]

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.

RE: [rfc][patch] setfiles: validate all file_contextsfileswhenusing -c

[Joshua Brindle]

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.

RE: [rfc][patch] setfiles: validate all file_contextsfileswhenusing -c

[Stephen Smalley]

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.

Re: Patch to make libselinux shut up when SELinux is disabled.

[Paul Howarth]

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.

Re: Patch to make libselinux shut up when SELinux is disabled.

[Daniel J Walsh]

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

Re: Patch to make libselinux shut up when SELinux is disabled.

[Stephen Smalley]

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.

Re: Patch to make libselinux shut up when SELinux is disabled.

[Paul Howarth]

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.