[PATCH 3/3] Separate local file contexts into file_contexts.local

[PATCH 3/3] Separate local file contexts into file_contexts.local

[Christopher J. PeBenito]

Semanage was looking at the main file contexts in some cases, since the
local file contexts were merged into the main file contexts.  This patch
fixes the code to look at the appropriate file context databases.

 policycoreutils/semanage/seobject.py |   24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --exclude .svn -urN trunk/policycoreutils/semanage/seobject.py fc-local-fix/policycoreutils/semanage/seobject.py
--- trunk/policycoreutils/semanage/seobject.py	2006-07-31 16:41:07.033676000 -0400
+++ fc-local-fix/policycoreutils/semanage/seobject.py	2006-08-10 09:57:09.205707250 -0400
@@ -1085,13 +1085,13 @@
 		if rc < 0:
 			raise ValueError(_("Could not create a key for %s") % target)
 
-		(rc,exists) = semanage_fcontext_exists(self.sh, k)
+		(rc,exists) = semanage_fcontext_exists_local(self.sh, k)
 		if rc < 0:
 			raise ValueError(_("Could not check if file context for %s is defined") % target)
 		if not exists:
 			raise ValueError(_("File context for %s is not defined") % target)
 		
-		(rc,fcontext) = semanage_fcontext_query(self.sh, k)
+		(rc,fcontext) = semanage_fcontext_query_local(self.sh, k)
 		if rc < 0:
 			raise ValueError(_("Could not query file context for %s") % target)
 
@@ -1124,17 +1124,17 @@
 		if rc < 0:
 			raise ValueError(_("Could not create a key for %s") % target)
 
-		(rc,exists) = semanage_fcontext_exists(self.sh, k)
-		if rc < 0:
-			raise ValueError(_("Could not check if file context for %s is defined") % target)
-		if not exists:
-			raise ValueError(_("File context for %s is not defined") % target)
-		
 		(rc,exists) = semanage_fcontext_exists_local(self.sh, k)
 		if rc < 0:
 			raise ValueError(_("Could not check if file context for %s is defined") % target)
 		if not exists:
-			raise ValueError(_("File context for %s is defined in policy, cannot be deleted") % target)
+			(rc,exists) = semanage_fcontext_exists(self.sh, k)
+			if rc < 0:
+				raise ValueError(_("Could not check if file context for %s is defined") % target)
+			if exists:
+				raise ValueError(_("File context for %s is defined in policy, cannot be deleted") % target)
+			else:
+				raise ValueError(_("File context for %s is not defined") % target)
 
 		rc = semanage_begin_transaction(self.sh)
 		if rc < 0:
@@ -1156,6 +1156,12 @@
 		if rc < 0:
 			raise ValueError(_("Could not list file contexts"))
 
+		(rc, fclocal) = semanage_fcontext_list_local(self.sh)
+		if rc < 0:
+			raise ValueError(_("Could not list local file contexts"))
+
+		self.flist += fclocal
+
 		for fcontext in self.flist:
 			expr = semanage_fcontext_get_expr(fcontext)
 			ftype = semanage_fcontext_get_type(fcontext)


-- 
Chris PeBenito
Tresys Technology, LLC
(410) 290-1411 x150


--
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 3/3] Separate local file contexts into file_contexts.local

[Stephen Smalley]

On Fri, 2006-08-11 at 09:34 -0400, Christopher J. PeBenito wrote:
> Semanage was looking at the main file contexts in some cases, since the
> local file contexts were merged into the main file contexts.  This patch
> fixes the code to look at the appropriate file context databases.

Thanks, merged, although I did make the libsemanage copy file error
non-fatal (another patch has made several others non-fatal too).

Also, I'm not sure that semanage handles conflicts very well presently,
e.g.
	semanage fcontext -a -t login_exec_t '/bin/bash'

-- 
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 3/3] Separate local file contexts into file_contexts.local

[Joshua Brindle]

Stephen Smalley wrote:
> On Fri, 2006-08-11 at 09:34 -0400, Christopher J. PeBenito wrote:
>> Semanage was looking at the main file contexts in some cases, since the
>> local file contexts were merged into the main file contexts.  This patch
>> fixes the code to look at the appropriate file context databases.
> 
> Thanks, merged, although I did make the libsemanage copy file error
> non-fatal (another patch has made several others non-fatal too).
> 

I never saw the other patch, did it not get sent to the list? I have 
some concerns about making those non-fatal, as I already told Karl. 
Leaving labeling files in an inconsistent state on policy update is an 
easy to way to break your system and with no automated reconciliation 
(eg., install the new policy, let it fix the permissions problems then 
reinstall it again automatically to make the labeling files consistent) 
I have serious doubts that the consistency will get fixed. Bailing on a 
policy update is probably better than killing the users network 
connection (for example) where they won't even be able to use the 
internet to find the solution.

