Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933099Ab0BDBj5 (ORCPT ); Wed, 3 Feb 2010 20:39:57 -0500 Received: from adelie.canonical.com ([91.189.90.139]:46120 "EHLO adelie.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932991Ab0BDBjv (ORCPT ); Wed, 3 Feb 2010 20:39:51 -0500 Message-ID: <4B6A2559.5080206@canonical.com> Date: Wed, 03 Feb 2010 17:39:37 -0800 From: John Johansen Organization: Canonical User-Agent: Thunderbird 2.0.0.23 (X11/20090817) MIME-Version: 1.0 To: "Serge E. Hallyn" CC: Kees Cook , James Morris , Casey Schaufler , linux-security-module@vger.kernel.org, Eric Paris , David Howells , Alexey Dobriyan , Ingo Molnar , Andrew Morton , Simon Kagstrom , David Woodhouse , Robin Getz , Greg Kroah-Hartman , Paul Moore , Tetsuo Handa , Stephen Smalley , Etienne Basset , "David P. Quigley" , LKLM Subject: Re: [PATCH v2 1/2] syslog: distinguish between /proc/kmsg and syscalls References: <20100202055354.GV19355@outflux.net> <4B67C2EA.705@schaufler-ca.com> <20100202202054.GW19355@outflux.net> <20100202212510.GG32305@us.ibm.com> <20100203233643.GI19355@outflux.net> <20100204003052.GA16681@us.ibm.com> In-Reply-To: <20100204003052.GA16681@us.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10892 Lines: 310 Serge E. Hallyn wrote: > Quoting Kees Cook (kees.cook@canonical.com): >> This allows the LSM to distinguish between syslog functions originating >> from /proc/kmsg access and direct syscalls. By default, the commoncaps >> will now no longer require CAP_SYS_ADMIN to read an opened /proc/kmsg >> file descriptor. For example the kernel syslog reader can now drop >> privileges after opening /proc/kmsg, instead of staying privileged with >> CAP_SYS_ADMIN. MAC systems that implement security_syslog have unchanged >> behavior. >> >> Signed-off-by: Kees Cook > > Acked-by: Serge Hallyn Acked-by: John Johansen > >> --- >> fs/proc/kmsg.c | 14 +++++++------- >> include/linux/security.h | 11 ++++++----- >> include/linux/syslog.h | 29 +++++++++++++++++++++++++++++ >> kernel/printk.c | 7 ++++--- >> security/commoncap.c | 7 ++++++- >> security/security.c | 4 ++-- >> security/selinux/hooks.c | 5 +++-- >> security/smack/smack_lsm.c | 4 ++-- >> 8 files changed, 59 insertions(+), 22 deletions(-) >> create mode 100644 include/linux/syslog.h >> >> diff --git a/fs/proc/kmsg.c b/fs/proc/kmsg.c >> index 7ca7834..6a3d843 100644 >> --- a/fs/proc/kmsg.c >> +++ b/fs/proc/kmsg.c >> @@ -12,37 +12,37 @@ >> #include >> #include >> #include >> +#include >> >> #include >> #include >> >> extern wait_queue_head_t log_wait; >> >> -extern int do_syslog(int type, char __user *bug, int count); >> - >> static int kmsg_open(struct inode * inode, struct file * file) >> { >> - return do_syslog(1,NULL,0); >> + return do_syslog(1, NULL, 0, SYSLOG_FROM_FILE); >> } >> >> static int kmsg_release(struct inode * inode, struct file * file) >> { >> - (void) do_syslog(0,NULL,0); >> + (void) do_syslog(0, NULL, 0, SYSLOG_FROM_FILE); >> return 0; >> } >> >> static ssize_t kmsg_read(struct file *file, char __user *buf, >> size_t count, loff_t *ppos) >> { >> - if ((file->f_flags & O_NONBLOCK) && !do_syslog(9, NULL, 0)) >> + if ((file->f_flags & O_NONBLOCK) && >> + !do_syslog(9, NULL, 0, SYSLOG_FROM_FILE)) >> return -EAGAIN; >> - return do_syslog(2, buf, count); >> + return do_syslog(2, buf, count, SYSLOG_FROM_FILE); >> } >> >> static unsigned int kmsg_poll(struct file *file, poll_table *wait) >> { >> poll_wait(file, &log_wait, wait); >> - if (do_syslog(9, NULL, 0)) >> + if (do_syslog(9, NULL, 0, SYSLOG_FROM_FILE)) >> return POLLIN | POLLRDNORM; >> return 0; >> } >> diff --git a/include/linux/security.h b/include/linux/security.h >> index 2c627d3..106786e 100644 >> --- a/include/linux/security.h >> +++ b/include/linux/security.h >> @@ -76,7 +76,7 @@ extern int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, >> extern int cap_task_setscheduler(struct task_struct *p, int policy, struct sched_param *lp); >> 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); >> +extern int cap_syslog(int type, bool from_file); >> extern int cap_vm_enough_memory(struct mm_struct *mm, long pages); >> >> struct msghdr; >> @@ -1348,6 +1348,7 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts) >> * logging to the console. >> * See the syslog(2) manual page for an explanation of the @type values. >> * @type contains the type of action. >> + * @from_file indicates the context of action (if it came from /proc). >> * Return 0 if permission is granted. >> * @settime: >> * Check permission to change the system time. >> @@ -1462,7 +1463,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); >> + int (*syslog) (int type, bool from_file); >> int (*settime) (struct timespec *ts, struct timezone *tz); >> int (*vm_enough_memory) (struct mm_struct *mm, long pages); >> >> @@ -1761,7 +1762,7 @@ int security_acct(struct file *file); >> 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); >> +int security_syslog(int type, bool from_file); >> 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); >> @@ -2007,9 +2008,9 @@ static inline int security_quota_on(struct dentry *dentry) >> return 0; >> } >> >> -static inline int security_syslog(int type) >> +static inline int security_syslog(int type, bool from_file) >> { >> - return cap_syslog(type); >> + return cap_syslog(type, from_file); >> } >> >> static inline int security_settime(struct timespec *ts, struct timezone *tz) >> diff --git a/include/linux/syslog.h b/include/linux/syslog.h >> new file mode 100644 >> index 0000000..5f02b18 >> --- /dev/null >> +++ b/include/linux/syslog.h >> @@ -0,0 +1,29 @@ >> +/* Syslog internals >> + * >> + * Copyright 2010 Canonical, Ltd. >> + * Author: Kees Cook >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2, or (at your option) >> + * any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; see the file COPYING. If not, write to >> + * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA. >> + */ >> + >> +#ifndef _LINUX_SYSLOG_H >> +#define _LINUX_SYSLOG_H >> + >> +#define SYSLOG_FROM_CALL 0 >> +#define SYSLOG_FROM_FILE 1 >> + >> +int do_syslog(int type, char __user *buf, int count, bool from_file); >> + >> +#endif /* _LINUX_SYSLOG_H */ >> diff --git a/kernel/printk.c b/kernel/printk.c >> index 1751c45..1771b34 100644 >> --- a/kernel/printk.c >> +++ b/kernel/printk.c >> @@ -35,6 +35,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> >> @@ -273,14 +274,14 @@ static inline void boot_delay_msec(void) >> * 9 -- Return number of unread characters in the log buffer >> * 10 -- Return size of the log buffer >> */ >> -int do_syslog(int type, char __user *buf, int len) >> +int do_syslog(int type, char __user *buf, int len, bool from_file) >> { >> unsigned i, j, limit, count; >> int do_clear = 0; >> char c; >> int error = 0; >> >> - error = security_syslog(type); >> + error = security_syslog(type, from_file); >> if (error) >> return error; >> >> @@ -417,7 +418,7 @@ out: >> >> SYSCALL_DEFINE3(syslog, int, type, char __user *, buf, int, len) >> { >> - return do_syslog(type, buf, len); >> + return do_syslog(type, buf, len, SYSLOG_FROM_CALL); >> } >> >> /* >> diff --git a/security/commoncap.c b/security/commoncap.c >> index f800fdb..677fad9 100644 >> --- a/security/commoncap.c >> +++ b/security/commoncap.c >> @@ -27,6 +27,7 @@ >> #include >> #include >> #include >> +#include >> >> /* >> * If a non-root user executes a setuid-root binary in >> @@ -888,12 +889,16 @@ 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) >> +int cap_syslog(int type, bool from_file) >> { >> + /* /proc/kmsg can open be opened by CAP_SYS_ADMIN */ >> + if (type != 1 && from_file) >> + return 0; >> if ((type != 3 && type != 10) && !capable(CAP_SYS_ADMIN)) >> return -EPERM; >> return 0; >> diff --git a/security/security.c b/security/security.c >> index 24e060b..9a127ae 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -203,9 +203,9 @@ int security_quota_on(struct dentry *dentry) >> return security_ops->quota_on(dentry); >> } >> >> -int security_syslog(int type) >> +int security_syslog(int type, bool from_file) >> { >> - return security_ops->syslog(type); >> + return security_ops->syslog(type, from_file); >> } >> >> int security_settime(struct timespec *ts, struct timezone *tz) >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 9a2ee84..a4862a0 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -76,6 +76,7 @@ >> #include >> #include >> #include >> +#include >> >> #include "avc.h" >> #include "objsec.h" >> @@ -2049,11 +2050,11 @@ static int selinux_quota_on(struct dentry *dentry) >> return dentry_has_perm(cred, NULL, dentry, FILE__QUOTAON); >> } >> >> -static int selinux_syslog(int type) >> +static int selinux_syslog(int type, bool from_file) >> { >> int rc; >> >> - rc = cap_syslog(type); >> + rc = cap_syslog(type, from_file); >> if (rc) >> return rc; >> >> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c >> index 529c9ca..a5721b3 100644 >> --- a/security/smack/smack_lsm.c >> +++ b/security/smack/smack_lsm.c >> @@ -157,12 +157,12 @@ static int smack_ptrace_traceme(struct task_struct *ptp) >> * >> * Returns 0 on success, error code otherwise. >> */ >> -static int smack_syslog(int type) >> +static int smack_syslog(int type, bool from_file) >> { >> int rc; >> char *sp = current_security(); >> >> - rc = cap_syslog(type); >> + rc = cap_syslog(type, from_file); >> if (rc != 0) >> return rc; >> >> -- >> 1.6.5 >> >> >> -- >> Kees Cook >> Ubuntu Security Team > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/