Return-Path: X-Original-To: apmail-aurora-dev-archive@minotaur.apache.org Delivered-To: apmail-aurora-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 9138010F9A for ; Thu, 12 Feb 2015 00:29:50 +0000 (UTC) Received: (qmail 84034 invoked by uid 500); 12 Feb 2015 00:29:50 -0000 Delivered-To: apmail-aurora-dev-archive@aurora.apache.org Received: (qmail 83986 invoked by uid 500); 12 Feb 2015 00:29:50 -0000 Mailing-List: contact dev-help@aurora.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@aurora.incubator.apache.org Delivered-To: mailing list dev@aurora.incubator.apache.org Received: (qmail 83975 invoked by uid 99); 12 Feb 2015 00:29:50 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 12 Feb 2015 00:29:50 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED,T_RP_MATCHES_RCVD X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO mail.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with SMTP; Thu, 12 Feb 2015 00:29:47 +0000 Received: (qmail 83799 invoked by uid 99); 12 Feb 2015 00:29:27 -0000 Received: from mail-relay.apache.org (HELO mail-relay.apache.org) (140.211.11.15) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 12 Feb 2015 00:29:27 +0000 Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com [209.85.212.177]) by mail-relay.apache.org (ASF Mail Server at mail-relay.apache.org) with ESMTPSA id 2C4B41A031F for ; Thu, 12 Feb 2015 00:29:27 +0000 (UTC) Received: by mail-wi0-f177.google.com with SMTP id bs8so494707wib.4 for ; Wed, 11 Feb 2015 16:29:26 -0800 (PST) X-Gm-Message-State: ALoCoQn+P8JTJMypVgMmZXLoOB0S7rpGY9HqxkBImHGM4yzapd55zCrp2c3yM10VeNWCzAPuTfv6 MIME-Version: 1.0 X-Received: by 10.194.88.131 with SMTP id bg3mr2075348wjb.119.1423700966080; Wed, 11 Feb 2015 16:29:26 -0800 (PST) Received: by 10.216.188.68 with HTTP; Wed, 11 Feb 2015 16:29:26 -0800 (PST) In-Reply-To: References: Date: Wed, 11 Feb 2015 16:29:26 -0800 Message-ID: Subject: Re: [DISCUSS] Use the Apache Shiro framework for scheduler security From: Maxim Khutornenko To: dev@aurora.incubator.apache.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Virus-Checked: Checked by ClamAV on apache.org I see. Would it rather make sense to hide "shiro-or-else" decision behind a light interface instead? Something along the lines of the existing SessionValidator.checkAuthorized() that would accept a wrapper object with all necessary security data. On Wed, Feb 11, 2015 at 4:20 PM, Kevin Sweeney wrote: > I'm proposing more-or-less the "if > SHIRO_ENABLED do shiro_auth else do old_stuff" approach so that the > SHIRO_ENABLED branch can throw UnsupportedOperationException for a period > of time. > > On Wed, Feb 11, 2015 at 4:14 PM, Maxim Khutornenko wro= te: > >> Right, I was asking about an example of the interim incremental >> approach you are proposing. Are you envisioning something like "if >> SHIRO_ENABLED do shiro_auth else do old_stuff" approach? The example >> you gave shows the end result not what we will see in 0.8.0 (unless >> you are suggesting moving "old_stuff" behind the new shiro facade). >> >> On Wed, Feb 11, 2015 at 4:08 PM, Kevin Sweeney wrot= e: >> > I think that would be too big a patch to land all at once. I'd like to >> add >> > in Shiro permission checks and annotations incrementally behind this f= lag >> > as there are conceptually different changes that will benefit from sma= ll >> > reviews. If I take the approach of ripping out the current framework >> first >> > we could be left in an "old way's broken, new way's not finished yet" >> state. >> > >> > On Wed, Feb 11, 2015 at 4:03 PM, Maxim Khutornenko >> wrote: >> > >> >> Any example? The original code fragment suggest our current security >> >> model does not map cleanly into shiro. I am +1 on the first pass to >> >> reduce the "if-else" ugliness if possible. >> >> >> >> On Wed, Feb 11, 2015 at 3:57 PM, Kevin Sweeney >> wrote: >> >> > I'm thinking that flag will control which Guice bindings are applie= d, >> so >> >> > there would be 2 parallel implementations for a bit. This would >> >> necessitate >> >> > factoring out capabilityValidator calls to a decorator class (or ri= sk >> >> > having #ifdef-like logic everywhere in SchedulerThriftInterface). >> >> > >> >> > On Wed, Feb 11, 2015 at 3:49 PM, Joshua Cohen < >> jcohen@twopensource.com> >> >> > wrote: >> >> > >> >> >> How do you envision things looking in the intermediate phase where= we >> >> have >> >> >> support for both security modes? >> >> >> >> >> >> I imagine it's easy enough on the Shiro side of if we go with the = AOP >> >> >> annotations for authorization (the interceptor can just check if >> >> >> security_mode =3D=3D SHIRO before doing anything), but that means = we'd >> still >> >> >> have the legacy sessionValidator code in every RPC impl that would >> need >> >> to >> >> >> be wrapped in the inverse check (security_mode =3D=3D >> CAPABILITY_VALIDATOR). >> >> >> >> >> >> Would it make sense to do a first pass to refactor the existing au= th >> >> >> checking logic to a reusable method, or are we ok living with the >> >> temporary >> >> >> ugliness involved in adding that mode checking wrapper to all the >> >> existing >> >> >> auth code? >> >> >> >> >> >> On Wed, Feb 11, 2015 at 3:23 PM, Kevin Sweeney >> >> wrote: >> >> >> >> >> >> > On Wed, Feb 11, 2015 at 3:19 PM, Zameer Manji >> >> wrote: >> >> >> > >> >> >> > > +1 to this proposal. >> >> >> > > >> >> >> > > Will we have dual implementations of API methods as we depreca= te >> the >> >> >> > > SessionKey based API methods? >> >> >> > > >> >> >> > Yes for backwards-compatibility I think we'll need a flag to >> indicate >> >> >> which >> >> >> > system to use. It will probably be an all-or-nothing setting (th= ink >> >> >> > -security_mode=3DSHIRO|CAPABILITY_VALIDATOR). >> >> >> > >> >> >> > > >> >> >> > > On Wed, Feb 11, 2015 at 3:13 PM, Kevin Sweeney < >> kevints@apache.org> >> >> >> > wrote: >> >> >> > > >> >> >> > > > I've been thinking about revamping the authentication and >> >> >> authorization >> >> >> > > in >> >> >> > > > the scheduler recently. I've investigated Apache Shiro >> >> >> > > > and I think it would fit into the >> >> >> scheduler >> >> >> > > > nicely as a replacement for our custom CapabilityValidator >> >> >> > > > < >> >> >> > > > >> >> >> > > >> >> >> > >> >> >> >> >> >> http://people.apache.org/~kevints/aurora/dist/0.5.0-incubating/javadoc/o= rg/apache/aurora/auth/CapabilityValidator.html >> >> >> > > > > >> >> >> > > > framework (for which there currently exists no implementatio= n). >> >> >> > > > >> >> >> > > > I'd like feedback on this proposal. >> >> >> > > > Status Quo >> >> >> > > > >> >> >> > > > Security is currently implemented by a hand-rolled >> >> SessionValidator >> >> >> > > > < >> >> >> > > > >> >> >> > > >> >> >> > >> >> >> >> >> >> http://people.apache.org/~kevints/aurora/dist/0.5.0-incubating/javadoc/o= rg/apache/aurora/auth/SessionValidator.html >> >> >> > > > > >> >> >> > > > framework. No public implementations exist. >> >> >> > > > Proposal >> >> >> > > > >> >> >> > > > Change the scheduler to use the Apache Shiro framework for >> >> >> > authentication >> >> >> > > > and authorization. Move authentication from application to >> >> transport >> >> >> > > layer >> >> >> > > > and move authorization to the Shiro Permissions model. >> >> >> > > > Advantages >> >> >> > > > >> >> >> > > > A few things that will become possible once this work is >> complete: >> >> >> > > > >> >> >> > > > 1. Ability to configure secure Aurora client-to-scheduler wi= th >> a >> >> >> simple >> >> >> > > > flat configuration file (shiro.ini >> >> >> > > > ). >> >> >> > > > >> >> >> > > > 2. Ability to integrate Aurora with my enterprise SSO >> >> (Kerberos+LDAP >> >> >> > for >> >> >> > > > example) by implementing a custom Shiro Realm >> >> >> > > > . >> >> >> > > > >> >> >> > > > 3. Ability to allow a CI server to continuously deploy to ev= ery >> >> >> role's >> >> >> > > > "staging" environment without being able to touch its "prod" >> one >> >> by >> >> >> > using >> >> >> > > > Shiro's WildcardPermission >> >> >> > > > < >> >> >> > > > >> >> >> > > >> >> >> > >> >> >> >> >> >> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/authz/per= mission/WildcardPermission.html >> >> >> > > > > >> >> >> > > > . >> >> >> > > > >> >> >> > > > 4. Ability to authenticate to the scheduler API using Kerber= os >> >> (via >> >> >> > > SPNEGO >> >> >> > > > ) or HTTP Basic auth. >> >> >> > > > >> >> >> > > > 5. Ability to perform authenticated write operations on a jo= b >> via >> >> the >> >> >> > web >> >> >> > > > UI >> >> >> > > > < >> >> >> > >> >> http://www.chromium.org/developers/design-documents/http-authenticati= on >> >> >> > > >. >> >> >> > > > Suggested Reading >> >> >> > > > >> >> >> > > > Shiro has excellent documentation and is a fellow Apache >> >> Foundation >> >> >> > > > project. I suggest you check out at least the 10-minute >> tutorial >> >> >> > > > and the >> Guice >> >> >> > > > integration >> >> >> > > > documentation . >> >> >> > > > Scheduler-side changes >> >> >> > > > >> >> >> > > > The best way to show the proposed changes is by example. In >> >> addition >> >> >> to >> >> >> > > > Guice wiring changes to place the Shiro authentication filte= r >> into >> >> >> the >> >> >> > > > request chain, code that previously looked like >> >> >> > > > >> >> >> > > > @Override >> >> >> > > > >> >> >> > > > public Response createJob( >> >> >> > > > >> >> >> > > > JobConfiguration mutableJob, >> >> >> > > > >> >> >> > > > @Nullable final Lock mutableLock, >> >> >> > > > >> >> >> > > > SessionKey session) { >> >> >> > > > >> >> >> > > > requireNonNull(session); >> >> >> > > > >> >> >> > > > try { >> >> >> > > > >> >> >> > > > sessionValidator.checkAuthenticated( >> >> >> > > > >> >> >> > > > session, >> >> >> > > > >> >> >> > > > ImmutableSet.of(mutableJob.getKey().getRole())); >> >> >> > > > >> >> >> > > > } catch (AuthFailedException e) { >> >> >> > > > >> >> >> > > > return errorResponse(AUTH_FAILED, e); >> >> >> > > > >> >> >> > > > } >> >> >> > > > >> >> >> > > > // Request is authenticated and authorized, continue. >> >> >> > > > >> >> >> > > > } >> >> >> > > > >> >> >> > > > becomes >> >> >> > > > >> >> >> > > > @Override >> >> >> > > > >> >> >> > > > public Response createJob( >> >> >> > > > >> >> >> > > > JobConfiguration mutableJob, >> >> >> > > > >> >> >> > > > @Nullable final Lock mutableLock) { >> >> >> > > > >> >> >> > > > // subject is injected in the constructor by Guice each >> >> request. >> >> >> > > > >> >> >> > > > // checkPermission will throw an unchecked >> >> >> > > > >> >> >> > > > // AuthorizationException that bubbles up as a 401. >> >> >> > > > >> >> >> > > > // This line could also be inserted by inspection of the >> method >> >> >> > > > >> >> >> > > > // call in a security AOP layer. >> >> >> > > > >> >> >> > > > subject.checkPermission( >> >> >> > > > >> >> >> > > > // A Shiro WildcardPermission job:create:mesos:prod:lab= rat >> >> >> > > > >> >> >> > > > new JobScopedPermission("job:create", >> mutableJob.getKey())); >> >> >> > > > >> >> >> > > > // Request is authenticated and authorized, continue. >> >> >> > > > >> >> >> > > > } >> >> >> > > > >> >> >> > > > Some admin methods are protected by annotations like >> >> >> > > > >> >> >> > > > @Requires(Capability.PROVISIONER) >> >> >> > > > >> >> >> > > > public Response startMaintenance(Set hosts, SessionK= ey >> >> >> session) >> >> >> > > { =E2=80=A6 >> >> >> > > > } >> >> >> > > > >> >> >> > > > They'd become >> >> >> > > > >> >> >> > > > @RequiresPermission("maintenance:create") >> >> >> > > > >> >> >> > > > public Response startMaintenance(Set hosts) { =E2=80= =A6 } >> >> >> > > > Client-side changes >> >> >> > > > >> >> >> > > > No changes are necessary to use HTTP Basic Auth - requests w= ill >> >> >> > > > automatically use a .netrc file today. >> >> >> > > > >> >> >> > > > An optional dependency on kerberos and requests-kerberos can= be >> >> added >> >> >> > to >> >> >> > > > support SPNEGO authentication. >> >> >> > > > Timeline >> >> >> > > > >> >> >> > > > I would like to land support for HTTP Basic Auth and SPNEGO = in >> >> 0.8.0 >> >> >> > and >> >> >> > > > deprecate the SessionKey-based API for authentication in fav= or >> of >> >> >> fully >> >> >> > > > transport-based authentication. >> >> >> > > > >> >> >> > > > In 0.9.0 I propose removing SessionKey from the API entirely >> along >> >> >> with >> >> >> > > > SessionValidator from the scheduler. >> >> >> > > > >> >> >> > > >> >> >> > > >> >> >> > > >> >> >> > > -- >> >> >> > > Zameer Manji >> >> >> > > >> >> >> > >> >> >> >> >> >>