aurora-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bill Farner <wfar...@apache.org>
Subject Re: Looking for feedback - Setting CommandInfo.user by default when launching tasks.
Date Tue, 29 Mar 2016 21:50:39 GMT
Aha, i think we have different notions of the proposal.  I was under the
impression that the executor itself would run as the target user (e.g. steve),
not as a system user (e.g. aurora).  I find the former more appealing, with
the exception that it leaves us without a solution for concealing the
credentials file.

On Tue, Mar 29, 2016 at 2:39 PM, John Sirois <john@conductant.com> wrote:

> On Tue, Mar 29, 2016 at 3:26 PM, Bill Farner <wfarner@apache.org> wrote:
>
> > If i'm understanding you correctly, that doesn't address preventing users
> > from reading the credentials though.
> >
>
> It does:
>
> Say:
> /var/lib/aurora/creds 400 root
>
> And then if the CommandInfo has user: aurora (executor user as Steve
> suggested), it will get a copy of /var/lib/aurora/creds  in its sandbox
> chowned to 400 aurora
>
> Now aurora's executor (thermos), launches a task in role www-data
> announcing for it using the cred.  The www-data user will not be able to
> read the local sandbox 400 aurora creds.
>
>
> > On Tue, Mar 29, 2016 at 1:52 PM, John Sirois <jsirois@apache.org> wrote:
> >
> > > On Tue, Mar 29, 2016 at 2:31 PM, Steve Niemitz <sniemitz@apache.org>
> > > wrote:
> > >
> > > > So maybe we add it, but don't change the current default behavior?
> > > >
> > >
> > > Could we use the CommandInfo.uris [1] to solve this?  IE: the scheduler
> > > would need to learn the credential file path, and with that knowledge,
> > the
> > > local mesos (root) readable credential file could be specified as a uir
> > > dependency for all launched executors (or bare commands).  IIUC, if the
> > URI
> > > if a file:// the local secured credentails file will be copied into the
> > > sandbox where it can be read by the executor (as aurora).
> > >
> > > [1]
> > >
> >
> https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L422
> > >
> > >
> > > >
> > > > On Tue, Mar 29, 2016 at 4:26 PM, Bill Farner <wfarner@apache.org>
> > wrote:
> > > >
> > > > > I'm in favor of moving forward.  There's no requirement to use the
> > > > > Announcer, and a non-root executor seems like a useful option.
> > > > >
> > > > > On Tue, Mar 29, 2016 at 1:00 PM, Steve Niemitz <
> sniemitz@apache.org>
> > > > > wrote:
> > > > >
> > > > > > Makes sense, I guess it can be up to the cluster operator which
> > model
> > > > to
> > > > > > choose.  Is there any interest in the feature I proposed or
> should
> > I
> > > > just
> > > > > > drop it?  It's not a lot of code, but also it's not a requirement
> > for
> > > > > > anything we're working on either (the docker stuff however,
is).
> > > > > >
> > > > > > On Tue, Mar 29, 2016 at 1:39 PM, Bill Farner <wfarner@apache.org
> >
> > > > wrote:
> > > > > >
> > > > > > > That's correct - those credentials should require privileged
> > > access.
> > > > > > >
> > > > > > > On Tue, Mar 29, 2016 at 10:25 AM, Steve Niemitz <
> > > > > > > sniemitz@twitter.com.invalid> wrote:
> > > > > > >
> > > > > > > > Re: ZK credential files, thats an interesting issue,
I assume
> > you
> > > > > don't
> > > > > > > > want the role user to be able to read it either, and
only
> root
> > or
> > > > > some
> > > > > > > > other privileged user?
> > > > > > > >
> > > > > > > > On Tue, Mar 29, 2016 at 12:14 PM, Erb, Stephan <
> > > > > > > > Stephan.Erb@blue-yonder.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > I am in favor of your proposal. We offer less
attack
> surface
> > if
> > > > the
> > > > > > > > > executor is not running as root.
> > > > > > > > >
> > > > > > > > > Interesting though, this introduces another security
> problem:
> > > The
> > > > > > > > > credentials file in the incoming Zookeeper  ACL
patch (
> > > > > > > > > https://reviews.apache.org/r/45042/) will have
to be
> > readable
> > > by
> > > > > > > > > everyone. That feels a little bit like being
back to square
> > > one.
> > > > > > > > > ________________________________________
> > > > > > > > > From: Steve Niemitz <sniemitz@apache.org>
> > > > > > > > > Sent: Tuesday, March 29, 2016 17:34
> > > > > > > > > To: dev@aurora.apache.org
> > > > > > > > > Subject: Looking for feedback - Setting CommandInfo.user
by
> > > > default
> > > > > > > when
> > > > > > > > > launching tasks.
> > > > > > > > >
> > > > > > > > > I've been working on some changes to how aurora
submits
> tasks
> > > to
> > > > > > mesos,
> > > > > > > > > specifically around Docker tasks, but I'd also
like to see
> > how
> > > > > people
> > > > > > > > feel
> > > > > > > > > about making it more general.
> > > > > > > > >
> > > > > > > > > Currently, when Aurora submits a task to mesos,
it does NOT
> > set
> > > > > > > > > command.user on the ExecutorInfo, this means
that mesos
> > > > configures
> > > > > > the
> > > > > > > > > sandbox (mesos sandbox that is) as root, and
launches the
> > > > executor
> > > > > > > > > (thermos_executor in our case) as root as well.
> > > > > > > > >
> > > > > > > > > What then happens is that the executor then chown()s
the
> > > sandbox
> > > > it
> > > > > > > > creates
> > > > > > > > > to the aurora role/user, and also setuid()s the
runners it
> > > forks
> > > > to
> > > > > > > that
> > > > > > > > > role/user.  However, the executor itself is still
running
> as
> > > > root.
> > > > > > > > >
> > > > > > > > > My proposal / change is to set command.user to
the aurora
> > role
> > > by
> > > > > > > > default,
> > > > > > > > > which will cause the executor to run as that
user.  I've
> > tested
> > > > > this
> > > > > > > > > already, and no changes are needed to the executor,
it will
> > > still
> > > > > try
> > > > > > > to
> > > > > > > > > chown the sandbox (which is fine since it already
owns it),
> > and
> > > > > > > setuid()
> > > > > > > > > the runners it forks (again, fine, since they're
already
> > > running
> > > > as
> > > > > > > that
> > > > > > > > > user).
> > > > > > > > >
> > > > > > > > > *The controversial part of this* however is I'd
like to
> > enable
> > > > this
> > > > > > > > > behavior BY DEFAULT, and allow disabling it (reverting
to
> the
> > > > > current
> > > > > > > > > behavior now) via a flag to the scheduler.  My
reasoning
> here
> > > is
> > > > > two
> > > > > > > > fold.
> > > > > > > > >  1) It's a more secure default, preventing a
compromised
> > > executor
> > > > > > from
> > > > > > > > > doing things it shouldn't, and 2) we already
have a lot of
> > > "flag
> > > > > > > bloat",
> > > > > > > > > and flags are hard enough to discover as they
are.
> However,
> > I
> > > do
> > > > > > > believe
> > > > > > > > > this should be considered as a "breaking change",
> > particularly
> > > > > > because
> > > > > > > of
> > > > > > > > > finicky PEX extraction for the executor.
> > > > > > > > >
> > > > > > > > > I'd like to hear people's thoughts on this.
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
>
>
> --
> John Sirois
> 303-512-3301
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message