On Oct 19, 2016 2:28 PM, "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> wrote:
>
> Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes:
>
> > On Wed, Oct 19, 2016 at 10:55 AM, Eric W. Biederman
> > <ebiederm@xxxxxxxxxxxx> wrote:
> >> Andy Lutomirski <luto@xxxxxxxxxxxxxx> writes:
> >>
> >>> On Wed, Oct 19, 2016 at 10:29 AM, Jann Horn <jann@xxxxxxxxx> wrote:
> >>>> On Wed, Oct 19, 2016 at 11:52:50AM -0500, Eric W. Biederman wrote:
> >>>>> Andy Lutomirski <luto@xxxxxxxxxxxxxx> 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?
$ strace sudo foobar
or
$ strace auditctl
I find the current behavior somewhat odd, but I've taken advantage of
it on a semi-regular basis.
That being said, the "May the user_ns root read the executable?" test
in your patch is not strictly correct. Do we keep a struct cred
around for the ns root?
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
