| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AANLkTimt5sxmgUmn-i-WcgMN4d0X00YVMujsA4QgS8nX@mail.gmail.com>
Date: Mon, 31 Jan 2011 18:27:11 +0200
From: Lucian Adrian Grijincu <lucian.grijincu@...il.com>
To: Stephen Smalley <sds@...ho.nsa.gov>
Cc: James Morris <jmorris@...ei.org>,
Eric Paris <eparis@...isplace.org>,
Al Viro <viro@...iv.linux.org.uk>,
Christoph Hellwig <hch@....de>,
Dave Chinner <dchinner@...hat.com>,
Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org,
selinux <selinux@...ho.nsa.gov>,
"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: [PATCH 2/2] RFC: selinux: sysctl: fix selinux labeling broken by
last patch
On Mon, Jan 31, 2011 at 3:59 PM, Stephen Smalley <sds@...ho.nsa.gov> wrote:
> - You don't need special handling of /proc/PID entries. Those are
> labeled via the security_task_to_inode -> selinux_task_to_inode hook,
> called from proc_pid_make_inode and the _revalidate functions.
On a clean 2.6.37 I ran:
ls -Z /proc/1/net/rpc/nfs >> find.txt
ls -Z /proc/1/limits >> find.txt
ls -Z /proc/1/net/netfilter >> find.txt
with a printk() for the path computed in selinux_proc_get_sid. These
paths printed were:
/net/rpc/nfs
/net/netfilter
/net/netfilter/nf_log [ and others files in netfilter dir ]
So, while there may be some ->sid initializing in the hooks you
mention, these ones get their sid from selinux_proc_get_sid called
from inode_doinit_with_dentry.
There was nothing for /proc/1/limits because it got filtered by this
check in inode_doinit_with_dentry():
struct proc_inode *proci = PROC_I(inode);
if (proci->pde) {
selinux_proc_get_sid(...)
I do need to get the PID part out to compute a valid path for
selinux_proc_get_sid().
Before Eric's patch the path was computed by walking the struct
proc_dir_entry->parent links. These links stopped before the PID
entry: /proc/PID/net/stuff -> /net/stuff.
Because now we walk the path without /proc/ -> /PID/net/stuff, we will
send a path to selinux_proc_get_sid that will not be mappable to a
valid label so it will default to proc_t.
I know understand why I saw some differences between running 'find
/proc | xargs ls -Z' with and without these patches: Eric's patches
called selinux_proc_get_sid() without this check:
struct proc_inode *proci = PROC_I(inode);
if (proci->pde) {
I've updated my patch to take this into consideration. I'll post an
update later with the patches merged and without the "//deleted"
manipulation.
Again, I have next to nothing selinux experience, and probably my test
system is very bad configured to run a relevant selinux test. I've
uploaded here the output and dmesg from running 'find /proc | xargs ls
-Z' on 2.6.37 with and without these two patches:
http://swarm.cs.pub.ro/~lucian/store/v4/
There are now fewer differences than before, but I'd like to point
something out: *without* the patches files in /proc/sys/* get labeled
like this.
-r--r--r-- unknown /proc/sys/fs/file-nr
-rw-r--r-- unknown /proc/sys/debug/exception-trace
-rw-r--r-- unknown /proc/sys/dev/cdrom/autoclose
-rw-r--r-- unknown /proc/sys/kernel/sem
-rw-r--r-- unknown
/proc/sys/net/ipv4/conf/all/accept_local
but with the patches:
-r--r--r-- system_u:object_r:sysctl_fs_t:s0 /proc/sys/fs/file-nr
-rw-r--r-- system_u:object_r:sysctl_t:s0 /proc/sys/debug/exception-trace
-rw-r--r-- system_u:object_r:sysctl_dev_t:s0 /proc/sys/dev/cdrom/autoclose
-rw-r--r-- system_u:object_r:sysctl_kernel_t:s0 /proc/sys/kernel/sem
-rw-r--r-- system_u:object_r:sysctl_net_t:s0
/proc/sys/net/ipv4/conf/all/accept_local
There seem to be no labeling mismatches elsewhere. So either sysctl
labeling is broken in 2.6.37 or my test setup is broken.
I'll try to find out if there's an earlier kernel that works on my
setul for sysctl too.
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index b652cb0..b17619d 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -31,7 +31,6 @@ static struct inode *proc_sys_make_inode(struct
super_block *sb,
ei->sysctl_entry = table;
inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
- inode->i_flags |= S_PRIVATE; /* tell selinux to ignore this inode */
inode->i_mode = table->mode;
if (!table->child) {
inode->i_mode |= S_IFREG;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 51615f6..af8be17 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1132,8 +1132,23 @@ static int selinux_proc_get_sid(struct dentry *dentry,
path = dentry_path(dentry, buffer, PAGE_SIZE);
if (IS_ERR(path))
rc = PTR_ERR(path);
- else
+ else {
+ /* because dentry is not hashed, dentry_path() will
+ * append "//deleted" to the end of the string. We'll
+ * remove this as no /proc/ file is named so. */
+ int pathlen = strlen(path);
+ int dellen = strlen("//deleted");
+ if ((pathlen > dellen) && strcmp(path + pathlen - dellen, "//deleted") == 0)
+ path[pathlen-dellen] = '\0';
+
+ /* each process gets a /proc/PID/ entry. Strip off the
+ * PID part to get a valid selinux labeling. */
+ while (path[1] >= '0' && path[1] <= '9') {
+ path[1] = '/';
+ path++;
+ }
rc = security_genfs_sid("proc", path, tclass, sid);
+ }
free_page((unsigned long)buffer);
return rc;
}
@@ -1302,7 +1317,8 @@ static int inode_doinit_with_dentry(struct inode
*inode, struct dentry *opt_dent
isec->sid = sbsec->sid;
if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) {
- if (opt_dentry) {
+ struct proc_inode *proci = PROC_I(inode);
+ if (opt_dentry && (proci->pde || proci->sysctl)) {
isec->sclass = inode_mode_to_security_class(inode->i_mode);
rc = selinux_proc_get_sid(opt_dentry,
isec->sclass,
@@ -1464,9 +1480,6 @@ static int inode_has_perm(const struct cred *cred,
validate_creds(cred);
- if (unlikely(IS_PRIVATE(inode)))
- return 0;
-
sid = cred_sid(cred);
isec = inode->i_security;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists