Red Hat Bugzilla – Bug 196567
CIFS does not set sclass before d_instantiate causes SELinux failures
Last modified: 2007-11-30 17:11:35 EST
Description of problem: When running httpd with php scripts in cgi mode and accessing object on a cifs share, Selinux report directories on that mount as target security class (tclass) "file", causing actions such as search and rmdir impossible on that directory. On such occasion the audit.log report an HEX value in the denial description instead of the mapped name, because selinux cannot map "search" to any existing action available for a file. example from audit.log: type=AVC msg=audit(1151178769.339:656): avc: denied { 0x200000 } for pid=2834 comm="rmdir" name="sample_dir" dev=cifs ino=9600 scontext=root:system_r:httpd_suexec_t:s0 tcontext=system_u:object_r:cifs_t:s0 tclass=file type=SYSCALL msg=audit(1151178769.339:656): arch=40000003 syscall=40 success=no exit=-13 a0=8b58c14 a1=81a4a60 a2=8b58c14 a3=bf8bbdd8 items=1 pid=2834 auid=0 uid=20000 gid=20000 euid=20000 suid=20000 fsuid=20000 egid=20000 sgid=20000 fsgid=20000 tty=(none) comm="rmdir" exe="/usr/bin/php" subj=root:system_r:httpd_suexec_t:s0 When accessing the cifs share with selinux disabled, or from an unconfined space no problem occurs. In the sample above I run the cgi script with suexec, but the problem is the same when running scripts in the httpd_t context (I really don't believe httpd is involved anyway, it should be the same with other confined spaces. But it's where observed the bug in the first place) Version-Release number of selected component (if applicable): Fedora core 5, with kernel 2.6.17-1.2139 (same behavior as previous 2 releases) httpd 2.2.0-5.1.2 libselinux 1.30.3-3 samba 3.0.22-1 selinux-policy-targeted 2.2.43-4 How reproducible: Always Steps to Reproduce: 1. install httpd samba samba-client selinux-targeted-policy php 2. set : "unix extensions = no" in /etc/samba/smb.conf 3. create a share locally and put some content in it (random files / dirs) 4. Add a user to use in accessing the share 5. map the share locally (for testing it is useful to set uid/gid of the mount to apache's one) I used autofs with the following options to mount in /home/testmount : testmount -fstype=cifs,username=shareuser,password=sharepass,uid=48,gid=48,dir_mode=0770,file_mode=0660 ://localhost/testmount 6. create a virtualhost or set apache to run php as cgi (result could be the same with mod_php) 7. create a test script "<?php rmdir('/home/testmount/sample_dir'); ?> 8. run that script with a web browser (9. Add all selinux rules that could be needed (for cifs_t, httpd_t, etc) and be sure unix permissions are sufficient on all files and folder. be sure that sample_dir is an empty directory) Actual results: Directory is not removed and selinux give the error denied { 0x200000 } on sample_dir and incorectly identify it as a file instead of a folder Expected results: Directory should be deleted with no denial Additional info: 0x200000 is related to rmdir. Using a script like "<?php shell_exec('du /home/testmount'); ?>", An error of 0x100000 is reported on some folder (search) which also are incorectly identified as files.
An easier way to test this issue using the same cifs share setup: enable the strict policy, (I set a permissive state) and as root do: rmdir /home/testmount/sample_dir observe in audit.log : type=AVC msg=audit(1151197545.712:144): avc: denied { 0x200000 } for pid=2608 comm="rmdir" name="sample_dir" dev=cifs ino=8889 scontext=root:staff_r:staff_t:s0-s0:c0.c255 tcontext=system_u:object_r:cifs_t:s0 tclass=file
SELinux sets up the inode security state upon d_instantiate, so it expects the inode mode to be set (so that it can set the inode security class). cifs appears to call d_instantiate before fully setting up the inode state. Either cifs needs to change to set up the inode first, or selinux needs to revalidate the inode security class on every hook call/permission check.
This bug is identical to bug 163493, which unfortunately was closed without resolution. I raised that bug with the upstream cifs maintainer at the time, and he acknowledged it, but I don't think anything happened to address it in the cifs code.
Re-raised issue with upstream cifs maintainer. cifs_filldir and cifs_mkdir seem to both call d_instantiate before setting up the inode state. I think that this is a general bug in cifs, as it would mean that the inode might be visible to other threads in an inconsistent state.
I think we need to work around this issue in the SELinux code, both for cifs and any other filesystems that may fail to set the inode mode properly before calling d_instantiate. We could recompute the isec->sclass via inode_mode_to_security_class(inode->i_mode) in inode_has_perm, as long as it is still SECCLASS_FILE. If it has already been set to any other class, we shouldn't touch it again, because it has already been set to a more specific value, and in particular, we don't want to disturb it if it has been set to a socket class by the socket hooks.
Would it be reasonable to print a warning if we actually make a change in inode_has_perm to sclass? Yes this is fixing the problem with CIFS since they don't seem to be interested in fixing it, but it may also paper over problems in other filesystems that may creep up and may be willing to fix it.
Possibly, but make sure you call printk_ratelimit(); otherwise, you could flood the system if the fs lazily sets the inode mode for all of its inodes...
Taking bug as I'm sure Dan doesn't mind one less on his list.
I found 4 hooks that seem to use isec->sclass and we may wish to have it rechecked. inode_has_perm may_link may_rename selinux_inode_setxattr At first I wondered if inode_has_perm covered all 4 cases, but I don't see that this is true. I'm going to check again tomorrow to see if there is one place I can put this to be sure it covers all 4 hooks. The code I plan to add (untested, not even compiled) is below. if (isec->sclass == SECCLASS_FILE) { isec->sclass = inode_mode_to_security_class(inode->i_mode); /* * can be caused by fs calling d_instantiate before setting i_mode. */ if (unlikely(isec->sclass != SECCLASS_FILE) && printk_ratelimit()) printk(KERN_INFO "Inode found with sclass set improperly." " Please contact the filesystem maintainer\n"); }
I'm not sure it is truly necessary to have complete coverage here, as the d_instantiate/d_splice_alias hook is supposed to provide us with complete coverage in the first place, and the inode_has_perm one is just to catch these erroneous cases which manifest as search attempts on directories that are mis-classified as files. If you are going to do it in multiple places, use a static inline helper. The message isn't useful as it stands because it doesn't provide any identifying information, e.g. the fs type, the device, the inode number, etc.
I think i'm going to go for complete coverage unless there is a strong objection. inode_has_perm does take care of the search requests. And actually it also took care of using mv (which I hoped would hit may_rename) and chcon (which I hoped would hit selinux_inode_setxattr) but I'm still able to hit 0x200000 with rmdir (which I'm reasonably sure is from running down the may_link path. I'll make the error message pretty and useful, use an inline helper and post something soon.
A CIFS fix was included in 2.6.18. If we still want to include my code to make checks, assurances, and reporting is still up in the air.
Since there are no known problems we are going to pass on the redundant check of the type on every access. Thus this bug is solved in 2.6.18 and later.