[REVIEW][PATCH] exec: Don't exec files the userns root can not read.

[Eric W. Biederman]

Andy Lutomirski <luto at amacapital.net> writes:

> On Wed, Oct 19, 2016 at 10:29 AM, Jann Horn <jann at thejh.net> wrote:
>> On Wed, Oct 19, 2016 at 11:52:50AM -0500, Eric W. Biederman wrote:
>>> Andy Lutomirski <luto at amacapital.net> writes:
>>> > Simply ptrace yourself, exec the
>>> > program, and then dump the program out.  A program that really wants
>>> > to be unreadable should have a stub: the stub is setuid and readable,
>>> > but all the stub does is to exec the real program, and the real
>>> > program should have mode 0500 or similar.
>>> >
>>> > ISTM the "right" check would be to enforce that the program's new
>>> > creds can read the program, but that will break backwards
>>> > compatibility.
>>>
>>> Last I looked I had the impression that exec of a setuid program kills
>>> the ptrace.
>>>
>>> If we are talking about a exec of a simple unreadable executable (aka
>>> something that sets undumpable but is not setuid or setgid).  Then I
>>> agree it should break the ptrace as well and since those programs are as
>>> rare as hens teeth I don't see any problem with changing the ptrace behavior
>>> in that case.
>>
>> Nope. check_unsafe_exec() sets LSM_UNSAFE_* flags in bprm->unsafe, and then
>> the flags are checked by the LSMs and cap_bprm_set_creds() in commoncap.c.
>> cap_bprm_set_creds() just degrades the execution to a non-setuid-ish one,
>> and e.g. ptracers stay attached.
>
> I think you're right.  I ought to be completely sure because I rewrote
> that code back in 2005 or so back when I thought kernel programming
> was only for the cool kids.  It was probably my first kernel patch
> ever and it closed an awkward-to-exploit root hole.  But it's been a
> while.  (Too bad my second (IIRC) kernel patch was more mundane and
> fixed the mute button on "new" Lenovo X60-era laptops and spend
> several years in limbo...)

Ah yes and this is only a problem if the ptracer does not have
CAP_SYS_PTRACE.

If the tracer does not have sufficient permissions any opinions on
failing the exec or kicking out the ptracer?  I am leaning towards failing
the exec as it is more obvious if someone cares.  Dropping the ptracer
could be a major mystery.

Eric