Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1504349

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

Show all headers | View raw

Andy Lutomirski <luto@amacapital.net> writes:

> On Wed, Oct 19, 2016 at 10:55 AM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> Andy Lutomirski <luto@amacapital.net> writes:
>>
>>> On Wed, Oct 19, 2016 at 10:29 AM, Jann Horn <jann@thejh.net> wrote:
>>>> On Wed, Oct 19, 2016 at 11:52:50AM -0500, Eric W. Biederman wrote:
>>>>> Andy Lutomirski <luto@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.
>
> I would suggest leaving it alone.  Changing it could break enough
> things that a sysctl would be needed, and I just don't see how this is
> a significant issue, especially since it's been insecure forever.
> Anyone who cares should do the stub executable trick:
>
> /sbin/foo: 04755, literally just does execve("/sbin/foo-helper");
>
> /sbin/foo-helper: 0500.

I can't imagine what non-malware would depend on being able to
circumvent file permissions and ptrace a read-only executable.  Is there
something you are thinking of?

I know I saw someone depending on read-only executables being read-only
earlier this week on the security list, and it could definitely act as
part of a counter measure to make binaries harder to exploit.

So given that people actually expect no-read permissions to be honored
on executables (with what seem valid and sensible use cases), that
I can't see any valid reason not to honor no-read permissions, that it
takes a really convoluted setup to bypass the current no-read
permissions, and that I can't believe anyone cares about the current
behavior of ptrace I think this is worth fixing.

Eric

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Michal Hocko <mhocko@kernel.org> - 2016-10-18 16:00 +0200
  Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Jann Horn <jann@thejh.net> - 2016-10-18 16:00 +0200
  Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access ebiederm@xmission.com (Eric W. Biederman) - 2016-10-18 17:00 +0200
    Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Jann Horn <jann@thejh.net> - 2016-10-18 17:10 +0200
      Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access ebiederm@xmission.com (Eric W. Biederman) - 2016-10-18 17:40 +0200
        Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Jann Horn <jann@thejh.net> - 2016-10-18 21:20 +0200
          Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access ebiederm@xmission.com (Eric W. Biederman) - 2016-10-18 23:10 +0200
            [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-18 23:20 +0200
              Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Amir Goldstein <amir73il@gmail.com> - 2016-10-19 08:20 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 17:10 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 19:10 +0200
              Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Andy Lutomirski <luto@amacapital.net> - 2016-10-19 17:40 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 19:00 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not  read. Jann Horn <jann@thejh.net> - 2016-10-19 19:30 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Andy Lutomirski <luto@amacapital.net> - 2016-10-19 19:40 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 20:00 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Andy Lutomirski <luto@amacapital.net> - 2016-10-19 20:40 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 23:30 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Andy Lutomirski <luto@amacapital.net> - 2016-10-20 01:20 +0200
    Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Michal Hocko <mhocko@kernel.org> - 2016-10-18 20:10 +0200

csiph-web