Patch for restorecond to not report an error if filesystem does not support XATTR

Patch for restorecond to not report an error if filesystem does not support XATTR

[Daniel J Walsh]

[-- Attachment #1: Type: text/plain, Size: 34 bytes --]

Caused by NFS Home directories.



[-- Attachment #2: restorecond-eopnotsupp.patch --]
[-- Type: text/x-patch, Size: 1108 bytes --]

diff --exclude-from=exclude --exclude POTFILES.in --exclude='*.po' --exclude='*.pot' -N -u -r nsapolicycoreutils/restorecond/restorecond.c policycoreutils-1.33.5/restorecond/restorecond.c
--- nsapolicycoreutils/restorecond/restorecond.c	2006-11-16 17:14:28.000000000 -0500
+++ policycoreutils-1.33.5/restorecond/restorecond.c	2006-11-29 12:44:06.000000000 -0500
@@ -210,9 +210,10 @@
 			}
 
 			if (fsetfilecon(fd, scontext) < 0) {
-				syslog(LOG_ERR,
-				       "set context %s->%s failed:'%s'\n",
-				       filename, scontext, strerror(errno));
+				if (errno != EOPNOTSUPP) 
+					syslog(LOG_ERR,
+					       "set context %s->%s failed:'%s'\n",
+					       filename, scontext, strerror(errno));
 				if (retcontext >= 0)
 					free(prev_context);
 				free(scontext);
@@ -225,8 +226,9 @@
 		if (retcontext >= 0)
 			free(prev_context);
 	} else {
-		syslog(LOG_ERR, "get context on %s failed: '%s'\n",
-		       filename, strerror(errno));
+		if (errno != EOPNOTSUPP) 
+			syslog(LOG_ERR, "get context on %s failed: '%s'\n",
+			       filename, strerror(errno));
 	}
 	free(scontext);
 	close(fd);

Re: Patch for restorecond to not report an error if filesystem does not support XATTR

[Joshua Brindle]

Daniel J Walsh wrote:
> Caused by NFS Home directories.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> diff --exclude-from=exclude --exclude POTFILES.in --exclude='*.po' --exclude='*.pot' -N -u -r nsapolicycoreutils/restorecond/restorecond.c policycoreutils-1.33.5/restorecond/restorecond.c
> --- nsapolicycoreutils/restorecond/restorecond.c	2006-11-16 17:14:28.000000000 -0500
> +++ policycoreutils-1.33.5/restorecond/restorecond.c	2006-11-29 12:44:06.000000000 -0500
> @@ -210,9 +210,10 @@
>  			}
>  
>  			if (fsetfilecon(fd, scontext) < 0) {
> -				syslog(LOG_ERR,
> -				       "set context %s->%s failed:'%s'\n",
> -				       filename, scontext, strerror(errno));
> +				if (errno != EOPNOTSUPP) 
> +					syslog(LOG_ERR,
> +					       "set context %s->%s failed:'%s'\n",
> +					       filename, scontext, strerror(errno));
>  				if (retcontext >= 0)
>  					free(prev_context);
>  				free(scontext);
> @@ -225,8 +226,9 @@
>  		if (retcontext >= 0)
>  			free(prev_context);
>  	} else {
> -		syslog(LOG_ERR, "get context on %s failed: '%s'\n",
> -		       filename, strerror(errno));
> +		if (errno != EOPNOTSUPP) 
> +			syslog(LOG_ERR, "get context on %s failed: '%s'\n",
> +			       filename, strerror(errno));
>  	}
>  	free(scontext);
>  	close(fd);

It sounds like this is caused by configuration errors, why wouldn't it 
check that the files/dirs being watched have xattr support before adding 
them to the watch list?


--
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 for restorecond to not report an error if filesystem does not support XATTR

[Daniel J Walsh]

Joshua Brindle wrote:
> Daniel J Walsh wrote:
>> Caused by NFS Home directories.
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> diff --exclude-from=exclude --exclude POTFILES.in --exclude='*.po' 
>> --exclude='*.pot' -N -u -r 
>> nsapolicycoreutils/restorecond/restorecond.c 
>> policycoreutils-1.33.5/restorecond/restorecond.c
>> --- nsapolicycoreutils/restorecond/restorecond.c    2006-11-16 
>> 17:14:28.000000000 -0500
>> +++ policycoreutils-1.33.5/restorecond/restorecond.c    2006-11-29 
>> 12:44:06.000000000 -0500
>> @@ -210,9 +210,10 @@
>>              }
>>  
>>              if (fsetfilecon(fd, scontext) < 0) {
>> -                syslog(LOG_ERR,
>> -                       "set context %s->%s failed:'%s'\n",
>> -                       filename, scontext, strerror(errno));
>> +                if (errno != EOPNOTSUPP) +                    
>> syslog(LOG_ERR,
>> +                           "set context %s->%s failed:'%s'\n",
>> +                           filename, scontext, strerror(errno));
>>                  if (retcontext >= 0)
>>                      free(prev_context);
>>                  free(scontext);
>> @@ -225,8 +226,9 @@
>>          if (retcontext >= 0)
>>              free(prev_context);
>>      } else {
>> -        syslog(LOG_ERR, "get context on %s failed: '%s'\n",
>> -               filename, strerror(errno));
>> +        if (errno != EOPNOTSUPP) +            syslog(LOG_ERR, "get 
>> context on %s failed: '%s'\n",
>> +                   filename, strerror(errno));
>>      }
>>      free(scontext);
>>      close(fd);
>
> It sounds like this is caused by configuration errors, why wouldn't it 
> check that the files/dirs being watched have xattr support before 
> adding them to the watch list?
>
I guess we can check before we add the directory to watch.  How do you 
check if a directory supports file_context without setting the file context?