Not to mention, anything being installed after the policy (in an rpm 
transaction) will be using the wrong file_contexts file for some amount 
of time before a user even could intervene and reinstall the policy to 
update the context files.

If these issues are fine with others (Karl indicated they were better 
than the alternative in his opinion) thats fine but I want them noted.

I honestly believe they'll create more problems than they will solve. It 
wasn't arbitrary that we made all those errors fatal, we had a system in 
place to be able to back out of a policy install when something that 
would leave the system in an inconsistent state happened.

> Also, I'm not sure that semanage handles conflicts very well presently,
> e.g.
> 	semanage fcontext -a -t login_exec_t '/bin/bash'
> 

we'll check on that.

--
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 3/3] Separate local file contexts into file_contexts.local

[Stephen Smalley]

On Sat, 2006-08-12 at 09:21 -0400, Joshua Brindle wrote:
> Stephen Smalley wrote:
> > On Fri, 2006-08-11 at 09:34 -0400, Christopher J. PeBenito wrote:
> >> Semanage was looking at the main file contexts in some cases, since the
> >> local file contexts were merged into the main file contexts.  This patch
> >> fixes the code to look at the appropriate file context databases.
> > 
> > Thanks, merged, although I did make the libsemanage copy file error
> > non-fatal (another patch has made several others non-fatal too).
> > 
> 
> I never saw the other patch, did it not get sent to the list? I have 
> some concerns about making those non-fatal, as I already told Karl. 
> Leaving labeling files in an inconsistent state on policy update is an 
> easy to way to break your system and with no automated reconciliation 
> (eg., install the new policy, let it fix the permissions problems then 
> reinstall it again automatically to make the labeling files consistent) 
> I have serious doubts that the consistency will get fixed. Bailing on a 
> policy update is probably better than killing the users network 
> connection (for example) where they won't even be able to use the 
> internet to find the solution.
> 
> Not to mention, anything being installed after the policy (in an rpm 
> transaction) will be using the wrong file_contexts file for some amount 
> of time before a user even could intervene and reinstall the policy to 
> update the context files.
> 
> If these issues are fine with others (Karl indicated they were better 
> than the alternative in his opinion) thats fine but I want them noted.
> 
> I honestly believe they'll create more problems than they will solve. It 
> wasn't arbitrary that we made all those errors fatal, we had a system in 
> place to be able to back out of a policy install when something that 
> would leave the system in an inconsistent state happened.

Possibly, although both the netfilter_contexts and now the
file_contexts.local changes have run into such errors in rawhide.  In
fact, you need to get the kernel policy update into place to address the
netfilter_contexts problem, so if the inability to install
netfilter_contexts prevents you from completing the transaction, how do
you make forward progress w/o going permissive?

I do agree that a partial write of any of the files should abort the
transaction, so we should distinguish that case in the error handling.
But if the source file is not present, that shouldn't be fatal.

Also, I think libsemanage is too dependent on pre-setup by the policy
package, e.g. if you don't already have
a /etc/selinux/$SELINUXTYPE/policy
and /etc/selinux/$SELINUXTYPE/contexts/files directory, it doesn't
create them for you.

-- 
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 3/3] Separate local file contexts into file_contexts.local

[Stephen Smalley]

