* [PATCH] fs: check for DAC_READ_SEARCH instead of SYS_ADMIN
@ 2017-10-21 13:24 ` Nicolas Belouin
0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Belouin @ 2017-10-21 13:24 UTC (permalink / raw)
To: Alexander Viro, linux-fsdevel, linux-kernel, linux-api, kernel-hardening
Cc: Nicolas Belouin
These checks are meant to prevent leaks or attacks via directory
traversal, the use of CAP_SYS_ADMIN here is a misuse,
CAP_DAC_READ_SEARCH being way more appropriate as a process
with CAP_DAC_READ_SEARCH is entrusted with going trough all directories.
CAP_SYS_ADMIN is not meant to flag such a process.
Signed-off-by: Nicolas Belouin <nicolas@belouin.fr>
---
fs/dcookies.c | 2 +-
fs/proc/base.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/dcookies.c b/fs/dcookies.c
index 0d0461cf2431..48491299a183 100644
--- a/fs/dcookies.c
+++ b/fs/dcookies.c
@@ -158,7 +158,7 @@ SYSCALL_DEFINE3(lookup_dcookie, u64, cookie64, char __user *, buf, size_t, len)
/* we could leak path information to users
* without dir read permission without this
*/
- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) && !capable(CAP_DAC_READ_SEARCH))
return -EPERM;
mutex_lock(&dcookie_mutex);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ad3b0762cc3e..965a3aa1a77f 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2006,16 +2006,16 @@ struct map_files_info {
};
/*
- * Only allow CAP_SYS_ADMIN to follow the links, due to concerns about how the
- * symlinks may be used to bypass permissions on ancestor directories in the
- * path to the file in question.
+ * Only allow CAP_SYS_ADMIN or CAP_DAC_READ_SEARCH to follow the links, due
+ * to concerns about how the symlinks may be used to bypass permissions on
+ * ancestor directories in the path to the file in question.
*/
static const char *
proc_map_files_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
{
- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) && !capable(CAP_DAC_READ_SEARCH))
return ERR_PTR(-EPERM);
return proc_pid_get_link(dentry, inode, done);
--
2.14.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] fs: check for DAC_READ_SEARCH instead of SYS_ADMIN
@ 2017-10-21 13:24 ` Nicolas Belouin
0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Belouin @ 2017-10-21 13:24 UTC (permalink / raw)
To: Alexander Viro, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8
Cc: Nicolas Belouin
These checks are meant to prevent leaks or attacks via directory
traversal, the use of CAP_SYS_ADMIN here is a misuse,
CAP_DAC_READ_SEARCH being way more appropriate as a process
with CAP_DAC_READ_SEARCH is entrusted with going trough all directories.
CAP_SYS_ADMIN is not meant to flag such a process.
Signed-off-by: Nicolas Belouin <nicolas-6zwZCx3K5ONGWvitb5QawA@public.gmane.org>
---
fs/dcookies.c | 2 +-
fs/proc/base.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/dcookies.c b/fs/dcookies.c
index 0d0461cf2431..48491299a183 100644
--- a/fs/dcookies.c
+++ b/fs/dcookies.c
@@ -158,7 +158,7 @@ SYSCALL_DEFINE3(lookup_dcookie, u64, cookie64, char __user *, buf, size_t, len)
/* we could leak path information to users
* without dir read permission without this
*/
- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) && !capable(CAP_DAC_READ_SEARCH))
return -EPERM;
mutex_lock(&dcookie_mutex);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ad3b0762cc3e..965a3aa1a77f 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2006,16 +2006,16 @@ struct map_files_info {
};
/*
- * Only allow CAP_SYS_ADMIN to follow the links, due to concerns about how the
- * symlinks may be used to bypass permissions on ancestor directories in the
- * path to the file in question.
+ * Only allow CAP_SYS_ADMIN or CAP_DAC_READ_SEARCH to follow the links, due
+ * to concerns about how the symlinks may be used to bypass permissions on
+ * ancestor directories in the path to the file in question.
*/
static const char *
proc_map_files_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
{
- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) && !capable(CAP_DAC_READ_SEARCH))
return ERR_PTR(-EPERM);
return proc_pid_get_link(dentry, inode, done);
--
2.14.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [kernel-hardening] [PATCH] fs: check for DAC_READ_SEARCH instead of SYS_ADMIN
@ 2017-10-21 13:24 ` Nicolas Belouin
0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Belouin @ 2017-10-21 13:24 UTC (permalink / raw)
To: Alexander Viro, linux-fsdevel, linux-kernel, linux-api, kernel-hardening
Cc: Nicolas Belouin
These checks are meant to prevent leaks or attacks via directory
traversal, the use of CAP_SYS_ADMIN here is a misuse,
CAP_DAC_READ_SEARCH being way more appropriate as a process
with CAP_DAC_READ_SEARCH is entrusted with going trough all directories.
CAP_SYS_ADMIN is not meant to flag such a process.
Signed-off-by: Nicolas Belouin <nicolas@belouin.fr>
---
fs/dcookies.c | 2 +-
fs/proc/base.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/dcookies.c b/fs/dcookies.c
index 0d0461cf2431..48491299a183 100644
--- a/fs/dcookies.c
+++ b/fs/dcookies.c
@@ -158,7 +158,7 @@ SYSCALL_DEFINE3(lookup_dcookie, u64, cookie64, char __user *, buf, size_t, len)
/* we could leak path information to users
* without dir read permission without this
*/
- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) && !capable(CAP_DAC_READ_SEARCH))
return -EPERM;
mutex_lock(&dcookie_mutex);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ad3b0762cc3e..965a3aa1a77f 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2006,16 +2006,16 @@ struct map_files_info {
};
/*
- * Only allow CAP_SYS_ADMIN to follow the links, due to concerns about how the
- * symlinks may be used to bypass permissions on ancestor directories in the
- * path to the file in question.
+ * Only allow CAP_SYS_ADMIN or CAP_DAC_READ_SEARCH to follow the links, due
+ * to concerns about how the symlinks may be used to bypass permissions on
+ * ancestor directories in the path to the file in question.
*/
static const char *
proc_map_files_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
{
- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) && !capable(CAP_DAC_READ_SEARCH))
return ERR_PTR(-EPERM);
return proc_pid_get_link(dentry, inode, done);
--
2.14.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: check for DAC_READ_SEARCH instead of SYS_ADMIN
@ 2017-10-22 5:25 ` Theodore Ts'o
0 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2017-10-22 5:25 UTC (permalink / raw)
To: Nicolas Belouin
Cc: Alexander Viro, linux-fsdevel, linux-kernel, linux-api, kernel-hardening
On Sat, Oct 21, 2017 at 03:24:46PM +0200, Nicolas Belouin wrote:
> These checks are meant to prevent leaks or attacks via directory
> traversal, the use of CAP_SYS_ADMIN here is a misuse,
> CAP_DAC_READ_SEARCH being way more appropriate as a process
> with CAP_DAC_READ_SEARCH is entrusted with going trough all directories.
> CAP_SYS_ADMIN is not meant to flag such a process.
>
> Signed-off-by: Nicolas Belouin <nicolas@belouin.fr>
No. lookup_dcookie() is a horrid, horrid, hack which is
*spectacularly* dangerous. We should not be trying to encourage its
use for anything beside its single legacy user, oprofile(8), for which
CAP_SYS_ADMIN is appropriate.
- Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: check for DAC_READ_SEARCH instead of SYS_ADMIN
@ 2017-10-22 5:25 ` Theodore Ts'o
0 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2017-10-22 5:25 UTC (permalink / raw)
To: Nicolas Belouin
Cc: Alexander Viro, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8
On Sat, Oct 21, 2017 at 03:24:46PM +0200, Nicolas Belouin wrote:
> These checks are meant to prevent leaks or attacks via directory
> traversal, the use of CAP_SYS_ADMIN here is a misuse,
> CAP_DAC_READ_SEARCH being way more appropriate as a process
> with CAP_DAC_READ_SEARCH is entrusted with going trough all directories.
> CAP_SYS_ADMIN is not meant to flag such a process.
>
> Signed-off-by: Nicolas Belouin <nicolas-6zwZCx3K5ONGWvitb5QawA@public.gmane.org>
No. lookup_dcookie() is a horrid, horrid, hack which is
*spectacularly* dangerous. We should not be trying to encourage its
use for anything beside its single legacy user, oprofile(8), for which
CAP_SYS_ADMIN is appropriate.
- Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
* [kernel-hardening] Re: [PATCH] fs: check for DAC_READ_SEARCH instead of SYS_ADMIN
@ 2017-10-22 5:25 ` Theodore Ts'o
0 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2017-10-22 5:25 UTC (permalink / raw)
To: Nicolas Belouin
Cc: Alexander Viro, linux-fsdevel, linux-kernel, linux-api, kernel-hardening
On Sat, Oct 21, 2017 at 03:24:46PM +0200, Nicolas Belouin wrote:
> These checks are meant to prevent leaks or attacks via directory
> traversal, the use of CAP_SYS_ADMIN here is a misuse,
> CAP_DAC_READ_SEARCH being way more appropriate as a process
> with CAP_DAC_READ_SEARCH is entrusted with going trough all directories.
> CAP_SYS_ADMIN is not meant to flag such a process.
>
> Signed-off-by: Nicolas Belouin <nicolas@belouin.fr>
No. lookup_dcookie() is a horrid, horrid, hack which is
*spectacularly* dangerous. We should not be trying to encourage its
use for anything beside its single legacy user, oprofile(8), for which
CAP_SYS_ADMIN is appropriate.
- Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-22 5:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-21 13:24 [PATCH] fs: check for DAC_READ_SEARCH instead of SYS_ADMIN Nicolas Belouin
2017-10-21 13:24 ` [kernel-hardening] " Nicolas Belouin
2017-10-21 13:24 ` Nicolas Belouin
2017-10-22 5:25 ` Theodore Ts'o
2017-10-22 5:25 ` [kernel-hardening] " Theodore Ts'o
2017-10-22 5:25 ` Theodore Ts'o
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.