Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Date | Mon, 15 Nov 2010 12:34:42 -0500 | Subject | Re: [PATCH] Fix dmesg_restrict build failure with CONFIG_EMBEDDED=y and CONFIG_PRINTK=n | From | Eric Paris <> |
| |
On Mon, Nov 15, 2010 at 12:04 PM, Eric Paris <eparis@parisplace.org> wrote: > On Sat, Nov 13, 2010 at 3:31 PM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: >> We had this exact problem with the whole "mmap_min_addr" thing. People >> _thought_ of it as generic, but because it was actually tested by the >> security logic, if you ended up enabling SELinux the test actually >> went away entirely (or maybe it was the other way around). So with >> certain security models, the whole thing was bypassed, and the >> security module actually became an _IN_security module. > > Your recollection is wrong, although your conclusions of the > ramifications are right. Either SELinux or capabilities checked > mmap_min_addr, depending on which was 'primary.' Just as they are > different modules they checked for different things. Only doing > SELinux checks was stronger for some situations, and only doing > capability checks was stronger in some ways (and the reverse was > obviously true). Today you get the best of both worlds since we > really have 2 different mmap_min_addr values... > > In any case the result of that is that LSMs (ok 'I') need to be more > careful making sure they interact properly with the generic > capabilities hooks. > > From: James Morris >> I want to ensure that LSMs which implement security_syslog can't end up >> with a less secure system than the default, regardless of whether they >> call cap_syslog or not. > > Which really means that this is total crap. If you don't call > cap_syslog() you broke it. That's all there is to it. Calling the > capability code is always required. full stop. > > I think this patch is broken though. SELinux and SMACK don't care > about from_file and want to check every time no matter what. Your > patch breaks that and only will call the LSM on occasion. It's only > capabilities that likes those semantics. I think the entire contents > of the cap_syslog hook should be moved up and that hook just dropped > entirely. > > I'll code up what I'm thinking in a minute..... > > -Eric
I'm sure somebody somewhere hates it, but I was thinking something like the attached.
include/linux/security.h | 9 ++++----- kernel/printk.c | 11 ++++++++++- security/capability.c | 5 +++++ security/commoncap.c | 21 --------------------- security/security.c | 4 ++-- security/selinux/hooks.c | 6 +----- security/smack/smack_lsm.c | 8 ++------ 7 files changed, 24 insertions(+), 40 deletions(-)
(Personally I think that most of the hooks in commoncap.c code should be moved out of security/ altogether and we should completely do away with our current ghetto inter LSM calls. But that's just me)
-Eric commit acef990cde0dbdc9e88df2da54cac4debc6a503f Author: Eric Paris <eparis@redhat.com> Date: Mon Nov 15 12:20:27 2010 -0500
fix capabilities-restric
diff --git a/include/linux/security.h b/include/linux/security.h index aee3b8f..d42619e 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -77,7 +77,6 @@ extern int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, extern int cap_task_setscheduler(struct task_struct *p); extern int cap_task_setioprio(struct task_struct *p, int ioprio); extern int cap_task_setnice(struct task_struct *p, int nice); -extern int cap_syslog(int type, bool from_file); extern int cap_vm_enough_memory(struct mm_struct *mm, long pages); struct msghdr; @@ -1389,7 +1388,7 @@ struct security_operations { int (*sysctl) (struct ctl_table *table, int op); int (*quotactl) (int cmds, int type, int id, struct super_block *sb); int (*quota_on) (struct dentry *dentry); - int (*syslog) (int type, bool from_file); + int (*syslog) (int type); int (*settime) (struct timespec *ts, struct timezone *tz); int (*vm_enough_memory) (struct mm_struct *mm, long pages); @@ -1675,7 +1674,7 @@ int security_real_capable_noaudit(struct task_struct *tsk, int cap); int security_sysctl(struct ctl_table *table, int op); int security_quotactl(int cmds, int type, int id, struct super_block *sb); int security_quota_on(struct dentry *dentry); -int security_syslog(int type, bool from_file); +int security_syslog(int type); int security_settime(struct timespec *ts, struct timezone *tz); int security_vm_enough_memory(long pages); int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); @@ -1907,9 +1906,9 @@ static inline int security_quota_on(struct dentry *dentry) return 0; } -static inline int security_syslog(int type, bool from_file) +static inline int security_syslog(int type) { - return cap_syslog(type, from_file); + return 0; } static inline int security_settime(struct timespec *ts, struct timezone *tz) diff --git a/kernel/printk.c b/kernel/printk.c index 38e7d58..d00efd1 100644 --- a/kernel/printk.c +++ b/kernel/printk.c @@ -274,7 +274,16 @@ int do_syslog(int type, char __user *buf, int len, bool from_file) char c; int error = 0; - error = security_syslog(type, from_file); + if (type == SYSLOG_ACTION_OPEN || !from_file) { + if (dmesg_restrict && !capable(CAP_SYS_ADMIN)) + return -EPERM; + if ((type != SYSLOG_ACTION_READ_ALL && + type != SYSLOG_ACTION_SIZE_BUFFER) && + !capable(CAP_SYS_ADMIN)) + return -EPERM; + } + + error = security_syslog(type); if (error) return error; diff --git a/security/capability.c b/security/capability.c index d6d613a..6b5c6e9 100644 --- a/security/capability.c +++ b/security/capability.c @@ -17,6 +17,11 @@ static int cap_sysctl(ctl_table *table, int op) return 0; } +static int cap_syslog(int type) +{ + return 0; +} + static int cap_quotactl(int cmds, int type, int id, struct super_block *sb) { return 0; diff --git a/security/commoncap.c b/security/commoncap.c index 04b80f9..64c2ed9 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -27,7 +27,6 @@ #include <linux/sched.h> #include <linux/prctl.h> #include <linux/securebits.h> -#include <linux/syslog.h> /* * If a non-root user executes a setuid-root binary in @@ -884,26 +883,6 @@ error: } /** - * cap_syslog - Determine whether syslog function is permitted - * @type: Function requested - * @from_file: Whether this request came from an open file (i.e. /proc) - * - * Determine whether the current process is permitted to use a particular - * syslog function, returning 0 if permission is granted, -ve if not. - */ -int cap_syslog(int type, bool from_file) -{ - if (type != SYSLOG_ACTION_OPEN && from_file) - return 0; - if (dmesg_restrict && !capable(CAP_SYS_ADMIN)) - return -EPERM; - if ((type != SYSLOG_ACTION_READ_ALL && - type != SYSLOG_ACTION_SIZE_BUFFER) && !capable(CAP_SYS_ADMIN)) - return -EPERM; - return 0; -} - -/** * cap_vm_enough_memory - Determine whether a new virtual mapping is permitted * @mm: The VM space in which the new mapping is to be made * @pages: The size of the mapping diff --git a/security/security.c b/security/security.c index 259d3ad..639a72a 100644 --- a/security/security.c +++ b/security/security.c @@ -197,9 +197,9 @@ int security_quota_on(struct dentry *dentry) return security_ops->quota_on(dentry); } -int security_syslog(int type, bool from_file) +int security_syslog(int type) { - return security_ops->syslog(type, from_file); + return security_ops->syslog(type); } int security_settime(struct timespec *ts, struct timezone *tz) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 8ba5001..e066bc2 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1971,14 +1971,10 @@ static int selinux_quota_on(struct dentry *dentry) return dentry_has_perm(cred, NULL, dentry, FILE__QUOTAON); } -static int selinux_syslog(int type, bool from_file) +static int selinux_syslog(int type) { int rc; - rc = cap_syslog(type, from_file); - if (rc) - return rc; - switch (type) { case SYSLOG_ACTION_READ_ALL: /* Read last kernel messages */ case SYSLOG_ACTION_SIZE_BUFFER: /* Return size of the log buffer */ diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 6cc47ef..f7b8bee 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -157,15 +157,11 @@ static int smack_ptrace_traceme(struct task_struct *ptp) * * Returns 0 on success, error code otherwise. */ -static int smack_syslog(int type, bool from_file) +static int smack_syslog(int typefrom_file) { - int rc; + int rc = 0; char *sp = current_security(); - rc = cap_syslog(type, from_file); - if (rc != 0) - return rc; - if (capable(CAP_MAC_OVERRIDE)) return 0; | ![\](/images/icornerr.gif) |