On Wed, Apr 24, 2013 at 07:44:33PM +0200, Kay Sievers wrote: > On Tue, Apr 9, 2013 at 6:33 PM, Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > On Tue, Apr 9, 2013 at 8:48 AM, Josh Boyer <jwboyer@xxxxxxxxxx> 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 <lists@xxxxxxxxxxxxxxx> > >> CC: stable@xxxxxxxxxxxxxxx > >> Signed-off-by: Eric Paris <eparis@xxxxxxxxxx> > >> Signed-off-by: Josh Boyer <jwboyer@xxxxxxxxxx> > > > > Thanks! > > > > Acked-by: Kees Cook <keescook@xxxxxxxxxxxx> > > 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 <jwboyer@xxxxxxxxxx> 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 <lists@xxxxxxxxxxxxxxx> CC: stable@xxxxxxxxxxxxxxx Signed-off-by: Eric Paris <eparis@xxxxxxxxxx> Signed-off-by: Josh Boyer <jwboyer@xxxxxxxxxx> --- 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 stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html