Mailing-List: contact dev-help@aurora.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@aurora.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CAMnduq8PLNTDRKeCmsUPEU0tzh=mACwMhJTgezZsiekqEnbicA@mail.gmail.com>
References: 
 <CAJ4qn4mZaJkS1V402hfGf+cLJJsmoY_g4j8yr-FTFzGwxwynHA@mail.gmail.com>
	<1459268085253.14761@blue-yonder.com>
	<CAJ4qn4k+U_TR2+Ar0XC=ft4Q+oYqx4edPkEhLQK0n2PrkYZPRA@mail.gmail.com>
	<CAFWq12XjS9sNkyhaJWQY3E0-x9b_TdfP9ZUDq6NR8+2U9JHQ=g@mail.gmail.com>
	<CAJ4qn4m55tfFoa4sgh-EdisOwODfrRjfmJL1jnoqsEvBS66U1A@mail.gmail.com>
	<CAFWq12ULZtJid0B1cM0sWPUnTOJpj5DC1DE6k+nft8m6MV-pRA@mail.gmail.com>
	<CAJ4qn4=vc2n8f4g+0dn9xeStXFs7juukuf_fV5+pxfxZt-Xd4w@mail.gmail.com>
	<CAC2vyCVgbMxDV70BB42XVE2W054vca5nvwc6yqzj7sCQ5F_S7Q@mail.gmail.com>
	<CAFWq12VuVfO8WE8qZFomKSnhxnbkdAQx2w-+8bu368LS2Ts5wg@mail.gmail.com>
	<CAHRu5Pb=Rgb+LmTfgy9XFNtxtt5OFni=oZ6=-Lxjt6qUMiwd8w@mail.gmail.com>
	<CAFWq12VMsDy8HYdrEeMh-1+uLg8vcLEqxFFMCXfT5A0i8ixVZA@mail.gmail.com>
	<CAHRu5PZmOMvc_CqE5BsPYkVLDbSmd7gE3skyWHqKPTH72P-Nkg@mail.gmail.com>
	<1460464502325.74265@blue-yonder.com>
	<CAMnduq8PLNTDRKeCmsUPEU0tzh=mACwMhJTgezZsiekqEnbicA@mail.gmail.com>
Date: Tue, 12 Apr 2016 10:40:21 -0700
Message-ID: 
 <CANMwFmUq4OaK6=gs_B7mnVDH8e3_3Q+c1sx21zLZ-Nhm2Z42eg@mail.gmail.com>
Subject: Re: Looking for feedback - Setting CommandInfo.user by default when
 launching tasks.
From: Zhitao Li <zhitaoli.cs@gmail.com>
To: dev@aurora.apache.org
Cc: Bill Farner <wfarner@apache.org>, John Sirois <jsirois@apache.org>
Content-Type: multipart/alternative; boundary=001a11425b64c9b12205304d279c

--001a11425b64c9b12205304d279c
Content-Type: text/plain; charset=UTF-8

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 <jcohen@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 <john@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 <wfarner@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 <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
> > >>
> > >
> > >
> >
> >
> > --
> > John Sirois
> > 303-512-3301
> >
>


-- 
Cheers,

Zhitao Li

--001a11425b64c9b12205304d279c--