Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756626Ab3DXR6r (ORCPT ); Wed, 24 Apr 2013 13:58:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53863 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756300Ab3DXR6p (ORCPT ); Wed, 24 Apr 2013 13:58:45 -0400 Date: Wed, 24 Apr 2013 13:58:35 -0400 From: Josh Boyer To: Kay Sievers Cc: Kees Cook , Andrew Morton , Eric Paris , Linus Torvalds , Christian Kujau , "# 3.4.x" , LKML , kzak@redhat.com Subject: Re: [PATCH v2] kmsg: Honor dmesg_restrict sysctl on /dev/kmsg Message-ID: <20130424175835.GF15272@hansolo.jdub.homelinux.org> References: <20130322145448.f44f9d10a36620c1c11535b7@linux-foundation.org> <20130322221444.GJ15821@hansolo.jdub.homelinux.org> <1256775981.281402.1364864751771.JavaMail.root@redhat.com> <20130409005050.GE18176@hansolo.jdub.homelinux.org> <20130409154820.GE32476@hansolo.jdub.homelinux.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6473 Lines: 209 On Wed, Apr 24, 2013 at 07:44:33PM +0200, Kay Sievers wrote: > On Tue, Apr 9, 2013 at 6:33 PM, Kees Cook wrote: > > On Tue, Apr 9, 2013 at 8:48 AM, Josh Boyer wrote: > >> The dmesg_restrict sysctl currently covers the syslog method for access > >> dmesg, however /dev/kmsg isn't covered by the same protections. Most > >> people haven't noticed because util-linux dmesg(1) defaults to using the > >> syslog method for access in older versions. With util-linux dmesg(1) > >> defaults to reading directly from /dev/kmsg. > >> > >> Fix this by reworking all of the access methods to use the > >> check_syslog_permissions function and adding checks to devkmsg_open and > >> devkmsg_read. > >> > >> This fixes https://bugzilla.redhat.com/show_bug.cgi?id=903192 > >> > >> Reported-by: Christian Kujau > >> CC: stable@vger.kernel.org > >> Signed-off-by: Eric Paris > >> Signed-off-by: Josh Boyer > > > > Thanks! > > > > Acked-by: Kees Cook > > If that's the version currently in Fedora, we just cannot do this. > https://bugzilla.redhat.com/show_bug.cgi?id=952655 > > /dev/kmsg is supposed, and was added, to be a sane alternative to > syslog(). It is already used in dmesg(1) which is now broken with this > patch. > > The access rules for /dev/kmsg should follow the access rules of > syslog(), and not be any stricter. I haven't tested it yet, but I think something like this should work while still honoring dmesg_restrict. I'll test it out while the rest of you debate things. josh From: Josh Boyer Date: Tue, 9 Apr 2013 11:08:13 -0400 Subject: [PATCH v3] kmsg: Honor dmesg_restrict sysctl on /dev/kmsg The dmesg_restrict sysctl currently covers the syslog method for access dmesg, however /dev/kmsg isn't covered by the same protections. Most people haven't noticed because util-linux dmesg(1) defaults to using the syslog method for access in older versions. With util-linux dmesg(1) defaults to reading directly from /dev/kmsg. Fix this by reworking all of the access methods to use the check_syslog_permissions function and adding checks to devkmsg_open and devkmsg_read. This fixes https://bugzilla.redhat.com/show_bug.cgi?id=903192 Reported-by: Christian Kujau CC: stable@vger.kernel.org Signed-off-by: Eric Paris Signed-off-by: Josh Boyer --- v3: Allow devkmsg_open to work without CAP_SYSLOG, but still make devkmsg_read honor dmesg_restrict kernel/printk.c | 91 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 47 insertions(+), 44 deletions(-) diff --git a/kernel/printk.c b/kernel/printk.c index abbdd9e..2d7be05 100644 --- a/kernel/printk.c +++ b/kernel/printk.c @@ -368,6 +368,46 @@ static void log_store(int facility, int level, log_next_seq++; } +#ifdef CONFIG_SECURITY_DMESG_RESTRICT +int dmesg_restrict = 1; +#else +int dmesg_restrict; +#endif + +static int syslog_action_restricted(int type) +{ + if (dmesg_restrict) + return 1; + /* Unless restricted, we allow "read all" and "get buffer size" for everybody */ + return type != SYSLOG_ACTION_READ_ALL && type != SYSLOG_ACTION_SIZE_BUFFER; +} + +static int check_syslog_permissions(int type, bool from_file) +{ + /* + * If this is from /proc/kmsg and we've already opened it, then we've + * already done the capabilities checks at open time. + */ + if (from_file && type != SYSLOG_ACTION_OPEN) + goto ok; + + if (syslog_action_restricted(type)) { + if (capable(CAP_SYSLOG)) + goto ok; + /* For historical reasons, accept CAP_SYS_ADMIN too, with a warning */ + if (capable(CAP_SYS_ADMIN)) { + printk_once(KERN_WARNING "%s (%d): " + "Attempt to access syslog with CAP_SYS_ADMIN " + "but no CAP_SYSLOG (deprecated).\n", + current->comm, task_pid_nr(current)); + goto ok; + } + return -EPERM; + } +ok: + return security_syslog(type); +} + /* /dev/kmsg - userspace message inject/listen interface */ struct devkmsg_user { u64 seq; @@ -443,10 +483,16 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf, char cont = '-'; size_t len; ssize_t ret; + int err; if (!user) return -EBADF; + err = check_syslog_permissions(SYSLOG_ACTION_READ_ALL, + SYSLOG_FROM_CALL); + if (err) + return err; + ret = mutex_lock_interruptible(&user->lock); if (ret) return ret; @@ -624,7 +670,7 @@ static int devkmsg_open(struct inode *inode, struct file *file) if ((file->f_flags & O_ACCMODE) == O_WRONLY) return 0; - err = security_syslog(SYSLOG_ACTION_READ_ALL); + err = check_syslog_permissions(SYSLOG_ACTION_READ_ALL, SYSLOG_FROM_FILE); if (err) return err; @@ -817,45 +863,6 @@ static inline void boot_delay_msec(int level) } #endif -#ifdef CONFIG_SECURITY_DMESG_RESTRICT -int dmesg_restrict = 1; -#else -int dmesg_restrict; -#endif - -static int syslog_action_restricted(int type) -{ - if (dmesg_restrict) - return 1; - /* Unless restricted, we allow "read all" and "get buffer size" for everybody */ - return type != SYSLOG_ACTION_READ_ALL && type != SYSLOG_ACTION_SIZE_BUFFER; -} - -static int check_syslog_permissions(int type, bool from_file) -{ - /* - * If this is from /proc/kmsg and we've already opened it, then we've - * already done the capabilities checks at open time. - */ - if (from_file && type != SYSLOG_ACTION_OPEN) - return 0; - - if (syslog_action_restricted(type)) { - if (capable(CAP_SYSLOG)) - return 0; - /* For historical reasons, accept CAP_SYS_ADMIN too, with a warning */ - if (capable(CAP_SYS_ADMIN)) { - printk_once(KERN_WARNING "%s (%d): " - "Attempt to access syslog with CAP_SYS_ADMIN " - "but no CAP_SYSLOG (deprecated).\n", - current->comm, task_pid_nr(current)); - return 0; - } - return -EPERM; - } - return 0; -} - #if defined(CONFIG_PRINTK_TIME) static bool printk_time = 1; #else @@ -1131,10 +1138,6 @@ int do_syslog(int type, char __user *buf, int len, bool from_file) if (error) goto out; - error = security_syslog(type); - if (error) - return error; - switch (type) { case SYSLOG_ACTION_CLOSE: /* Close log */ break; -- 1.8.1.4 -- 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/