FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 05-21-2010, 02:04 PM
Tim Gardner
 
Default UBUNTU: SAUCE: fs: block hardlinks to non-accessible sources

On 05/12/2010 10:03 AM, Kees Cook wrote:
> 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@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 83d5a18..133ca08 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
> @@ -304,6 +306,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
",
> + current->comm);
> + return -EPERM;
> + }
> +
> + return 0;
> +}
> +#endif /* CONFIG_SECURITY_PATH */
> +
> /*
> * Calculate the new process capability sets from the capability sets attached
> * to a file.

Again, very similar to soft links and easily testable.

Acked-by: Tim Gardner <tim.gardner@canonical.com>
--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 05-27-2010, 09:28 AM
Stefan Bader
 
Default UBUNTU: SAUCE: fs: block hardlinks to non-accessible sources

It is similar to the softlinks, though maybe a little harder to see the results
it causes. The patch itself seems to be ok.
For Lucid SRU I would definitely want to see effects on applications. In the
view of SRU changes not being allowed to cause regressions, would it be an
option to make the default weak_nonaccess_hardlinks=1 for the Lucid version and
therefore rather allow an opt-in?

-Stefan

On 05/12/2010 06:03 PM, Kees Cook wrote:
> 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@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 83d5a18..133ca08 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
> @@ -304,6 +306,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
",
> + current->comm);
> + return -EPERM;
> + }
> +
> + return 0;
> +}
> +#endif /* CONFIG_SECURITY_PATH */
> +
> /*
> * Calculate the new process capability sets from the capability sets attached
> * to a file.


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 05-27-2010, 05:19 PM
Kees Cook
 
Default UBUNTU: SAUCE: fs: block hardlinks to non-accessible sources

Hi Stefan,

On Thu, May 27, 2010 at 11:28:20AM +0200, Stefan Bader wrote:
> It is similar to the softlinks, though maybe a little harder to see the results
> it causes. The patch itself seems to be ok.
> For Lucid SRU I would definitely want to see effects on applications. In the
> view of SRU changes not being allowed to cause regressions, would it be an
> option to make the default weak_nonaccess_hardlinks=1 for the Lucid version and
> therefore rather allow an opt-in?

If it gets an SRU, I would want the default set to 1 for sure. Originally
I didn't intend for this to go into Lucid, but if there is demand, I would
support it being done that way.

-Kees

--
Kees Cook
Ubuntu Security Team

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 

Thread Tools




All times are GMT. The time now is 02:03 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org