netdev - Re: [kernel-hardening] [PATCH net-next v6 07/11] landlock: Add ptrace restrictions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEiveUdXGEOywJoYTsBcxCp02_2gtj7Zw0zT_YdGWM3fJLYY7g@mail.gmail.com>
Date:   Mon, 10 Apr 2017 08:48:36 +0200
From:   Djalal Harouni <tixxdz@...il.com>
To:     Mickaël Salaün <mic@...ikod.net>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Andy Lutomirski <luto@...capital.net>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Casey Schaufler <casey@...aufler-ca.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Drysdale <drysdale@...gle.com>,
        "David S . Miller" <davem@...emloft.net>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        James Morris <james.l.morris@...cle.com>,
        Jann Horn <jann@...jh.net>, Jonathan Corbet <corbet@....net>,
        Matthew Garrett <mjg59@...f.ucam.org>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Paul Moore <paul@...l-moore.com>,
        Sargun Dhillon <sargun@...gun.me>,
        "Serge E . Hallyn" <serge@...lyn.com>,
        Shuah Khan <shuah@...nel.org>, Tejun Heo <tj@...nel.org>,
        Thomas Graf <tgraf@...g.ch>, Will Drewry <wad@...omium.org>,
        kernel-hardening@...ts.openwall.com,
        Linux API <linux-api@...r.kernel.org>,
        LSM List <linux-security-module@...r.kernel.org>,
        netdev@...r.kernel.org
Subject: Re: [kernel-hardening] [PATCH net-next v6 07/11] landlock: Add ptrace restrictions

On Wed, Mar 29, 2017 at 1:46 AM, Mickaël Salaün <mic@...ikod.net> wrote:
> A landlocked process has less privileges than a non-landlocked process
> and must then be subject to additional restrictions when manipulating
> processes. To be allowed to use ptrace(2) and related syscalls on a
> target process, a landlocked process must have a subset of the target
> process' rules.
>
> New in v6
>
> Signed-off-by: Mickaël Salaün <mic@...ikod.net>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Andy Lutomirski <luto@...capital.net>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: David S. Miller <davem@...emloft.net>
> Cc: James Morris <james.l.morris@...cle.com>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Serge E. Hallyn <serge@...lyn.com>
> ---
>  security/landlock/Makefile       |   2 +-
>  security/landlock/hooks_ptrace.c | 126 +++++++++++++++++++++++++++++++++++++++
>  security/landlock/hooks_ptrace.h |  11 ++++
>  security/landlock/init.c         |   2 +
>  4 files changed, 140 insertions(+), 1 deletion(-)
>  create mode 100644 security/landlock/hooks_ptrace.c
>  create mode 100644 security/landlock/hooks_ptrace.h
>
[...]

> +/**
> + * landlock_ptrace_access_check - determine whether the current process may
> + *                               access another
> + *
> + * @child: the process to be accessed
> + * @mode: the mode of attachment
> + *
> + * If the current task has Landlock rules, then the child must have at least
> + * the same rules.  Else denied.
> + *
> + * Determine whether a process may access another, returning 0 if permission
> + * granted, -errno if denied.
> + */
> +static int landlock_ptrace_access_check(struct task_struct *child,
> +               unsigned int mode)
> +{
> +       if (!landlocked(current))
> +               return 0;
> +
> +       if (!landlocked(child))
> +               return -EPERM;
> +
> +       if (landlock_task_has_subset_events(current, child))
> +               return 0;
> +
> +       return -EPERM;
> +}
> +

Maybe you want to check the mode argument here if it is a
PTRACE_ATTACH which may translate to read/writes ? PTRACE_READ are
normally for reads only. Or also which creds were used if this was a
direct syscall or a filesystem call through procfs.

I'm bringing this, since you may want to make some room for landlock
ptrace events and what others may want to do with it. Also I'm
planning to send another v2 RFC for procfs separate instances [1], the
aim is to give LSMs a security_ptrace_access_check hook path when
dealing with /proc/<pids>/ [2]  . Right now LSMs don't really have a
security path there, and the implementation does not guarantee that.
With this Yama ptrace scope or other LSMs may take advantage of it and
check the 'PTRACE_MODE_READ_FSCRED' mode for filesystem accesses.
That's why I think it would be better if the default landlock ptrace
semantics are not that wide.

Thanks!

[1] https://lkml.org/lkml/2017/3/30/670
[2] http://lxr.free-electrons.com/source/fs/proc/base.c#L719