--
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 for restorecond to not report an error if filesystem does not support XATTR

[Joshua Brindle]

> From: Daniel J Walsh [mailto:dwalsh@redhat.com] 
> 
> Joshua Brindle wrote:
> > Daniel J Walsh wrote:
> >> Caused by NFS Home directories.
> >>
> ---------------------------------------------------------------------
> >> ---
> >>
> >> diff --exclude-from=exclude --exclude POTFILES.in --exclude='*.po' 
> >> --exclude='*.pot' -N -u -r
> >> nsapolicycoreutils/restorecond/restorecond.c
> >> policycoreutils-1.33.5/restorecond/restorecond.c
> >> --- nsapolicycoreutils/restorecond/restorecond.c    2006-11-16 
> >> 17:14:28.000000000 -0500
> >> +++ policycoreutils-1.33.5/restorecond/restorecond.c    2006-11-29 
> >> 12:44:06.000000000 -0500
> >> @@ -210,9 +210,10 @@
> >>              }
> >>  
> >>              if (fsetfilecon(fd, scontext) < 0) {
> >> -                syslog(LOG_ERR,
> >> -                       "set context %s->%s failed:'%s'\n",
> >> -                       filename, scontext, strerror(errno));
> >> +                if (errno != EOPNOTSUPP) +                    
> >> syslog(LOG_ERR,
> >> +                           "set context %s->%s failed:'%s'\n",
> >> +                           filename, scontext, strerror(errno));
> >>                  if (retcontext >= 0)
> >>                      free(prev_context);
> >>                  free(scontext);
> >> @@ -225,8 +226,9 @@
> >>          if (retcontext >= 0)
> >>              free(prev_context);
> >>      } else {
> >> -        syslog(LOG_ERR, "get context on %s failed: '%s'\n",
> >> -               filename, strerror(errno));
> >> +        if (errno != EOPNOTSUPP) +            
> syslog(LOG_ERR, "get 
> >> context on %s failed: '%s'\n",
> >> +                   filename, strerror(errno));
> >>      }
> >>      free(scontext);
> >>      close(fd);
> >
> > It sounds like this is caused by configuration errors, why 
> wouldn't it 
> > check that the files/dirs being watched have xattr support before 
> > adding them to the watch list?
> >
> I guess we can check before we add the directory to watch.  
> How do you check if a directory supports file_context without 
> setting the file context?
> 

