All of lore.kernel.org
 help / color / mirror / Atom feed
From: Salvatore Mesoraca <s.mesoraca16@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>,
	linux-fsdevel@vger.kernel.org,
	Salvatore Mesoraca <s.mesoraca16@gmail.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Jann Horn <jannh@google.com>, Kees Cook <keescook@chromium.org>,
	Solar Designer <solar@openwall.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: [PATCH v3 2/2] Protected O_CREAT open in sticky directories
Date: Wed, 22 Nov 2017 09:01:46 +0100	[thread overview]
Message-ID: <1511337706-8297-3-git-send-email-s.mesoraca16@gmail.com> (raw)
In-Reply-To: <1511337706-8297-1-git-send-email-s.mesoraca16@gmail.com>

Disallows O_CREAT open missing the O_EXCL flag, in world or
group writable directories, even if the file doesn't exist yet.
With few exceptions (e.g. shared lock files based on flock())
if a program tries to open a file, in a sticky directory,
with the O_CREAT flag and without the O_EXCL, it probably has a bug.
This feature allows to detect and potentially block programs that
act this way, it can be used to find vulnerabilities (like those
prevented by patch #1) and to do policy enforcement.

Suggested-by: Solar Designer <solar@openwall.com>
Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
---
 Documentation/sysctl/fs.txt | 30 ++++++++++++++++++++++++
 fs/namei.c                  | 56 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h          |  1 +
 kernel/sysctl.c             |  9 ++++++++
 4 files changed, 96 insertions(+)

diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt
index f3cf2cd..7f24b4f 100644
--- a/Documentation/sysctl/fs.txt
+++ b/Documentation/sysctl/fs.txt
@@ -37,6 +37,7 @@ Currently, these files are in /proc/sys/fs:
 - protected_fifos
 - protected_hardlinks
 - protected_regular
+- protected_sticky_child_create
 - protected_symlinks
 - suid_dumpable
 - super-max
@@ -238,6 +239,35 @@ When set to "2" it also applies to group writable sticky directories.
 
 ==============================================================
 
+protected_sticky_child_create:
+
+An O_CREAT open missing the O_EXCL flag in a sticky directory is,
+often, a bug or a synthom of the fact that the program is not
+using appropriate procedures to access sticky directories.
+This protection allow to detect and possibly block these unsafe
+open invocations, even if the files don't exist yet.
+Though should be noted that, sometimes, it's OK to open a file
+with O_CREAT and without O_EXCL (e.g. shared lock files based
+on flock()), for this reason values above 2 should be set
+with care.
+
+When set to "0" the protection is disabled.
+
+When set to "1", notify about O_CREAT open missing the O_EXCL flag
+in world writable sticky directories.
+
+When set to "2", notify about O_CREAT open missing the O_EXCL flag
+in world or group writable sticky directories.
+
+When set to "3", block O_CREAT open missing the O_EXCL flag
+in world writable sticky directories and notify (but don't block)
+in group writable sticky directories.
+
+When set to "4", block O_CREAT open missing the O_EXCL flag
+in world writable and group writable sticky directories.
+
+==============================================================
+
 protected_symlinks:
 
 A long-standing class of security issues is the symlink-based
diff --git a/fs/namei.c b/fs/namei.c
index 92992ad..fcee423 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -904,6 +904,7 @@ static inline void put_link(struct nameidata *nd)
 int sysctl_protected_hardlinks __read_mostly = 0;
 int sysctl_protected_fifos __read_mostly;
 int sysctl_protected_regular __read_mostly;
+int sysctl_protected_sticky_child_create __read_mostly;
 
 /**
  * may_follow_link - Check symlink following for unsafe situations
@@ -1065,6 +1066,53 @@ static int may_create_in_sticky(struct dentry * const dir,
 	return 0;
 }
 
+/**
+ * may_create_no_excl - Detect and possibly block unsafe O_CREAT open
+ *			without O_EXCL.
+ * @dir: the stick parent directory
+ * @name: the file name
+ * @inode: the inode of the file to open (can be NULL to skip uid checks)
+ *
+ * When sysctl_protected_sticky_child_create is set to "0" the
+ * protection is disabled.
+ * When it's set to "1", notify about O_CREAT open missing the O_EXCL flag
+ * in world writable sticky directories.
+ * When it's set to "2", notify about O_CREAT open missing the O_EXCL flag
+ * in group writable sticky directories.
+ * When it's set to "3", block O_CREAT open missing the O_EXCL flag
+ * in world writable sticky directories and notify (but don't block)
+ * in group writable sticky directories.
+ * When it's set to "4", block O_CREAT open missing the O_EXCL flag
+ * in world writable and group writable sticky directories.
+ *
+ * Returns 0 if the open is allowed, -ve on error.
+ */
+static int may_create_no_excl(struct dentry * const dir,
+			      const unsigned char * const name,
+			      struct inode * const inode)
+{
+	umode_t mode = dir->d_inode->i_mode;
+
+	if (likely(!(mode & S_ISVTX)))
+		return 0;
+	if (inode && uid_eq(current_fsuid(), inode->i_uid))
+		return 0;
+
+	if ((sysctl_protected_sticky_child_create && likely(mode & 0002)) ||
+	    (sysctl_protected_sticky_child_create >= 2 && mode & 0020)) {
+		pr_notice_ratelimited("unsafe O_CREAT open (missing O_EXCL) of '%s' in a sticky directory by UID %u, EUID %u, process %s:%d.\n",
+				      name,
+				      from_kuid(&init_user_ns, current_uid()),
+				      from_kuid(&init_user_ns, current_euid()),
+				      current->comm, current->pid);
+		if (sysctl_protected_sticky_child_create >= 4 ||
+		    (sysctl_protected_sticky_child_create == 3 &&
+		     likely(mode & 0002)))
+			return -EACCES;
+	}
+	return 0;
+}
+
 static __always_inline
 const char *get_link(struct nameidata *nd)
 {
@@ -3256,6 +3304,11 @@ static int lookup_open(struct nameidata *nd, struct path *path,
 			error = -EACCES;
 			goto out_dput;
 		}
+		if (!(open_flag & O_EXCL)) {
+			error = may_create_no_excl(dir, nd->last.name, NULL);
+			if (unlikely(error))
+				goto out_dput;
+		}
 		error = dir_inode->i_op->create(dir_inode, dentry, mode,
 						open_flag & O_EXCL);
 		if (error)
@@ -3422,6 +3475,9 @@ static int do_last(struct nameidata *nd,
 		error = may_create_in_sticky(dir, nd->last.name, inode);
 		if (unlikely(error))
 			goto out;
+		error = may_create_no_excl(dir, nd->last.name, inode);
+		if (unlikely(error))
+			goto out;
 	}
 	error = -ENOTDIR;
 	if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry))
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6fb45a52..3ab37e0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -74,6 +74,7 @@
 extern int sysctl_protected_hardlinks;
 extern int sysctl_protected_fifos;
 extern int sysctl_protected_regular;
+extern int sysctl_protected_sticky_child_create;
 
 typedef __kernel_rwf_t rwf_t;
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 590fbc9..012c739 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1817,6 +1817,15 @@ static int sysrq_sysctl_handler(struct ctl_table *table, int write,
 		.extra2		= &two,
 	},
 	{
+		.procname	= "protected_sticky_child_create",
+		.data		= &sysctl_protected_sticky_child_create,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &four,
+	},
+	{
 		.procname	= "suid_dumpable",
 		.data		= &suid_dumpable,
 		.maxlen		= sizeof(int),
-- 
1.9.1

WARNING: multiple messages have this Message-ID (diff)
From: Salvatore Mesoraca <s.mesoraca16@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>,
	linux-fsdevel@vger.kernel.org,
	Salvatore Mesoraca <s.mesoraca16@gmail.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Jann Horn <jannh@google.com>, Kees Cook <keescook@chromium.org>,
	Solar Designer <solar@openwall.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: [kernel-hardening] [PATCH v3 2/2] Protected O_CREAT open in sticky directories
Date: Wed, 22 Nov 2017 09:01:46 +0100	[thread overview]
Message-ID: <1511337706-8297-3-git-send-email-s.mesoraca16@gmail.com> (raw)
In-Reply-To: <1511337706-8297-1-git-send-email-s.mesoraca16@gmail.com>

Disallows O_CREAT open missing the O_EXCL flag, in world or
group writable directories, even if the file doesn't exist yet.
With few exceptions (e.g. shared lock files based on flock())
if a program tries to open a file, in a sticky directory,
with the O_CREAT flag and without the O_EXCL, it probably has a bug.
This feature allows to detect and potentially block programs that
act this way, it can be used to find vulnerabilities (like those
prevented by patch #1) and to do policy enforcement.

Suggested-by: Solar Designer <solar@openwall.com>
Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
---
 Documentation/sysctl/fs.txt | 30 ++++++++++++++++++++++++
 fs/namei.c                  | 56 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h          |  1 +
 kernel/sysctl.c             |  9 ++++++++
 4 files changed, 96 insertions(+)

diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt
index f3cf2cd..7f24b4f 100644
--- a/Documentation/sysctl/fs.txt
+++ b/Documentation/sysctl/fs.txt
@@ -37,6 +37,7 @@ Currently, these files are in /proc/sys/fs:
 - protected_fifos
 - protected_hardlinks
 - protected_regular
+- protected_sticky_child_create
 - protected_symlinks
 - suid_dumpable
 - super-max
@@ -238,6 +239,35 @@ When set to "2" it also applies to group writable sticky directories.
 
 ==============================================================
 
+protected_sticky_child_create:
+
+An O_CREAT open missing the O_EXCL flag in a sticky directory is,
+often, a bug or a synthom of the fact that the program is not
+using appropriate procedures to access sticky directories.
+This protection allow to detect and possibly block these unsafe
+open invocations, even if the files don't exist yet.
+Though should be noted that, sometimes, it's OK to open a file
+with O_CREAT and without O_EXCL (e.g. shared lock files based
+on flock()), for this reason values above 2 should be set
+with care.
+
+When set to "0" the protection is disabled.
+
+When set to "1", notify about O_CREAT open missing the O_EXCL flag
+in world writable sticky directories.
+
+When set to "2", notify about O_CREAT open missing the O_EXCL flag
+in world or group writable sticky directories.
+
+When set to "3", block O_CREAT open missing the O_EXCL flag
+in world writable sticky directories and notify (but don't block)
+in group writable sticky directories.
+
+When set to "4", block O_CREAT open missing the O_EXCL flag
+in world writable and group writable sticky directories.
+
+==============================================================
+
 protected_symlinks:
 
 A long-standing class of security issues is the symlink-based
diff --git a/fs/namei.c b/fs/namei.c
index 92992ad..fcee423 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -904,6 +904,7 @@ static inline void put_link(struct nameidata *nd)
 int sysctl_protected_hardlinks __read_mostly = 0;
 int sysctl_protected_fifos __read_mostly;
 int sysctl_protected_regular __read_mostly;
+int sysctl_protected_sticky_child_create __read_mostly;
 
 /**
  * may_follow_link - Check symlink following for unsafe situations
@@ -1065,6 +1066,53 @@ static int may_create_in_sticky(struct dentry * const dir,
 	return 0;
 }
 
+/**
+ * may_create_no_excl - Detect and possibly block unsafe O_CREAT open
+ *			without O_EXCL.
+ * @dir: the stick parent directory
+ * @name: the file name
+ * @inode: the inode of the file to open (can be NULL to skip uid checks)
+ *
+ * When sysctl_protected_sticky_child_create is set to "0" the
+ * protection is disabled.
+ * When it's set to "1", notify about O_CREAT open missing the O_EXCL flag
+ * in world writable sticky directories.
+ * When it's set to "2", notify about O_CREAT open missing the O_EXCL flag
+ * in group writable sticky directories.
+ * When it's set to "3", block O_CREAT open missing the O_EXCL flag
+ * in world writable sticky directories and notify (but don't block)
+ * in group writable sticky directories.
+ * When it's set to "4", block O_CREAT open missing the O_EXCL flag
+ * in world writable and group writable sticky directories.
+ *
+ * Returns 0 if the open is allowed, -ve on error.
+ */
+static int may_create_no_excl(struct dentry * const dir,
+			      const unsigned char * const name,
+			      struct inode * const inode)
+{
+	umode_t mode = dir->d_inode->i_mode;
+
+	if (likely(!(mode & S_ISVTX)))
+		return 0;
+	if (inode && uid_eq(current_fsuid(), inode->i_uid))
+		return 0;
+
+	if ((sysctl_protected_sticky_child_create && likely(mode & 0002)) ||
+	    (sysctl_protected_sticky_child_create >= 2 && mode & 0020)) {
+		pr_notice_ratelimited("unsafe O_CREAT open (missing O_EXCL) of '%s' in a sticky directory by UID %u, EUID %u, process %s:%d.\n",
+				      name,
+				      from_kuid(&init_user_ns, current_uid()),
+				      from_kuid(&init_user_ns, current_euid()),
+				      current->comm, current->pid);
+		if (sysctl_protected_sticky_child_create >= 4 ||
+		    (sysctl_protected_sticky_child_create == 3 &&
+		     likely(mode & 0002)))
+			return -EACCES;
+	}
+	return 0;
+}
+
 static __always_inline
 const char *get_link(struct nameidata *nd)
 {
@@ -3256,6 +3304,11 @@ static int lookup_open(struct nameidata *nd, struct path *path,
 			error = -EACCES;
 			goto out_dput;
 		}
+		if (!(open_flag & O_EXCL)) {
+			error = may_create_no_excl(dir, nd->last.name, NULL);
+			if (unlikely(error))
+				goto out_dput;
+		}
 		error = dir_inode->i_op->create(dir_inode, dentry, mode,
 						open_flag & O_EXCL);
 		if (error)
@@ -3422,6 +3475,9 @@ static int do_last(struct nameidata *nd,
 		error = may_create_in_sticky(dir, nd->last.name, inode);
 		if (unlikely(error))
 			goto out;
+		error = may_create_no_excl(dir, nd->last.name, inode);
+		if (unlikely(error))
+			goto out;
 	}
 	error = -ENOTDIR;
 	if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry))
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6fb45a52..3ab37e0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -74,6 +74,7 @@
 extern int sysctl_protected_hardlinks;
 extern int sysctl_protected_fifos;
 extern int sysctl_protected_regular;
+extern int sysctl_protected_sticky_child_create;
 
 typedef __kernel_rwf_t rwf_t;
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 590fbc9..012c739 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1817,6 +1817,15 @@ static int sysrq_sysctl_handler(struct ctl_table *table, int write,
 		.extra2		= &two,
 	},
 	{
+		.procname	= "protected_sticky_child_create",
+		.data		= &sysctl_protected_sticky_child_create,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &four,
+	},
+	{
 		.procname	= "suid_dumpable",
 		.data		= &suid_dumpable,
 		.maxlen		= sizeof(int),
-- 
1.9.1

  parent reply	other threads:[~2017-11-22  8:02 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-22  8:01 [PATCH v3 0/2] Restrict dangerous open in sticky directories Salvatore Mesoraca
2017-11-22  8:01 ` [kernel-hardening] " Salvatore Mesoraca
2017-11-22  8:01 ` [PATCH v3 1/2] Protected FIFOs and regular files Salvatore Mesoraca
2017-11-22  8:01   ` [kernel-hardening] " Salvatore Mesoraca
2017-11-23 22:43   ` Tobin C. Harding
2017-11-24  8:24     ` Salvatore Mesoraca
2017-11-22  8:01 ` Salvatore Mesoraca [this message]
2017-11-22  8:01   ` [kernel-hardening] [PATCH v3 2/2] Protected O_CREAT open in sticky directories Salvatore Mesoraca
2017-11-22 12:03   ` Pavel Vasilyev
2017-11-24  8:28     ` Salvatore Mesoraca
2017-11-22 13:22   ` Matthew Wilcox
2017-11-22 13:22     ` [kernel-hardening] " Matthew Wilcox
2017-11-22 14:38     ` Pavel Vasilyev
2017-11-24  8:29     ` Salvatore Mesoraca
2017-11-24  8:29       ` [kernel-hardening] " Salvatore Mesoraca
2017-11-22 16:51   ` Alan Cox
2017-11-22 16:51     ` [kernel-hardening] " Alan Cox
2017-11-24  8:31     ` Salvatore Mesoraca
2017-11-24  8:31       ` [kernel-hardening] " Salvatore Mesoraca
2017-11-24 10:53     ` David Laight
2017-11-24 10:53       ` [kernel-hardening] " David Laight
2017-11-24 11:43       ` Salvatore Mesoraca
2017-11-24 11:43         ` [kernel-hardening] " Salvatore Mesoraca
2017-11-24 11:53         ` David Laight
2017-11-24 11:53           ` [kernel-hardening] " David Laight
2017-11-26 11:29           ` Salvatore Mesoraca
2017-11-26 11:29             ` [kernel-hardening] " Salvatore Mesoraca
2017-11-27  0:26         ` Solar Designer
2017-11-27  0:26           ` [kernel-hardening] " Solar Designer
2017-11-30 14:39           ` Salvatore Mesoraca
2017-11-30 14:39             ` [kernel-hardening] " Salvatore Mesoraca
2017-11-30 14:57             ` Ian Campbell
2017-11-30 16:30               ` [kernel-hardening] " Solar Designer
2017-12-05 10:21                 ` Salvatore Mesoraca
2017-12-07 21:47                   ` Solar Designer
2017-12-11 12:08                     ` Salvatore Mesoraca
2017-11-23 22:57   ` Tobin C. Harding
2017-11-24  8:34     ` Salvatore Mesoraca
2017-11-30 16:53   ` David Laight
2017-11-30 16:53     ` [kernel-hardening] " David Laight
2017-11-30 17:51     ` Solar Designer
2017-11-30 17:51       ` [kernel-hardening] " Solar Designer
2017-12-01  9:46       ` David Laight
2017-12-01  9:46         ` [kernel-hardening] " David Laight
2017-12-01 15:52         ` Alan Cox
2017-12-01 15:52           ` [kernel-hardening] " Alan Cox
2017-11-27  1:14 ` [kernel-hardening] Re: [PATCH v3 0/2] Restrict dangerous " Solar Designer
2017-11-27 13:18   ` Solar Designer
2017-11-30 14:38     ` Salvatore Mesoraca
2017-11-30 14:37   ` Salvatore Mesoraca
2017-11-30 19:05     ` Solar Designer
2017-11-30 19:14       ` Solar Designer
2017-12-05 10:13       ` Salvatore Mesoraca
2017-12-07 22:23         ` Solar Designer
2017-12-08 12:17           ` Solar Designer
2017-12-11 12:07             ` Salvatore Mesoraca
2017-12-11 15:33               ` Solar Designer
2017-12-12 18:00                 ` Salvatore Mesoraca
2017-12-11 16:07           ` Solar Designer
2017-12-12 18:01             ` Salvatore Mesoraca

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1511337706-8297-3-git-send-email-s.mesoraca16@gmail.com \
    --to=s.mesoraca16@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=jannh@google.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=solar@openwall.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.