You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@aurora.apache.org by Steve Niemitz <sn...@apache.org> on 2016/03/29 17:34:21 UTC
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.
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by Zhitao Li <zh...@gmail.com>.
Stephan and Joshua,
I am working on design of an external component which will consume
DiscoveryInfo from Mesos and announce it to Zookeeper. (You see why I added
DiscoveryInfo to Mesos now :) )
The reason I like to delegate this component to a third party component
(calling it a Mesos-ZK-Bridge) here:
- it maintains loose coupling between deployment and service discovery,
which are related but not the same thing;
- it makes operations like reconfiguring zk path, rotating ACL and so on a
bit easier;
- this design enables us to support service discovery for other mesos
frameworks in a multi-tenancy mesos cluster.
On Tue, Apr 12, 2016 at 7:49 AM, Joshua Cohen <jc...@apache.org> wrote:
> As things stand today, once a task is scheduled, the scheduler can die, or
> be shut down for maintenance, etc. with no impact to running tasks. If the
> scheduler were responsible for announcing and it maintained the current
> practice of creating znodes as ephemeral, it would need to maintain a
> persistent ZK connection to ensure all announceable tasks are discoverable.
> This means if the scheduler went down for any reason (or just whenever it
> failed over normally) all serversets would be lost. That's obviously not
> acceptable, so the alternative, if we wanted the scheduler to manage
> announcing tasks, would be to stop using ephemeral nodes and instead have
> the scheduler create persistent znodes and manually tear them down when
> tasks finish. While not as problematic as the above, this is still
> potentially a degradation from the current behavior in that there can be a
> long delay between a task exiting and the scheduler becoming aware of this
> (e.g. task finishes during a netsplit). This might be equivalent in scope
> to the issue of a zombie instance that's possible today (i.e. scheduler
> thinks task has been killed and restarts it elsewhere, but Mesos fails to
> clean up the cgroup for whatever reason leading to double registration for
> some time). However, today, due to their ephemeral nature, cleaning up
> duplicate znodes is guaranteed as soon as the executor exits. If the znodes
> were persistent, we'd have to manually seek out and clean up duplicate
> znodes (or build tooling to support this).
>
> It's certainly possible to transition responsibility for announcing from
> the executor to the scheduler, but I think it would be a fairly significant
> amount of work and add a fair amount of complexity.
>
> On Tue, Apr 12, 2016 at 7:35 AM, Erb, Stephan <Stephan.Erb@blue-yonder.com
> >
> wrote:
>
> > I have recently run into another issue due to Thermos running as root (
> > https://issues.apache.org/jira/browse/MESOS-5187). I would therefore
> like
> > to re-open the discussion.
> >
> > IIRC there was a JIRA ticket proposing that the scheduler should be
> > responsible for announcing services in Zookeeper.
> >
> > Would that work? It looks like this could solve a couple of issues at
> once:
> >
> > * no need for credentials on slaves, we could therefore run the executor
> > as a normal user
> > * the announcement would also work for tasks using alternative executors
> >
> >
> > ________________________________________
> > From: John Sirois <jo...@conductant.com>
> > Sent: Tuesday, March 29, 2016 23:55
> > To: Bill Farner
> > Cc: dev@aurora.apache.org; John Sirois
> > Subject: Re: Looking for feedback - Setting CommandInfo.user by default
> > when launching tasks.
> >
> > On Tue, Mar 29, 2016 at 3:50 PM, Bill Farner <wf...@apache.org> wrote:
> >
> > > 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.
> > >
> >
> > My sketch was nonsense anyhow, since thermos running as aurora couldn't
> > launch a task as www-data. Fundamentally we need a priviledged executor
> > (thermos) that can 1: read the creds and announce 2: run the task and
> > health checks nnoth as the targeted un-privileged user.
> >
> >
> > >
> > > On Tue, Mar 29, 2016 at 2:39 PM, John Sirois <jo...@conductant.com>
> > wrote:
> > >
> > >> On Tue, Mar 29, 2016 at 3:26 PM, Bill Farner <wf...@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 <js...@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 <sn...@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
> > >>
> > >
> > >
> >
> >
> > --
> > John Sirois
> > 303-512-3301
> >
>
--
Cheers,
Zhitao Li
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by Joshua Cohen <jc...@apache.org>.
As things stand today, once a task is scheduled, the scheduler can die, or
be shut down for maintenance, etc. with no impact to running tasks. If the
scheduler were responsible for announcing and it maintained the current
practice of creating znodes as ephemeral, it would need to maintain a
persistent ZK connection to ensure all announceable tasks are discoverable.
This means if the scheduler went down for any reason (or just whenever it
failed over normally) all serversets would be lost. That's obviously not
acceptable, so the alternative, if we wanted the scheduler to manage
announcing tasks, would be to stop using ephemeral nodes and instead have
the scheduler create persistent znodes and manually tear them down when
tasks finish. While not as problematic as the above, this is still
potentially a degradation from the current behavior in that there can be a
long delay between a task exiting and the scheduler becoming aware of this
(e.g. task finishes during a netsplit). This might be equivalent in scope
to the issue of a zombie instance that's possible today (i.e. scheduler
thinks task has been killed and restarts it elsewhere, but Mesos fails to
clean up the cgroup for whatever reason leading to double registration for
some time). However, today, due to their ephemeral nature, cleaning up
duplicate znodes is guaranteed as soon as the executor exits. If the znodes
were persistent, we'd have to manually seek out and clean up duplicate
znodes (or build tooling to support this).
It's certainly possible to transition responsibility for announcing from
the executor to the scheduler, but I think it would be a fairly significant
amount of work and add a fair amount of complexity.
On Tue, Apr 12, 2016 at 7:35 AM, Erb, Stephan <St...@blue-yonder.com>
wrote:
> I have recently run into another issue due to Thermos running as root (
> https://issues.apache.org/jira/browse/MESOS-5187). I would therefore like
> to re-open the discussion.
>
> IIRC there was a JIRA ticket proposing that the scheduler should be
> responsible for announcing services in Zookeeper.
>
> Would that work? It looks like this could solve a couple of issues at once:
>
> * no need for credentials on slaves, we could therefore run the executor
> as a normal user
> * the announcement would also work for tasks using alternative executors
>
>
> ________________________________________
> From: John Sirois <jo...@conductant.com>
> Sent: Tuesday, March 29, 2016 23:55
> To: Bill Farner
> Cc: dev@aurora.apache.org; John Sirois
> Subject: Re: Looking for feedback - Setting CommandInfo.user by default
> when launching tasks.
>
> On Tue, Mar 29, 2016 at 3:50 PM, Bill Farner <wf...@apache.org> wrote:
>
> > 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.
> >
>
> My sketch was nonsense anyhow, since thermos running as aurora couldn't
> launch a task as www-data. Fundamentally we need a priviledged executor
> (thermos) that can 1: read the creds and announce 2: run the task and
> health checks nnoth as the targeted un-privileged user.
>
>
> >
> > On Tue, Mar 29, 2016 at 2:39 PM, John Sirois <jo...@conductant.com>
> wrote:
> >
> >> On Tue, Mar 29, 2016 at 3:26 PM, Bill Farner <wf...@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 <js...@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 <wf...@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 <sn...@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
> >>
> >
> >
>
>
> --
> John Sirois
> 303-512-3301
>
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by "Erb, Stephan" <St...@blue-yonder.com>.
I have recently run into another issue due to Thermos running as root (https://issues.apache.org/jira/browse/MESOS-5187). I would therefore like to re-open the discussion.
IIRC there was a JIRA ticket proposing that the scheduler should be responsible for announcing services in Zookeeper.
Would that work? It looks like this could solve a couple of issues at once:
* no need for credentials on slaves, we could therefore run the executor as a normal user
* the announcement would also work for tasks using alternative executors
________________________________________
From: John Sirois <jo...@conductant.com>
Sent: Tuesday, March 29, 2016 23:55
To: Bill Farner
Cc: dev@aurora.apache.org; John Sirois
Subject: Re: Looking for feedback - Setting CommandInfo.user by default when launching tasks.
On Tue, Mar 29, 2016 at 3:50 PM, Bill Farner <wf...@apache.org> wrote:
> 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.
>
My sketch was nonsense anyhow, since thermos running as aurora couldn't
launch a task as www-data. Fundamentally we need a priviledged executor
(thermos) that can 1: read the creds and announce 2: run the task and
health checks nnoth as the targeted un-privileged user.
>
> On Tue, Mar 29, 2016 at 2:39 PM, John Sirois <jo...@conductant.com> wrote:
>
>> On Tue, Mar 29, 2016 at 3:26 PM, Bill Farner <wf...@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 <js...@apache.org>
>> wrote:
>> >
>> > > On Tue, Mar 29, 2016 at 2:31 PM, Steve Niemitz <sn...@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 <wf...@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 <sn...@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
>>
>
>
--
John Sirois
303-512-3301
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by John Sirois <jo...@conductant.com>.
On Tue, Mar 29, 2016 at 3:50 PM, Bill Farner <wf...@apache.org> wrote:
> 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.
>
My sketch was nonsense anyhow, since thermos running as aurora couldn't
launch a task as www-data. Fundamentally we need a priviledged executor
(thermos) that can 1: read the creds and announce 2: run the task and
health checks nnoth as the targeted un-privileged user.
>
> On Tue, Mar 29, 2016 at 2:39 PM, John Sirois <jo...@conductant.com> wrote:
>
>> On Tue, Mar 29, 2016 at 3:26 PM, Bill Farner <wf...@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 <js...@apache.org>
>> wrote:
>> >
>> > > On Tue, Mar 29, 2016 at 2:31 PM, Steve Niemitz <sn...@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 <wf...@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 <sn...@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
>>
>
>
--
John Sirois
303-512-3301
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by Bill Farner <wf...@apache.org>.
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 <jo...@conductant.com> wrote:
> On Tue, Mar 29, 2016 at 3:26 PM, Bill Farner <wf...@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 <js...@apache.org> wrote:
> >
> > > On Tue, Mar 29, 2016 at 2:31 PM, Steve Niemitz <sn...@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 <wf...@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 <sn...@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
>
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by John Sirois <jo...@conductant.com>.
On Tue, Mar 29, 2016 at 3:26 PM, Bill Farner <wf...@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 <js...@apache.org> wrote:
>
> > On Tue, Mar 29, 2016 at 2:31 PM, Steve Niemitz <sn...@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 <wf...@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 <sn...@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 <wf...@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 <sn...@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
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by Bill Farner <wf...@apache.org>.
If i'm understanding you correctly, that doesn't address preventing users
from reading the credentials though.
On Tue, Mar 29, 2016 at 1:52 PM, John Sirois <js...@apache.org> wrote:
> On Tue, Mar 29, 2016 at 2:31 PM, Steve Niemitz <sn...@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 <wf...@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 <sn...@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 <wf...@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 <sn...@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.
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by John Sirois <js...@apache.org>.
On Tue, Mar 29, 2016 at 2:31 PM, Steve Niemitz <sn...@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 <wf...@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 <sn...@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 <wf...@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 <sn...@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.
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by Steve Niemitz <sn...@apache.org>.
So maybe we add it, but don't change the current default behavior?
On Tue, Mar 29, 2016 at 4:26 PM, Bill Farner <wf...@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 <sn...@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 <wf...@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 <sn...@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.
> > > > >
> > > >
> > >
> >
>
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by Bill Farner <wf...@apache.org>.
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 <sn...@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 <wf...@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 <sn...@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.
> > > >
> > >
> >
>
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by Steve Niemitz <sn...@apache.org>.
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 <wf...@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 <sn...@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.
> > >
> >
>
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by Bill Farner <wf...@apache.org>.
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 <sn...@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.
> >
>
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by Steve Niemitz <sn...@twitter.com.INVALID>.
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 <St...@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 <sn...@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.
>
Re: Looking for feedback - Setting CommandInfo.user by default when
launching tasks.
Posted by "Erb, Stephan" <St...@blue-yonder.com>.
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 <sn...@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.