Good question, this may be yet-another-thing messed up by the
canonicalized selinux contexts since getxattr will return the actual
label and not necessarilly the one on disk (or an error if the
filesystem doesn't support it).

Stephen might know a better way to figure out if a filesystem supports
xattr labeling from userland, otherwise I guess the above patch is
better than flooding the logs (though it won't give the user an
indication why restorecond isn't doing what he thinks it should be), I'd
really rather us figure out a way to report a configuration error to the
user than hide the problem.


--
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 for restorecond to not report an error if filesystem does not support XATTR

[Stephen Smalley]

On Thu, 2006-11-30 at 13:15 -0500, Joshua Brindle wrote:
> > From: Daniel J Walsh [mailto:dwalsh@redhat.com] 
> > 
> > Joshua Brindle wrote:
> > > Daniel J Walsh wrote:
> > >> Caused by NFS Home directories.
> > >>
> > ---------------------------------------------------------------------
> > >> ---
> > >>
> > >> diff --exclude-from=exclude --exclude POTFILES.in --exclude='*.po' 
> > >> --exclude='*.pot' -N -u -r
> > >> nsapolicycoreutils/restorecond/restorecond.c
> > >> policycoreutils-1.33.5/restorecond/restorecond.c
> > >> --- nsapolicycoreutils/restorecond/restorecond.c    2006-11-16 
> > >> 17:14:28.000000000 -0500
> > >> +++ policycoreutils-1.33.5/restorecond/restorecond.c    2006-11-29 
> > >> 12:44:06.000000000 -0500
> > >> @@ -210,9 +210,10 @@
> > >>              }
> > >>  
> > >>              if (fsetfilecon(fd, scontext) < 0) {
> > >> -                syslog(LOG_ERR,
> > >> -                       "set context %s->%s failed:'%s'\n",
> > >> -                       filename, scontext, strerror(errno));
> > >> +                if (errno != EOPNOTSUPP) +                    
> > >> syslog(LOG_ERR,
> > >> +                           "set context %s->%s failed:'%s'\n",
> > >> +                           filename, scontext, strerror(errno));
> > >>                  if (retcontext >= 0)
> > >>                      free(prev_context);
> > >>                  free(scontext);
> > >> @@ -225,8 +226,9 @@
> > >>          if (retcontext >= 0)
> > >>              free(prev_context);
> > >>      } else {
> > >> -        syslog(LOG_ERR, "get context on %s failed: '%s'\n",
> > >> -               filename, strerror(errno));
> > >> +        if (errno != EOPNOTSUPP) +            
> > syslog(LOG_ERR, "get 
> > >> context on %s failed: '%s'\n",
> > >> +                   filename, strerror(errno));
> > >>      }
> > >>      free(scontext);
> > >>      close(fd);
> > >
> > > It sounds like this is caused by configuration errors, why 
> > wouldn't it 
> > > check that the files/dirs being watched have xattr support before 
> > > adding them to the watch list?
> > >
> > I guess we can check before we add the directory to watch.  
> > How do you check if a directory supports file_context without 
> > setting the file context?
> > 
> 
> Good question, this may be yet-another-thing messed up by the
> canonicalized selinux contexts since getxattr will return the actual
> label and not necessarilly the one on disk (or an error if the
> filesystem doesn't support it).
> 
> Stephen might know a better way to figure out if a filesystem supports
> xattr labeling from userland, otherwise I guess the above patch is
> better than flooding the logs (though it won't give the user an
> indication why restorecond isn't doing what he thinks it should be), I'd
> really rather us figure out a way to report a configuration error to the
> user than hide the problem.

Interesting question...the policy Makefile and the fixfiles script have
always just used a hardcoded list of filesystem types known to support
file security labeling.  And the policy configuration explicitly
specifies the filesystem types for which SELinux tries to use xattrs, as
the mere presence of an xattr handler has never been a sufficient test.
Possibly we should be extracting that list from policy and making it
available for programs that need to know it.

Even testing the result of setxattr for EOPNOTSUPP is not 100%, as
setxattr will also fall back to setting the incore value if a) you pass
permission checks and b) the filesystem provides no setxattr method at
all (NFS happens to provide one, but it only supports ACLs).  So it
could succeed (e.g. devpts nodes) or fail with -EACCES (e.g. proc).

-- 
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 for restorecond to not report an error if filesystem does not support XATTR

[Joshua Brindle]

Stephen Smalley wrote:
> 
> Interesting question...the policy Makefile and the fixfiles script have
> always just used a hardcoded list of filesystem types known to support
> file security labeling.  And the policy configuration explicitly
> specifies the filesystem types for which SELinux tries to use xattrs, as
> the mere presence of an xattr handler has never been a sufficient test.
> Possibly we should be extracting that list from policy and making it
> available for programs that need to know it.
> 
> Even testing the result of setxattr for EOPNOTSUPP is not 100%, as
> setxattr will also fall back to setting the incore value if a) you pass
> permission checks and b) the filesystem provides no setxattr method at
> all (NFS happens to provide one, but it only supports ACLs).  So it
> could succeed (e.g. devpts nodes) or fail with -EACCES (e.g. proc).
> 

Can we make an selinuxfs node that just outputs the list of fs_use_xattr 
filesystems?


--
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 for restorecond to not report an error if filesystem does not support XATTR

[Stephen Smalley]

On Fri, 2006-12-01 at 22:37 -0500, Joshua Brindle wrote:
> Stephen Smalley wrote:
> > 
> > Interesting question...the policy Makefile and the fixfiles script have
> > always just used a hardcoded list of filesystem types known to support
> > file security labeling.  And the policy configuration explicitly
> > specifies the filesystem types for which SELinux tries to use xattrs, as
> > the mere presence of an xattr handler has never been a sufficient test.
> > Possibly we should be extracting that list from policy and making it
> > available for programs that need to know it.
> > 
> > Even testing the result of setxattr for EOPNOTSUPP is not 100%, as
> > setxattr will also fall back to setting the incore value if a) you pass
> > permission checks and b) the filesystem provides no setxattr method at
> > all (NFS happens to provide one, but it only supports ACLs).  So it
> > could succeed (e.g. devpts nodes) or fail with -EACCES (e.g. proc).
> > 
> 
> Can we make an selinuxfs node that just outputs the list of fs_use_xattr 
> filesystems?

