Bug 171846

Summary: setxattr doesn't appear to work
Product: [Fedora] Fedora Reporter: Nalin Dahyabhai <nalin>
Component: kernelAssignee: James Morris <jmorris>
Status: CLOSED RAWHIDE QA Contact: Brian Brock <bbrock>
Severity: high Docs Contact:
Priority: medium    
Version: rawhideCC: davej, dwalsh, redhat-bugzilla, sdsmall, wtogami
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2005-11-20 20:22:37 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Description Flags
Extend selinuxfs context interface to allow canonicalization of contexts none

Description Nalin Dahyabhai 2005-10-26 23:07:41 UTC
Description of problem:
The setxattr() syscall returns 0, but doesn't seem to do anything.

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:
1. Run "chcon -t shlib_t /lib/security/pam_xauth.so" (or use "lib_t" if its type
is already "shlib_t".
2. Use "ls -lZ" to read the context of the file.
Actual results:
The file context doesn't change.
Running under strace shows this:
setxattr("/lib/security/pam_xauth.so", "security.selinux",
"system_u:object_r:shlib_t:s0", 29, 0) = 0
so it looks like it's a kernel bug.

Expected results:
It should change.

Additional info:
This is on an ext3 filesystem.

Comment 1 Daniel Walsh 2005-10-27 12:29:36 UTC
Seems to be some new feature of the kernel.

On targeted systems shlib_t and lib_t are typealiased in policy.

But there seems to be a new behavior.

ls -lZ log
-rw-r--r--  root     root     system_u:object_r:usr_t          log
 chcon -t shlib_t log
 ls -lZ log
-rw-r--r--  root     root     system_u:object_r:lib_t          log
chcon -t usr_t log
ls -lZ log
-rw-r--r--  root     root     system_u:object_r:usr_t          log

Problem is if you do multiple restorecon over /lib it will always report things
as wrong even when it attempts to clean it up.

Comment 2 Stephen Smalley 2005-10-27 12:35:37 UTC
This is a result of James' patch to allow SELinux to canonicalize the value
returned by getxattr to the incore security context of the inode rather than the
on-disk xattr.  This was originally requested to support MCS (so that the
contexts would get the :s0 suffix even for files that hadn't been relabeled for
MCS, so that a getxattr/setxattr sequence would always work).  It was also
desired to ensure consistency between what userspace sees and what SELinux sees
internally, as that has been a problem in the past (e.g. unlabeled_t vs on-disk
Internally, SELinux always caonicalizes type aliases to the primary type name.

Options are:
- Alter file_contexts for targeted policy to use the primary name,
- Alter restorecon and setfiles to check for aliases (requires a sepol interface
to compare contexts for equivalency).

Comment 3 Stephen Smalley 2005-10-27 15:32:49 UTC
Note:  I'm looking into the latter option, but doing it in matchpathcon instead
(i.e. call sepol to canonicalize the context returned by matchpathcon, so that
setfiles, restorecon, et al will end up with canonical contexts from both
matchpathcon and getfilecon and thus they will match).

Comment 4 Stephen Smalley 2005-10-27 16:06:02 UTC
Alternative approach would be to add an interface to selinuxfs by which the
SELinux module could provide the canonical context string for a given context
string based on the currently loaded policy, and change matchpathcon to use that
interface.  Advantage would be that the selinuxfs interface would ensure that we
are dealing with the same policy as the kernel, avoids the need to allow all
users of matchpathcon to directly read the kernel binary policy file, and avoids
the overhead of parsing the kernel binary policy file on matchpathcon_init.  The
disadvantage is primarily that it requires a kernel patch, and thus will take
longer to get upstream than a change to libsepol  Thoughts?

Comment 5 Daniel Walsh 2005-10-27 16:20:52 UTC
chcon would need to warn about this situation, then.

chcon -t shlib_t /lib/libselinux.so.1
Warning: shlib_t is an alias of lib_t...

Comment 6 Stephen Smalley 2005-10-28 12:35:55 UTC
I'd recommend creating a patch to selinuxfs to extend the semantics of
/selinux/context such that a write/read sequence will validate the provided
context on the write (as is already done) and then return the canonical context
on the read (a transaction op, like the /selinux/create interface).  That won't
break existing users of security_check_context(3), but will allow us to add a
security_canonicalize_context() function to libselinux that uses the same
underlying selinuxfs interface.  Then matchpathcon_init can use this interface
rather than security_check_context() to both validate and canonicalize the
context.  That will avoid spurious warnings from setfiles and restorecon and
ensure that the canonical name is always applied by them (and rpm and anything
else using matchpathcon).  chcon can also use the new interface to check whether
the canonical context differs from the supplied one and can warn in that case if
you like.

I prefer this approach to the experimental patches I posted on selinux list for
libsepol and libselinux for the reasons stated in comment #4, i.e. it ensures
that we are dealing with the same policy as the kernel, avoids the need to allow
all users of matchpathcon to read the kernel binary policy file, and avoids the
overhead of parsing the kernel binary policy file on matchpathcon_init.

If, due to time pressure, we need a short term workaround, we could go with the
libsepol patch until the kernel change is upstreamed, but I think that the
kernel patch is the right long term approach.  James?

Comment 7 Stephen Smalley 2005-10-28 13:58:12 UTC
Note btw that to canonicalize the context, all you have to do is go through a
sequence.  The latter will always yield the canonical form.  The former is
already done by sel_write_context() to validate the context, so one would just
have to convert it over to using the transaction ops, perform the
security_sid_to_context call to get the canonical form, and copy that into the
buffer, similar to sel_write_create.

Comment 8 Stephen Smalley 2005-10-31 20:28:08 UTC
Created attachment 120588 [details]
Extend selinuxfs context interface to allow canonicalization of contexts

This patch extends the selinuxfs context interface to allow canonicalization of
Patch is by a colleague.

Comment 9 Stephen Smalley 2005-11-04 17:21:51 UTC
Ping.  Do we want to proceed with this kernel patch to enable userspace to
obtain the canonical form of a context?  

Comment 10 James Morris 2005-11-04 18:12:36 UTC
Yep, I think so.

Comment 11 Stephen Smalley 2005-11-18 12:23:27 UTC
Kernel patch was upstreamed and is in the rawhide kernel, and corresponding
patches to libselinux (matchpathcon) and setfiles were likewise committed and
included in rawhide, so restorecon, setfiles, and other users of matchpathcon
should no longer warn about type mismatches in lib_t/shlib_t and other type
alias cases in the targeted policy.

chcon could be patched to use security_canonicalize_context() to obtain the
canonical form of the user-supplied context in order to let the user know when
it is being canonicalized.

Comment 12 Robert Scheck 2005-11-20 19:05:44 UTC
Linux kernel 2.6.14-1.1697_FC5 includes the patch and solves the problem for me.