From patchwork Fri May 21 15:03:46 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 53162 X-Patchwork-Delegate: leann.ogasawara@canonical.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id A41DFB7D59 for ; Sat, 22 May 2010 01:04:01 +1000 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.69) (envelope-from ) id 1OFTlU-0008OH-UZ; Fri, 21 May 2010 16:03:57 +0100 Received: from mail.tpi.com ([70.99.223.143]) by chlorine.canonical.com with esmtp (Exim 4.69) (envelope-from ) id 1OFTlR-0008NT-So for kernel-team@lists.ubuntu.com; Fri, 21 May 2010 16:03:54 +0100 Received: from sepang.rtg.net (unknown [10.0.2.5]) by mail.tpi.com (Postfix) with ESMTP id ECF73245202; Fri, 21 May 2010 08:03:15 -0700 (PDT) Received: by sepang.rtg.net (Postfix, from userid 1000) id EB8B6F899A; Fri, 21 May 2010 09:03:46 -0600 (MDT) To: leann.ogasawara@canonical.com Subject: Maverick pull request, soft/hard link protection Message-Id: <20100521150346.EB8B6F899A@sepang.rtg.net> Date: Fri, 21 May 2010 09:03:46 -0600 (MDT) From: timg@tpi.com (Tim Gardner) Cc: kernel-team@lists.ubuntu.com X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.9 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Mime-version: 1.0 Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com I've boot tested with these soft and hard link patches and can find no obvious signs of carnage. The following changes since commit 82222c9b7caca5df688c007c767bc3085fae821e: Chase Douglas (1): Revert "UBUNTU: SAUCE: Disable function tracing after hitting __schedule_bug" are available in the git repository at: git://kernel.ubuntu.com/rtg/ubuntu-maverick.git links Kees Cook (2): UBUNTU: SAUCE: fs: block cross-uid sticky symlinks UBUNTU: SAUCE: fs: block hardlinks to non-accessible sources include/linux/security.h | 5 +++ kernel/sysctl.c | 18 ++++++++++++ security/apparmor/lsm.c | 5 +++- security/capability.c | 12 -------- security/commoncap.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 13 deletions(-) From ef2f4f2445a49b395058d08484cfa6cd59ee474a Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 11 May 2010 16:51:24 -0700 Subject: [PATCH 1/2] UBUNTU: SAUCE: fs: block cross-uid sticky symlinks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A long-standing class of security issues is the symlink-based time-of-check-time-of-use race, most commonly seen in world-writable directories like /tmp. The common method of exploitation of this flaw is to cross privilege boundaries when following a given symlink (i.e. a root process follows a symlink belonging to another user). The solution is to not permit symlinks to be followed when users do not match, but only in a world-writable sticky directory (with an additional improvement that the directory owner's symlinks can always be followed, regardless who is following them). Some pointers to the history of earlier discussion that I could find: 1996 Aug, Zygo Blaxell http://marc.info/?l=bugtraq&m=87602167419830&w=2 1996 Oct, Andrew Tridgell http://lkml.indiana.edu/hypermail/linux/kernel/9610.2/0086.html 1997 Dec, Albert D Cahalan http://lkml.org/lkml/1997/12/16/4 2005 Feb, Lorenzo Hernández García-Hierro http://lkml.indiana.edu/hypermail/linux/kernel/0502.0/1896.html Past objections and rebuttals could be summarized as: - Violates POSIX. - POSIX didn't consider this situation, and it's not useful to follow a broken specification at the cost of security. Also, please reference where POSIX says this. - Might break unknown applications that use this feature. - Applications that break because of the change are easy to spot and fix. Applications that are vulnerable to symlink ToCToU by not having the change aren't. - Applications should just use mkstemp() or O_CREATE|O_EXCL. - True, but applications are not perfect, and new software is written all the time that makes these mistakes; blocking this flaw at the kernel is a single solution to the entire class of vulnerability. This patch is based on the patch in grsecurity, which is similar to the patch in Openwall. I have added a sysctl to toggle the behavior back to the old handling via /proc/sys/fs/weak-sticky-symlinks, as well as a ratelimited deprecation warning. Signed-off-by: Kees Cook --- include/linux/security.h | 1 + kernel/sysctl.c | 8 ++++++++ security/capability.c | 6 ------ security/commoncap.c | 24 ++++++++++++++++++++++++ 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/include/linux/security.h b/include/linux/security.h index 3158dd9..92eca95 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -67,6 +67,7 @@ extern int cap_inode_setxattr(struct dentry *dentry, const char *name, extern int cap_inode_removexattr(struct dentry *dentry, const char *name); extern int cap_inode_need_killpriv(struct dentry *dentry); extern int cap_inode_killpriv(struct dentry *dentry); +extern int cap_inode_follow_link(struct dentry *dentry, struct nameidata *nd); extern int cap_file_mmap(struct file *file, unsigned long reqprot, unsigned long prot, unsigned long flags, unsigned long addr, unsigned long addr_only); diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 8686b0f..36a104c 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -86,6 +86,7 @@ extern int sysctl_oom_dump_tasks; extern int max_threads; extern int core_uses_pid; extern int suid_dumpable; +extern int weak_sticky_symlinks; extern char core_pattern[]; extern unsigned int core_pipe_limit; extern int pid_max; @@ -1416,6 +1417,13 @@ static struct ctl_table fs_table[] = { .extra1 = &zero, .extra2 = &two, }, + { + .procname = "weak-sticky-symlinks", + .data = &weak_sticky_symlinks, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = &proc_dointvec, + }, #if defined(CONFIG_BINFMT_MISC) || defined(CONFIG_BINFMT_MISC_MODULE) { .procname = "binfmt_misc", diff --git a/security/capability.c b/security/capability.c index 4875142..d4633f3 100644 --- a/security/capability.c +++ b/security/capability.c @@ -200,12 +200,6 @@ static int cap_inode_readlink(struct dentry *dentry) return 0; } -static int cap_inode_follow_link(struct dentry *dentry, - struct nameidata *nameidata) -{ - return 0; -} - static int cap_inode_permission(struct inode *inode, int mask) { return 0; diff --git a/security/commoncap.c b/security/commoncap.c index 6166973..4a6b670 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -29,6 +29,9 @@ #include #include +/* sysctl for symlink permissions checking */ +int weak_sticky_symlinks; + /* * If a non-root user executes a setuid-root binary in * !secure(SECURE_NOROOT) mode, then we raise capabilities. @@ -281,6 +284,27 @@ int cap_inode_killpriv(struct dentry *dentry) return inode->i_op->removexattr(dentry, XATTR_NAME_CAPS); } +int cap_inode_follow_link(struct dentry *dentry, + struct nameidata *nameidata) +{ + const struct inode *parent = dentry->d_parent->d_inode; + const struct inode *inode = dentry->d_inode; + const struct cred *cred = current_cred(); + + if (weak_sticky_symlinks) + return 0; + + if (S_ISLNK(inode->i_mode) && (parent->i_mode & S_ISVTX) && + (parent->i_mode & S_IWOTH) && (parent->i_uid != inode->i_uid) && + (cred->fsuid != inode->i_uid)) { + printk_ratelimited(KERN_INFO "deprecated sticky-directory" + " non-matching uid symlink following was attempted" + " by: %s\n", current->comm); + return -EACCES; + } + return 0; +} + /* * Calculate the new process capability sets from the capability sets attached * to a file. -- 1.7.0.4 From 508159be6bda4cd7d721e6b9c05a58ad3b87af07 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 12 May 2010 09:03:08 -0700 Subject: [PATCH 2/2] UBUNTU: SAUCE: fs: block hardlinks to non-accessible sources Hardlinks can be abused in a similar fashion to symlinks above, but they are not limited to world-writable directories. If /etc and /home are on the same partition, a regular user can create a hardlink to /etc/shadow in their home directory. While it retains the original owner and permissions, it is possible for privileged programs that are otherwise symlink-safe to mistakenly access the file through its hardlink. Additionally, a very minor untraceable quota-bypassing local denial of service is possible by an attacker exhausting disk space by filling a world-writable directory with hardlinks. The solution is to not allow the creation of hardlinks to files that a given user would be unable to read or write originally, or are otherwise sensitive. Some links to the history of its discussion: 1997 Dec, Yuri Kuzmenko http://lkml.org/lkml/1997/12/29/20 2002 Apr, Chris Wright http://lkml.org/lkml/2002/4/13/99 Past objections and rebuttals could be summarized as: - Violates POSIX. - POSIX didn't consider this situation, and it's not useful to follow a broken specification at the cost of security. Also, please reference where POSIX says this. - Might break atd, courier, and other unknown applications that use this feature. - These applications are easy to spot and can be tested and fixed. Applications that are vulnerable to hardlink attacks by not having the change aren't. - Applications should correctly drop privileges before attempting to access user files. - True, but applications are not perfect, and new software is written all the time that makes these mistakes; blocking this flaw at the kernel is a single solution to the entire class of vulnerability. This patch is based on the patch in grsecurity, which is similar to the patch in Openwall. I have added a sysctl to toggle the behavior back to the old handling via /proc/sys/fs/weak-nonaccess-hardlinks, as well as a ratelimited deprecation warning. Signed-off-by: Kees Cook --- include/linux/security.h | 4 ++++ kernel/sysctl.c | 10 ++++++++++ security/apparmor/lsm.c | 5 ++++- security/capability.c | 6 ------ security/commoncap.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 7 deletions(-) diff --git a/include/linux/security.h b/include/linux/security.h index 92eca95..6c1a6bf 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -79,6 +79,10 @@ 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); +#ifdef CONFIG_SECURITY_PATH +extern int cap_path_link(struct dentry *old_dentry, struct path *new_dir, + struct dentry *new_dentry); +#endif struct msghdr; struct sk_buff; diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 36a104c..4f3ffd0 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -87,6 +87,7 @@ extern int max_threads; extern int core_uses_pid; extern int suid_dumpable; extern int weak_sticky_symlinks; +extern int weak_nonaccess_hardlinks; extern char core_pattern[]; extern unsigned int core_pipe_limit; extern int pid_max; @@ -1424,6 +1425,15 @@ static struct ctl_table fs_table[] = { .mode = 0644, .proc_handler = &proc_dointvec, }, +#ifdef CONFIG_SECURITY_PATH + { + .procname = "weak-nonaccess-hardlinks", + .data = &weak_nonaccess_hardlinks, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = &proc_dointvec, + }, +#endif #if defined(CONFIG_BINFMT_MISC) || defined(CONFIG_BINFMT_MISC_MODULE) { .procname = "binfmt_misc", diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 814b086..82e222d 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -317,7 +317,10 @@ static int apparmor_path_link(struct dentry *old_dentry, struct path *new_dir, struct dentry *new_dentry) { struct aa_profile *profile; - int error = 0; + int error = 0, rc; + + if ( (rc = cap_path_link(old_dentry, new_dir, new_dentry)) ) + return rc; if (!mediated_filesystem(old_dentry->d_inode)) return 0; diff --git a/security/capability.c b/security/capability.c index d4633f3..75eb6b0 100644 --- a/security/capability.c +++ b/security/capability.c @@ -285,12 +285,6 @@ static int cap_path_symlink(struct path *dir, struct dentry *dentry, return 0; } -static int cap_path_link(struct dentry *old_dentry, struct path *new_dir, - struct dentry *new_dentry) -{ - return 0; -} - static int cap_path_rename(struct path *old_path, struct dentry *old_dentry, struct path *new_path, struct dentry *new_dentry) { diff --git a/security/commoncap.c b/security/commoncap.c index 4a6b670..5f52c33 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -31,6 +31,8 @@ /* sysctl for symlink permissions checking */ int weak_sticky_symlinks; +/* sysctl for hardlink permissions checking */ +int weak_nonaccess_hardlinks; /* * If a non-root user executes a setuid-root binary in @@ -305,6 +307,48 @@ int cap_inode_follow_link(struct dentry *dentry, return 0; } +#ifdef CONFIG_SECURITY_PATH +/* + * cap_path_link - verify that hardlinking is allowed + * @old_dentry: the source inode/dentry to hardlink from + * @new_dir: target directory + * @new_dentry: the target inode/dentry to hardlink to + * + * Block hardlink when all of: + * - fsuid does not match inode + * - not CAP_FOWNER + * - and at least one of: + * - inode is not a regular file + * - inode is setuid + * - inode is setgid and group-exec + * - access failure for read or write + * + * Returns 0 if successful, -ve on error. + */ +int cap_path_link(struct dentry *old_dentry, struct path *new_dir, + struct dentry *new_dentry) +{ + struct inode *inode = old_dentry->d_inode; + const int mode = inode->i_mode; + const struct cred *cred = current_cred(); + + if (weak_nonaccess_hardlinks) return 0; + + if (cred->fsuid != inode->i_uid && + (!S_ISREG(mode) || (mode & S_ISUID) || + ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) || + (generic_permission(inode, MAY_READ | MAY_WRITE, NULL))) && + !capable(CAP_FOWNER)) { + printk_ratelimited(KERN_INFO "deprecated non-accessible" + " hardlink creation was attempted by: %s\n", + current->comm); + return -EPERM; + } + + return 0; +} +#endif /* CONFIG_SECURITY_PATH */ + /* * Calculate the new process capability sets from the capability sets attached * to a file.