That could be done.  But that will require a kernel patch, whereas you
could just have the policy build process emit the list to a config file
and install it somewhere under /etc/selinux/ for reading by restorecond
and fixfiles (and the policy Makefile could generate the list privately
for its own use for the relabel target).

-- 
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 for restorecond to not report an error if filesystem does not support XATTR

[Karl MacMillan]

Daniel J Walsh wrote:
> Caused by NFS Home directories.
> 

In the absence of some mechanism to determine which file systems support 
labeling, do we want to merge this patch or something similar? Would it 
be better to remove files from the watch list if a fatal error is 
encountered?

Karl

> 
> 
> ------------------------------------------------------------------------
> 
> diff --exclude-from=exclude --exclude POTFILES.in --exclude='*.po' --exclude='*.pot' -N -u -r nsapolicycoreutils/restorecond/restorecond.c policycoreutils-1.33.5/restorecond/restorecond.c
> --- nsapolicycoreutils/restorecond/restorecond.c	2006-11-16 17:14:28.000000000 -0500
> +++ policycoreutils-1.33.5/restorecond/restorecond.c	2006-11-29 12:44:06.000000000 -0500
> @@ -210,9 +210,10 @@
>  			}
>  
>  			if (fsetfilecon(fd, scontext) < 0) {
> -				syslog(LOG_ERR,
> -				       "set context %s->%s failed:'%s'\n",
> -				       filename, scontext, strerror(errno));
> +				if (errno != EOPNOTSUPP) 
> +					syslog(LOG_ERR,
> +					       "set context %s->%s failed:'%s'\n",
> +					       filename, scontext, strerror(errno));
>  				if (retcontext >= 0)
>  					free(prev_context);
>  				free(scontext);
> @@ -225,8 +226,9 @@
>  		if (retcontext >= 0)
>  			free(prev_context);
>  	} else {
> -		syslog(LOG_ERR, "get context on %s failed: '%s'\n",
> -		       filename, strerror(errno));
> +		if (errno != EOPNOTSUPP) 
> +			syslog(LOG_ERR, "get context on %s failed: '%s'\n",
> +			       filename, strerror(errno));
>  	}
>  	free(scontext);
>  	close(fd);


--
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 for restorecond to not report an error if filesystem does not support XATTR

[Stephen Smalley]

On Wed, 2006-12-06 at 12:19 -0500, Karl MacMillan wrote:
> Daniel J Walsh wrote:
> > Caused by NFS Home directories.
> > 
> 
> In the absence of some mechanism to determine which file systems support 
> labeling, do we want to merge this patch or something similar? Would it 
> be better to remove files from the watch list if a fatal error is 
> encountered?

As Joshua noted, you could test for the same condition when processing
the configuration before you add the file to the watch list, and issue
an error or warning at that time rather than waiting until you try to
restore the file's context.  Just do an open+fgetfilecon followed by a
fsetfilecon to the same context to check for any errors (not just
EOPNOTSUPP) before adding to the watch list.  Only corner case is ENOENT
if you want to allow the watch list to include files that may not yet
exist; I suppose you could ignore that one at config processing time.

> 
> Karl
> 
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > diff --exclude-from=exclude --exclude POTFILES.in --exclude='*.po' --exclude='*.pot' -N -u -r nsapolicycoreutils/restorecond/restorecond.c policycoreutils-1.33.5/restorecond/restorecond.c
> > --- nsapolicycoreutils/restorecond/restorecond.c	2006-11-16 17:14:28.000000000 -0500
> > +++ policycoreutils-1.33.5/restorecond/restorecond.c	2006-11-29 12:44:06.000000000 -0500
> > @@ -210,9 +210,10 @@
> >  			}
> >  
> >  			if (fsetfilecon(fd, scontext) < 0) {
> > -				syslog(LOG_ERR,
> > -				       "set context %s->%s failed:'%s'\n",
> > -				       filename, scontext, strerror(errno));
> > +				if (errno != EOPNOTSUPP) 
> > +					syslog(LOG_ERR,
> > +					       "set context %s->%s failed:'%s'\n",
> > +					       filename, scontext, strerror(errno));
> >  				if (retcontext >= 0)
> >  					free(prev_context);
> >  				free(scontext);
> > @@ -225,8 +226,9 @@
> >  		if (retcontext >= 0)
> >  			free(prev_context);
> >  	} else {
> > -		syslog(LOG_ERR, "get context on %s failed: '%s'\n",
> > -		       filename, strerror(errno));
> > +		if (errno != EOPNOTSUPP) 
> > +			syslog(LOG_ERR, "get context on %s failed: '%s'\n",
> > +			       filename, strerror(errno));
> >  	}
> >  	free(scontext);
> >  	close(fd);
> 
> 
> --
> 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.
-- 
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.