linux-kernel - Re: [PATCH v3] ptrace: reintroduce usage of subjective credentials in ptrace_has

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] [day] [month] [year] [list]

Date:   Fri, 17 Jan 2020 22:35:11 +0100
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     Kees Cook <keescook@...omium.org>
CC:     linux-kernel@...r.kernel.org, Jann Horn <jannh@...gle.com>,
        Oleg Nesterov <oleg@...hat.com>, stable@...r.kernel.org,
        Serge Hallyn <serge@...lyn.com>, Eric Paris <eparis@...hat.com>
Subject: Re: [PATCH v3] ptrace: reintroduce usage of subjective credentials in ptrace_has_cap()

On January 17, 2020 10:15:04 PM GMT+01:00, Kees Cook <keescook@...omium.org> wrote:
>On Fri, Jan 17, 2020 at 11:57:18AM +0100, Christian Brauner wrote:
>> -static int ptrace_has_cap(struct user_namespace *ns, unsigned int
>mode)
>> +static int ptrace_has_cap(const struct cred *cred, struct
>user_namespace *ns,
>> +			  unsigned int mode)
>>  {
>> -	if (mode & PTRACE_MODE_NOAUDIT)
>> -		return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE);
>> -	else
>> -		return has_ns_capability(current, ns, CAP_SYS_PTRACE);
>> +	return security_capable(cred, ns, CAP_SYS_PTRACE,
>> +				(mode & PTRACE_MODE_NOAUDIT) ? CAP_OPT_NOAUDIT :
>> +							       CAP_OPT_NONE);
>>  }
>
>Eek, no. I think this inverts the check.
>
>Before:
>bool has_ns_capability(struct task_struct *t,
>                       struct user_namespace *ns, int cap)
>{
>	...
>        ret = security_capable(__task_cred(t), ns, cap, CAP_OPT_NONE);
>	...
>        return (ret == 0);
>}
>
>static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
>{
>	...
>                return has_ns_capability(current, ns, CAP_SYS_PTRACE);
>}
>
>After:
>static int ptrace_has_cap(const struct cred *cred, struct
>user_namespace *ns,
>                       unsigned int mode)
>{
>	return security_capable(cred, ns, CAP_SYS_PTRACE,
>				(mode & PTRACE_MODE_NOAUDIT) ? CAP_OPT_NOAUDIT :
>							       CAP_OPT_NONE);
>}
>
>Note lack of "== 0" on the security_capable() return value, but it's
>needed. To avoid confusion, I think ptrace_has_cap() should likely
>return bool too.
>
>-Kees

Ok, I'll make it bool. Can I retain your reviewed-by or do you want to provide a new one?
I want to have this in mainline asap because this is a cve waiting to happen as soon as io_uring for open and openat lands in v5.6.
I plan on sending a on sending a pr before Sunday.

Christian