Re: [PATCH RFC 0/4] Per-task PTI activation

[Willy Tarreau <w@1wt.eu>]

On Tue, Jan 09, 2018 at 03:07:07PM -0600, Eric W. Biederman wrote:
> > In fact that's what I liked with the wrapper approach, except that it
> > had the downside of being harder to manage in terms of administration
> > and we'd risk to see it used everywhere by default. The arch_prctl()
> > approach ensures that only applications where this is relevant can do
> > it. In the case of haproxy, I can trivially add a config option like
> > "disable-page-isolation" to let the admin enable it on purpose.
> 
> How is that different from the option?

Not sure what is different from the option, so let me just enumerate the
two approaches I'm seeing :
  - either a wrapper which work for the next execve() and not further.
    The benefit is that applications do not need to be recompiled. The
    problem is that it requires some changes to various places ranging
    from init scripts to API wrappers and whatever to place this wrapper.
    Also it can start to be perceived as the "wrapper that makes things
    fast" and some admins might start to routinely use it like we use
    taskset, and I'm not fond of this.

  - the option I mentionned would be a configuration setting enabling
    arch_prctl(). The option name should explicitly indicate that the
    admin wants to screw up his system's security. For example,
    "destroy-security-for-performance" is not the type of option you
    leave enabled in your config templates. It *will* require to
    recompile applications. I don't think it's a problem for the most
    performance sensitive ones, but I may be biased by my own
    experience.

  - or a combination of the two if some admins need to support stuff
    they can't rebuild (well LD_PRELOAD might be an option after all...)

> > But I suspect there might be some performance critical applications that
> > cannot be patched, and that's where the wrapper could still provide some
> > value.
> 
> I just don't want to encourage changning this option by default.

That's the same for me. Even the prctl name should be scary enough. Boris
suggested tainting the kernel and that's a very good idea, it will even
ensure that the setting is not used by default in applications because
admins won't want to see their systems tainted for no reason.

> As a
> lot of applications get installed in home servers or other places where
> they are not performance critical.  At which point disabling the kpti
> protection by default would be reducing the level of protection of
> everything.

Definitely. That's why I don't want to see the hard-coded prctl() either.

> But ultimately I only brought this up so that people are thinking about
> the other side of this.   About how it will affect not the high
> performance servers single function but how it will affect the millions
> of little servers that do many things all from a single machine.

This is my concern as well, which is why I'm seeking the most balanced
approach I can think of.

> Certainly I would not want this enabled in a container or a virtual
> private server.  The capable(CAP_RAWIO) seems to handle that beautifully.
> 
> >> Certainly this is something that should look at no-new-privs and if
> >> no-new-privs is set not allow disabling this protection.
> >
> > I don't know what is "no-new-privs" and couldn't find info on it
> > unfortunately. Do you have a link please ?
> 
> Probably because I used dashes.  The no new privs flag is documented
> in:
> Documentation/userspace-api/no_new_privs.rst

Ah stupid me, I should have tried underscores as well. Thank you.

> It is a sandboxing flag that guarantees a process can not gain
> privileges after it has been set.  You can search for PFA_NO_NEW_PRIVS
> in sched.h if you want to see where it is defined.

Oh that's very interesting, I wasn't aware of this! Definitely something
I need to set after dropping privileges in haproxy I guess ;-)

Thanks,
Willy