From 277d342fc423fca5e66e677fe629d1b2f8f1b9e2 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 31 Dec 2008 12:54:11 -0500 Subject: selinux: Deprecate and schedule the removal of the the compat_net functionality This patch is the first step towards removing the old "compat_net" code from the kernel. Secmark, the "compat_net" replacement was first introduced in 2.6.18 (September 2006) and the major Linux distributions with SELinux support have transitioned to Secmark so it is time to start deprecating the "compat_net" mechanism. Testing a patched version of 2.6.28-rc6 with the initial release of Fedora Core 5 did not show any problems when running in enforcing mode. This patch adds an entry to the feature-removal-schedule.txt file and removes the SECURITY_SELINUX_ENABLE_SECMARK_DEFAULT configuration option, forcing Secmark on by default although it can still be disabled at runtime. The patch also makes the Secmark permission checks "dynamic" in the sense that they are only executed when Secmark is configured; this should help prevent problems with older distributions that have not yet migrated to Secmark. Signed-off-by: Paul Moore Acked-by: James Morris --- security/selinux/hooks.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index dbeaa783b2a9..df30a7555d8a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4185,7 +4185,7 @@ static int selinux_sock_rcv_skb_iptables_compat(struct sock *sk, static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb, u16 family) { - int err; + int err = 0; struct sk_security_struct *sksec = sk->sk_security; u32 peer_sid; u32 sk_sid = sksec->sid; @@ -4202,7 +4202,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb, if (selinux_compat_net) err = selinux_sock_rcv_skb_iptables_compat(sk, skb, &ad, family, addrp); - else + else if (selinux_secmark_enabled()) err = avc_has_perm(sk_sid, skb->secmark, SECCLASS_PACKET, PACKET__RECV, &ad); if (err) @@ -4705,7 +4705,7 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb, if (selinux_ip_postroute_iptables_compat(skb->sk, ifindex, &ad, family, addrp)) return NF_DROP; - } else { + } else if (selinux_secmark_enabled()) { if (avc_has_perm(sksec->sid, skb->secmark, SECCLASS_PACKET, PACKET__SEND, &ad)) return NF_DROP; -- cgit v1.2.3 From 14eaddc967b16017d4a1a24d2be6c28ecbe06ed8 Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 31 Dec 2008 15:15:42 +0000 Subject: CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2] Fix a regression in cap_capable() due to: commit 5ff7711e635b32f0a1e558227d030c7e45b4a465 Author: David Howells Date: Wed Dec 31 02:52:28 2008 +0000 CRED: Differentiate objective and effective subjective credentials on a task The problem is that the above patch allows a process to have two sets of credentials, and for the most part uses the subjective credentials when accessing current's creds. There is, however, one exception: cap_capable(), and thus capable(), uses the real/objective credentials of the target task, whether or not it is the current task. Ordinarily this doesn't matter, since usually the two cred pointers in current point to the same set of creds. However, sys_faccessat() makes use of this facility to override the credentials of the calling process to make its test, without affecting the creds as seen from other processes. One of the things sys_faccessat() does is to make an adjustment to the effective capabilities mask, which cap_capable(), as it stands, then ignores. The affected capability check is in generic_permission(): if (!(mask & MAY_EXEC) || execute_ok(inode)) if (capable(CAP_DAC_OVERRIDE)) return 0; This change splits capable() from has_capability() down into the commoncap and SELinux code. The capable() security op now only deals with the current process, and uses the current process's subjective creds. A new security op - task_capable() - is introduced that can check any task's objective creds. strictly the capable() security op is superfluous with the presence of the task_capable() op, however it should be faster to call the capable() op since two fewer arguments need be passed down through the various layers. This can be tested by compiling the following program from the XFS testsuite: /* * t_access_root.c - trivial test program to show permission bug. * * Written by Michael Kerrisk - copyright ownership not pursued. * Sourced from: http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-10/6030.html */ #include #include #include #include #include #include #define UID 500 #define GID 100 #define PERM 0 #define TESTPATH "/tmp/t_access" static void errExit(char *msg) { perror(msg); exit(EXIT_FAILURE); } /* errExit */ static void accessTest(char *file, int mask, char *mstr) { printf("access(%s, %s) returns %d\n", file, mstr, access(file, mask)); } /* accessTest */ int main(int argc, char *argv[]) { int fd, perm, uid, gid; char *testpath; char cmd[PATH_MAX + 20]; testpath = (argc > 1) ? argv[1] : TESTPATH; perm = (argc > 2) ? strtoul(argv[2], NULL, 8) : PERM; uid = (argc > 3) ? atoi(argv[3]) : UID; gid = (argc > 4) ? atoi(argv[4]) : GID; unlink(testpath); fd = open(testpath, O_RDWR | O_CREAT, 0); if (fd == -1) errExit("open"); if (fchown(fd, uid, gid) == -1) errExit("fchown"); if (fchmod(fd, perm) == -1) errExit("fchmod"); close(fd); snprintf(cmd, sizeof(cmd), "ls -l %s", testpath); system(cmd); if (seteuid(uid) == -1) errExit("seteuid"); accessTest(testpath, 0, "0"); accessTest(testpath, R_OK, "R_OK"); accessTest(testpath, W_OK, "W_OK"); accessTest(testpath, X_OK, "X_OK"); accessTest(testpath, R_OK | W_OK, "R_OK | W_OK"); accessTest(testpath, R_OK | X_OK, "R_OK | X_OK"); accessTest(testpath, W_OK | X_OK, "W_OK | X_OK"); accessTest(testpath, R_OK | W_OK | X_OK, "R_OK | W_OK | X_OK"); exit(EXIT_SUCCESS); } /* main */ This can be run against an Ext3 filesystem as well as against an XFS filesystem. If successful, it will show: [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043 ---------- 1 dhowells dhowells 0 2008-12-31 03:00 /tmp/xxx access(/tmp/xxx, 0) returns 0 access(/tmp/xxx, R_OK) returns 0 access(/tmp/xxx, W_OK) returns 0 access(/tmp/xxx, X_OK) returns -1 access(/tmp/xxx, R_OK | W_OK) returns 0 access(/tmp/xxx, R_OK | X_OK) returns -1 access(/tmp/xxx, W_OK | X_OK) returns -1 access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1 If unsuccessful, it will show: [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043 ---------- 1 dhowells dhowells 0 2008-12-31 02:56 /tmp/xxx access(/tmp/xxx, 0) returns 0 access(/tmp/xxx, R_OK) returns -1 access(/tmp/xxx, W_OK) returns -1 access(/tmp/xxx, X_OK) returns -1 access(/tmp/xxx, R_OK | W_OK) returns -1 access(/tmp/xxx, R_OK | X_OK) returns -1 access(/tmp/xxx, W_OK | X_OK) returns -1 access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1 I've also tested the fix with the SELinux and syscalls LTP testsuites. Signed-off-by: David Howells Signed-off-by: James Morris --- security/selinux/hooks.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index df30a7555d8a..eb6c45107a05 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk, /* Check whether a task is allowed to use a capability. */ static int task_has_capability(struct task_struct *tsk, + const struct cred *cred, int cap, int audit) { struct avc_audit_data ad; struct av_decision avd; u16 sclass; - u32 sid = task_sid(tsk); + u32 sid = cred_sid(cred); u32 av = CAP_TO_MASK(cap); int rc; @@ -1865,15 +1866,27 @@ static int selinux_capset(struct cred *new, const struct cred *old, return cred_has_perm(old, new, PROCESS__SETCAP); } -static int selinux_capable(struct task_struct *tsk, int cap, int audit) +static int selinux_capable(int cap, int audit) +{ + int rc; + + rc = secondary_ops->capable(cap, audit); + if (rc) + return rc; + + return task_has_capability(current, current_cred(), cap, audit); +} + +static int selinux_task_capable(struct task_struct *tsk, + const struct cred *cred, int cap, int audit) { int rc; - rc = secondary_ops->capable(tsk, cap, audit); + rc = secondary_ops->task_capable(tsk, cred, cap, audit); if (rc) return rc; - return task_has_capability(tsk, cap, audit); + return task_has_capability(tsk, cred, cap, audit); } static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid) @@ -2037,7 +2050,7 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages) { int rc, cap_sys_admin = 0; - rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT); + rc = selinux_capable(CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT); if (rc == 0) cap_sys_admin = 1; @@ -2880,7 +2893,7 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name * and lack of permission just means that we fall back to the * in-core context value, not a denial. */ - error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT); + error = selinux_capable(CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT); if (!error) error = security_sid_to_context_force(isec->sid, &context, &size); @@ -5568,6 +5581,7 @@ static struct security_operations selinux_ops = { .capset = selinux_capset, .sysctl = selinux_sysctl, .capable = selinux_capable, + .task_capable = selinux_task_capable, .quotactl = selinux_quotactl, .quota_on = selinux_quota_on, .syslog = selinux_syslog, -- cgit v1.2.3 From 29881c4502ba05f46bc12ae8053d4e08d7e2615c Mon Sep 17 00:00:00 2001 From: James Morris Date: Wed, 7 Jan 2009 09:21:54 +1100 Subject: Revert "CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #2]" This reverts commit 14eaddc967b16017d4a1a24d2be6c28ecbe06ed8. David has a better version to come. --- security/selinux/hooks.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index eb6c45107a05..df30a7555d8a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1433,13 +1433,12 @@ static int current_has_perm(const struct task_struct *tsk, /* Check whether a task is allowed to use a capability. */ static int task_has_capability(struct task_struct *tsk, - const struct cred *cred, int cap, int audit) { struct avc_audit_data ad; struct av_decision avd; u16 sclass; - u32 sid = cred_sid(cred); + u32 sid = task_sid(tsk); u32 av = CAP_TO_MASK(cap); int rc; @@ -1866,27 +1865,15 @@ static int selinux_capset(struct cred *new, const struct cred *old, return cred_has_perm(old, new, PROCESS__SETCAP); } -static int selinux_capable(int cap, int audit) -{ - int rc; - - rc = secondary_ops->capable(cap, audit); - if (rc) - return rc; - - return task_has_capability(current, current_cred(), cap, audit); -} - -static int selinux_task_capable(struct task_struct *tsk, - const struct cred *cred, int cap, int audit) +static int selinux_capable(struct task_struct *tsk, int cap, int audit) { int rc; - rc = secondary_ops->task_capable(tsk, cred, cap, audit); + rc = secondary_ops->capable(tsk, cap, audit); if (rc) return rc; - return task_has_capability(tsk, cred, cap, audit); + return task_has_capability(tsk, cap, audit); } static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid) @@ -2050,7 +2037,7 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages) { int rc, cap_sys_admin = 0; - rc = selinux_capable(CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT); + rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT); if (rc == 0) cap_sys_admin = 1; @@ -2893,7 +2880,7 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name * and lack of permission just means that we fall back to the * in-core context value, not a denial. */ - error = selinux_capable(CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT); + error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT); if (!error) error = security_sid_to_context_force(isec->sid, &context, &size); @@ -5581,7 +5568,6 @@ static struct security_operations selinux_ops = { .capset = selinux_capset, .sysctl = selinux_sysctl, .capable = selinux_capable, - .task_capable = selinux_task_capable, .quotactl = selinux_quotactl, .quota_on = selinux_quota_on, .syslog = selinux_syslog, -- cgit v1.2.3 From 3699c53c485bf0168e6500d0ed18bf931584dd7c Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 6 Jan 2009 22:27:01 +0000 Subject: CRED: Fix regression in cap_capable() as shown up by sys_faccessat() [ver #3] Fix a regression in cap_capable() due to: commit 3b11a1decef07c19443d24ae926982bc8ec9f4c0 Author: David Howells Date: Fri Nov 14 10:39:26 2008 +1100 CRED: Differentiate objective and effective subjective credentials on a task The problem is that the above patch allows a process to have two sets of credentials, and for the most part uses the subjective credentials when accessing current's creds. There is, however, one exception: cap_capable(), and thus capable(), uses the real/objective credentials of the target task, whether or not it is the current task. Ordinarily this doesn't matter, since usually the two cred pointers in current point to the same set of creds. However, sys_faccessat() makes use of this facility to override the credentials of the calling process to make its test, without affecting the creds as seen from other processes. One of the things sys_faccessat() does is to make an adjustment to the effective capabilities mask, which cap_capable(), as it stands, then ignores. The affected capability check is in generic_permission(): if (!(mask & MAY_EXEC) || execute_ok(inode)) if (capable(CAP_DAC_OVERRIDE)) return 0; This change passes the set of credentials to be tested down into the commoncap and SELinux code. The security functions called by capable() and has_capability() select the appropriate set of credentials from the process being checked. This can be tested by compiling the following program from the XFS testsuite: /* * t_access_root.c - trivial test program to show permission bug. * * Written by Michael Kerrisk - copyright ownership not pursued. * Sourced from: http://linux.derkeiler.com/Mailing-Lists/Kernel/2003-10/6030.html */ #include #include #include #include #include #include #define UID 500 #define GID 100 #define PERM 0 #define TESTPATH "/tmp/t_access" static void errExit(char *msg) { perror(msg); exit(EXIT_FAILURE); } /* errExit */ static void accessTest(char *file, int mask, char *mstr) { printf("access(%s, %s) returns %d\n", file, mstr, access(file, mask)); } /* accessTest */ int main(int argc, char *argv[]) { int fd, perm, uid, gid; char *testpath; char cmd[PATH_MAX + 20]; testpath = (argc > 1) ? argv[1] : TESTPATH; perm = (argc > 2) ? strtoul(argv[2], NULL, 8) : PERM; uid = (argc > 3) ? atoi(argv[3]) : UID; gid = (argc > 4) ? atoi(argv[4]) : GID; unlink(testpath); fd = open(testpath, O_RDWR | O_CREAT, 0); if (fd == -1) errExit("open"); if (fchown(fd, uid, gid) == -1) errExit("fchown"); if (fchmod(fd, perm) == -1) errExit("fchmod"); close(fd); snprintf(cmd, sizeof(cmd), "ls -l %s", testpath); system(cmd); if (seteuid(uid) == -1) errExit("seteuid"); accessTest(testpath, 0, "0"); accessTest(testpath, R_OK, "R_OK"); accessTest(testpath, W_OK, "W_OK"); accessTest(testpath, X_OK, "X_OK"); accessTest(testpath, R_OK | W_OK, "R_OK | W_OK"); accessTest(testpath, R_OK | X_OK, "R_OK | X_OK"); accessTest(testpath, W_OK | X_OK, "W_OK | X_OK"); accessTest(testpath, R_OK | W_OK | X_OK, "R_OK | W_OK | X_OK"); exit(EXIT_SUCCESS); } /* main */ This can be run against an Ext3 filesystem as well as against an XFS filesystem. If successful, it will show: [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043 ---------- 1 dhowells dhowells 0 2008-12-31 03:00 /tmp/xxx access(/tmp/xxx, 0) returns 0 access(/tmp/xxx, R_OK) returns 0 access(/tmp/xxx, W_OK) returns 0 access(/tmp/xxx, X_OK) returns -1 access(/tmp/xxx, R_OK | W_OK) returns 0 access(/tmp/xxx, R_OK | X_OK) returns -1 access(/tmp/xxx, W_OK | X_OK) returns -1 access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1 If unsuccessful, it will show: [root@andromeda src]# ./t_access_root /tmp/xxx 0 4043 4043 ---------- 1 dhowells dhowells 0 2008-12-31 02:56 /tmp/xxx access(/tmp/xxx, 0) returns 0 access(/tmp/xxx, R_OK) returns -1 access(/tmp/xxx, W_OK) returns -1 access(/tmp/xxx, X_OK) returns -1 access(/tmp/xxx, R_OK | W_OK) returns -1 access(/tmp/xxx, R_OK | X_OK) returns -1 access(/tmp/xxx, W_OK | X_OK) returns -1 access(/tmp/xxx, R_OK | W_OK | X_OK) returns -1 I've also tested the fix with the SELinux and syscalls LTP testsuites. Signed-off-by: David Howells Tested-by: J. Bruce Fields Acked-by: Serge Hallyn Signed-off-by: James Morris --- security/selinux/hooks.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index df30a7555d8a..00815973d412 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1433,12 +1433,13 @@ static int current_has_perm(const struct task_struct *tsk, /* Check whether a task is allowed to use a capability. */ static int task_has_capability(struct task_struct *tsk, + const struct cred *cred, int cap, int audit) { struct avc_audit_data ad; struct av_decision avd; u16 sclass; - u32 sid = task_sid(tsk); + u32 sid = cred_sid(cred); u32 av = CAP_TO_MASK(cap); int rc; @@ -1865,15 +1866,16 @@ static int selinux_capset(struct cred *new, const struct cred *old, return cred_has_perm(old, new, PROCESS__SETCAP); } -static int selinux_capable(struct task_struct *tsk, int cap, int audit) +static int selinux_capable(struct task_struct *tsk, const struct cred *cred, + int cap, int audit) { int rc; - rc = secondary_ops->capable(tsk, cap, audit); + rc = secondary_ops->capable(tsk, cred, cap, audit); if (rc) return rc; - return task_has_capability(tsk, cap, audit); + return task_has_capability(tsk, cred, cap, audit); } static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid) @@ -2037,7 +2039,8 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages) { int rc, cap_sys_admin = 0; - rc = selinux_capable(current, CAP_SYS_ADMIN, SECURITY_CAP_NOAUDIT); + rc = selinux_capable(current, current_cred(), CAP_SYS_ADMIN, + SECURITY_CAP_NOAUDIT); if (rc == 0) cap_sys_admin = 1; @@ -2880,7 +2883,8 @@ static int selinux_inode_getsecurity(const struct inode *inode, const char *name * and lack of permission just means that we fall back to the * in-core context value, not a denial. */ - error = selinux_capable(current, CAP_MAC_ADMIN, SECURITY_CAP_NOAUDIT); + error = selinux_capable(current, current_cred(), CAP_MAC_ADMIN, + SECURITY_CAP_NOAUDIT); if (!error) error = security_sid_to_context_force(isec->sid, &context, &size); -- cgit v1.2.3