On Mon, 2006-08-14 at 10:38 -0400, Stephen Smalley wrote:
> On Sat, 2006-08-12 at 09:21 -0400, Joshua Brindle wrote:
> > Stephen Smalley wrote:
> > > On Fri, 2006-08-11 at 09:34 -0400, Christopher J. PeBenito wrote:
> > >> Semanage was looking at the main file contexts in some cases, since the
> > >> local file contexts were merged into the main file contexts.  This patch
> > >> fixes the code to look at the appropriate file context databases.
> > > 
> > > Thanks, merged, although I did make the libsemanage copy file error
> > > non-fatal (another patch has made several others non-fatal too).
> > > 
> > 
> > I never saw the other patch, did it not get sent to the list? I have 
> > some concerns about making those non-fatal, as I already told Karl. 
> > Leaving labeling files in an inconsistent state on policy update is an 
> > easy to way to break your system and with no automated reconciliation 
> > (eg., install the new policy, let it fix the permissions problems then 
> > reinstall it again automatically to make the labeling files consistent) 
> > I have serious doubts that the consistency will get fixed. Bailing on a 
> > policy update is probably better than killing the users network 
> > connection (for example) where they won't even be able to use the 
> > internet to find the solution.
> > 
> > Not to mention, anything being installed after the policy (in an rpm 
> > transaction) will be using the wrong file_contexts file for some amount 
> > of time before a user even could intervene and reinstall the policy to 
> > update the context files.
> > 
> > If these issues are fine with others (Karl indicated they were better 
> > than the alternative in his opinion) thats fine but I want them noted.
> > 
> > I honestly believe they'll create more problems than they will solve. It 
> > wasn't arbitrary that we made all those errors fatal, we had a system in 
> > place to be able to back out of a policy install when something that 
> > would leave the system in an inconsistent state happened.
> 
> Possibly, although both the netfilter_contexts and now the
> file_contexts.local changes have run into such errors in rawhide.  In
> fact, you need to get the kernel policy update into place to address the
> netfilter_contexts problem, so if the inability to install
> netfilter_contexts prevents you from completing the transaction, how do
> you make forward progress w/o going permissive?
> 
> I do agree that a partial write of any of the files should abort the
> transaction, so we should distinguish that case in the error handling.
> But if the source file is not present, that shouldn't be fatal.

Patch below restores the errors to being fatal, with exceptions being
granted for an ENOENT for file_contexts.local (optional), seusers (was
already non-fatal and optional), and netfilter_contexts (may not exist
in all policies, e.g. legacy ones and ones that are using compat_net=1).
Acceptable to all parties?

Index: libsemanage/src/semanage_store.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/semanage_store.c,v
retrieving revision 1.56
diff -u -p -r1.56 semanage_store.c
--- libsemanage/src/semanage_store.c	11 Aug 2006 20:03:44 -0000	1.56
+++ libsemanage/src/semanage_store.c	14 Aug 2006 18:40:53 -0000
@@ -1057,37 +1057,33 @@ static int semanage_install_active(seman
 
 	snprintf(store_hd, PATH_MAX, "%s%s", storepath, running_hd);
 	if (semanage_copy_file(active_hd, store_hd, sh->conf->file_mode) == -1) {
-		INFO(sh, "Non-fatal error:  Could not copy %s to %s.",
-		     active_hd, store_hd);
-		/* Non-fatal; fall through */
+		ERR(sh, "Could not copy %s to %s.", active_hd, store_hd);
+		goto cleanup;
 	}
 
 	snprintf(store_fc, PATH_MAX, "%s%s", storepath, running_fc);
 	if (semanage_copy_file(active_fc, store_fc, sh->conf->file_mode) == -1) {
-		INFO(sh, "Non-fatal error:  Could not copy %s to %s.",
-		     active_fc, store_fc);
-		/* Non-fatal; fall through */
+		ERR(sh, "Could not copy %s to %s.", active_fc, store_fc);
+		goto cleanup;
 	}
 
 	snprintf(store_fc_loc, PATH_MAX, "%s%s", storepath, running_fc_loc);
-	if (semanage_copy_file(active_fc_loc, store_fc_loc, sh->conf->file_mode) == -1) {
-		INFO(sh, "Non-fatal error:  Could not copy %s to %s.",
-		     active_fc_loc, store_fc_loc);
-		/* Non-fatal; fall through */
+	if (semanage_copy_file(active_fc_loc, store_fc_loc, sh->conf->file_mode) == -1 && errno != ENOENT) {
+		ERR(sh, "Could not copy %s to %s.", active_fc_loc, store_fc_loc);
+		goto cleanup;
 	}
 
 	snprintf(store_seusers, PATH_MAX, "%s%s", storepath, running_seusers);
 	if (semanage_copy_file
-	    (active_seusers, store_seusers, sh->conf->file_mode) == -1) {
-		INFO(sh, "Non-fatal error:  Could not copy %s to %s.",
-		     active_seusers, store_seusers);
-		/* Non-fatal; fall through */
+	    (active_seusers, store_seusers, sh->conf->file_mode) == -1 && errno != ENOENT) {
+		ERR(sh, "Could not copy %s to %s.", active_seusers, store_seusers);
+		goto cleanup;
 	}
 
 	snprintf(store_nc, PATH_MAX, "%s%s", storepath, running_nc);
-	if (semanage_copy_file(active_nc, store_nc, sh->conf->file_mode) == -1) {
-		INFO(sh, "Non-fatal error:  Could not copy %s to %s.", active_nc, store_nc);
-		/* Non-fatal; fall through */
+	if (semanage_copy_file(active_nc, store_nc, sh->conf->file_mode) == -1 && errno != ENOENT) {
+		ERR(sh, "Could not copy %s to %s.", active_nc, store_nc);
+		goto cleanup;
 	}
 
 	if (!sh->do_reload)


-- 
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 3/3] Separate local file contextsinto file_contexts.local

[Joshua Brindle]

> From: Stephen Smalley [mailto:sds@tycho.nsa.gov] 
> 
> On Mon, 2006-08-14 at 10:38 -0400, Stephen Smalley wrote:
> > On Sat, 2006-08-12 at 09:21 -0400, Joshua Brindle wrote:
> > > Stephen Smalley wrote:
> > > > On Fri, 2006-08-11 at 09:34 -0400, Christopher J. 
> PeBenito wrote:
> 
> Patch below restores the errors to being fatal, with 
> exceptions being granted for an ENOENT for 
> file_contexts.local (optional), seusers (was already 
> non-fatal and optional), and netfilter_contexts (may not 
> exist in all policies, e.g. legacy ones and ones that are 
> using compat_net=1).
> Acceptable to all parties?
> 

I'm happy with this but does it fix the permission problem? I thought
the motivation behind the patch was to be able to install the policy
when the policy didn't allow copying of some files? ENOENT will just
check if it doesn't exist won't it?


--
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 3/3] Separate local file contextsinto file_contexts.local

[Stephen Smalley]

On Tue, 2006-08-15 at 19:42 -0400, Joshua Brindle wrote:
> > From: Stephen Smalley [mailto:sds@tycho.nsa.gov] 
> > 
> > On Mon, 2006-08-14 at 10:38 -0400, Stephen Smalley wrote:
> > > On Sat, 2006-08-12 at 09:21 -0400, Joshua Brindle wrote:
> > > > Stephen Smalley wrote:
> > > > > On Fri, 2006-08-11 at 09:34 -0400, Christopher J. 
> > PeBenito wrote:
> > 
> > Patch below restores the errors to being fatal, with 
> > exceptions being granted for an ENOENT for 
> > file_contexts.local (optional), seusers (was already 
> > non-fatal and optional), and netfilter_contexts (may not 
> > exist in all policies, e.g. legacy ones and ones that are 
> > using compat_net=1).
> > Acceptable to all parties?
> > 
> 
> I'm happy with this but does it fix the permission problem? I thought
> the motivation behind the patch was to be able to install the policy
> when the policy didn't allow copying of some files? ENOENT will just
> check if it doesn't exist won't it?

I decided that it wasn't adequate justification, since:
a) the same kind of permission problem could occur on the kernel policy
file too, at which point we have to go permissive to recover,
b) such a denial would reflect a bug in policy, and there is no general
way to recover from bugs in policy without going permissive,
c) all such issues would presumably be caught and resolved in
development and never hit a production system.

