Maverick pull request, soft/hard link protection

Tim Gardner timg at tpi.com
Fri May 21 15:03:46 UTC 2010 

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 <kees at ubuntu.com>
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 <kees.cook at canonical.com>
---
 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 <linux/securebits.h>
 #include <linux/syslog.h>
 
+/* 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 <kees at ubuntu.com>
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 <kees.cook at canonical.com>
---
 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.
-- 
1.7.0.4

More information about the kernel-team mailing list