So if everyone is satisfied with the patch, I'll go ahead and commit it.

-- 
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 3/3] Separate local file contextsinto file_contexts.local

[Karl MacMillan]

On Wed, 2006-08-16 at 08:41 -0400, Stephen Smalley wrote:
> On Tue, 2006-08-15 at 19:42 -0400, Joshua Brindle wrote:
> > > From: Stephen Smalley [mailto:sds@tycho.nsa.gov] 
> > > 
> > > On Mon, 2006-08-14 at 10:38 -0400, Stephen Smalley wrote:
> > > > On Sat, 2006-08-12 at 09:21 -0400, Joshua Brindle wrote:
> > > > > Stephen Smalley wrote:
> > > > > > On Fri, 2006-08-11 at 09:34 -0400, Christopher J. 
> > > PeBenito wrote:
> > > 
> > > Patch below restores the errors to being fatal, with 
> > > exceptions being granted for an ENOENT for 
> > > file_contexts.local (optional), seusers (was already 
> > > non-fatal and optional), and netfilter_contexts (may not 
> > > exist in all policies, e.g. legacy ones and ones that are 
> > > using compat_net=1).
> > > Acceptable to all parties?
> > > 
> > 
> > I'm happy with this but does it fix the permission problem? I thought
> > the motivation behind the patch was to be able to install the policy
> > when the policy didn't allow copying of some files? ENOENT will just
> > check if it doesn't exist won't it?
> 
> I decided that it wasn't adequate justification, since:
> a) the same kind of permission problem could occur on the kernel policy
> file too, at which point we have to go permissive to recover,
> b) such a denial would reflect a bug in policy, and there is no general
> way to recover from bugs in policy without going permissive,
> c) all such issues would presumably be caught and resolved in
> development and never hit a production system.
> 
> So if everyone is satisfied with the patch, I'll go ahead and commit it.
> 

It's fine with me since you and Josh are in agreement about this.

Karl

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