Hello Dan, Thanks a lot for the feedback!. Please find my answers below.
(1). Great point, I totally missed this one... I wrongly assumed that *this particular method [1]* was in charge of authorizing the query execution on the region but I was clearly wrong, the actual authorization happens *here [2]. *The former just kicks in whenever a method is executed on the region itself (like *getValues*, *keySet*, etc.). (2). Agreed, another oversight on my side, mostly derived from my misunderstanding in item 1. 3). I'm not sure about how this is going to work either, neither if it's even possible without hugely impacting the performance... truth be told, the whole idea of this particular authorizer is to allow users to get up and running *without configuring anything, *but I'm not sure how it can be done in the real world (specially now that you made me aware my error on item 1). I'd actually love to just eliminate this authorizer altogether but it was brought to my attention that the other two proposed out of box implementations require the user to actually configure something (the package or the regex expression), and that we should provide some sort of automatic trust boundary in the sense that users just deploy the domain model, insert objects into the region, and can execute OQL invoking methods as long as those methods belong to the objects stored within the region... determining whether an object belongs to a region or not is fairly complicated, not achievable right now and will certainly incur into performance degradation, but I guess is up to the community to decide whether we should pursue this objective or not. As a summary, and before I go ahead and incorporate all the feedback provided and modify the proposal, I guess the community needs to answer the following questions: A. For the sake of security, is it okay to force our users to do some extra configuration steps (no matter how little or easy they might be) whenever they deploy the domain model to the cluster and want to execute some methods on those objects from OQL?. B. Should we just change the approach from *deny everything* to *allow everything *instead, and assume that users with enough privileges to execute queries on the region (*DATA:READ:RegionName*) are trustworthy (at the end of the day *somebody* gave them that role) and won't compromise the security through RCE from OQL?. If nothing changes during the weekend, I'll wait until Monday and apply the suggested changes to the proposal in favour of a "YES" to question "A" (just because that's my preference at the moment), sending a new email afterward for another feedback round. Changes will still be accepted anyway as this is just the proposal and the community as whole should deny/accept it. Cheers. [1]: https://github.com/apache/geode/blob/develop/geode-core/src/main/java/org/apache/geode/cache/query/internal/RestrictedMethodInvocationAuthorizer.java#L165-L171 [2]: https://github.com/apache/geode/blob/develop/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java#L106-L108 On Thu, Jun 20, 2019 at 7:22 PM Dan Smith <dsm...@pivotal.io> wrote: > +1 > > This looks really good! > > I put a couple of comments inline, and I have a few more general questions > here: > 1. Is the RegionQueryInvocationAuthorizer different than our existing shiro > permissions? I thought users can already grant permissions for specific > regions. What does this add in addition to that? > 2. I'm a little unclear on if your > MethodInvocationAuthorizer.authorizeMethodInvocation is supposed to take a > region or the target object. If it is really accepting a region, do we > actually have a region available in all cases? We could be invoking methods > on an object in lots of places in the query tree. > 3. The DataAwareBasedMethodAuthorizer seems a bit vague on how it's > actually going to work. It also might be a security risk, since it will > allow users with DATA:READ permission to invoke any method on these > objects. > > -Dan > > On Wed, Jun 19, 2019 at 11:34 AM Jacob Barrett <jbarr...@pivotal.io> > wrote: > > > Thanks Juan! > > > > > On Jun 19, 2019, at 9:55 AM, Juan José Ramos <jra...@pivotal.io> > wrote: > > > > > > Hello all, > > > > > > I've removed all "biased" words I could find from the original document > > so > > > the *Proposal [1]* is ready for review and discussion now. All feedback > > is > > > welcome. > > > Best regards. > > > > > > [1]: > > > > > > https://cwiki.apache.org/confluence/display/GEODE/OQL+Method+Invocation+Security > > > > > >> On Fri, Jun 14, 2019 at 8:39 PM Juan José Ramos <jra...@pivotal.io> > > wrote: > > >> > > >> Hey Jake, > > >> > > >> Thanks for bringing this up. As you might have found out already, > > english > > >> is not my native language, I actually had to do some research to find > > out > > >> *exactly what you meant* regarding the bias around the "whitelist" > word > > >> :-|... It was an honest mistake and I sincerely apologize in advance > if > > >> anyone got offended in any way. > > >> That said, I won't have time to go through the proposal and make the > > >> required changes until next week, so I'll keep the document hidden > until > > >> all biased words are replaced. > > >> Cheers. > > >> > > >> > > >> On Sat, Jun 15, 2019 at 12:25 AM Jacob Barrett <jbarr...@pivotal.io> > > >> wrote: > > >> > > >>>> As part of GEODE-3247 < > > https://issues.apache.org/jira/browse/GEODE-3247>, > > >>> several options were analysed and, after considering the wealth of > > security > > >>> holes and the difficulty of determining which methods deployed by the > > >>> developer were intended to be available for queries and which were > > not, the > > >>> decision was made to tighten up the Security and, by default, > disallow > > any > > >>> method call not explicitly whitelisted. > > >>> > > >>> Please avoid biased words, like whitelist, in source and proposals. > > There > > >>> are several other places in this document that use these terms. Can > you > > >>> please update the document without them. > > >>> > > >>> Thanks, > > >>> Jake > > >>> > > >>> > > >> > > >> -- > > >> Juan José Ramos Cassella > > >> Senior Technical Support Engineer > > >> Email: jra...@pivotal.io > > >> Office#: +353 21 4238611 > > >> Mobile#: +353 87 2074066 > > >> After Hours Contact#: +1 877 477 2269 > > >> Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT > > >> How to upload artifacts: > > >> https://support.pivotal.io/hc/en-us/articles/204369073 > > >> How to escalate a ticket: > > >> https://support.pivotal.io/hc/en-us/articles/203809556 > > >> > > >> [image: support] <https://support.pivotal.io/> [image: twitter] > > >> <https://twitter.com/pivotal> [image: linkedin] > > >> <https://www.linkedin.com/company/3048967> [image: facebook] > > >> <https://www.facebook.com/pivotalsoftware> [image: google plus] > > >> <https://plus.google.com/+Pivotal> [image: youtube] > > >> < > > https://www.youtube.com/playlist?list=PLAdzTan_eSPScpj2J50ErtzR9ANSzv3kl > > > > >> > > > > > > > > > -- > > > Juan José Ramos Cassella > > > Senior Technical Support Engineer > > > Email: jra...@pivotal.io > > > Office#: +353 21 4238611 > > > Mobile#: +353 87 2074066 > > > After Hours Contact#: +1 877 477 2269 > > > Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT > > > How to upload artifacts: > > > https://support.pivotal.io/hc/en-us/articles/204369073 > > > How to escalate a ticket: > > > https://support.pivotal.io/hc/en-us/articles/203809556 > > > > > > [image: support] <https://support.pivotal.io/> [image: twitter] > > > <https://twitter.com/pivotal> [image: linkedin] > > > <https://www.linkedin.com/company/3048967> [image: facebook] > > > <https://www.facebook.com/pivotalsoftware> [image: google plus] > > > <https://plus.google.com/+Pivotal> [image: youtube] > > > < > > https://www.youtube.com/playlist?list=PLAdzTan_eSPScpj2J50ErtzR9ANSzv3kl > > > > > -- Juan José Ramos Cassella Senior Technical Support Engineer Email: jra...@pivotal.io Office#: +353 21 4238611 Mobile#: +353 87 2074066 After Hours Contact#: +1 877 477 2269 Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT How to upload artifacts: https://support.pivotal.io/hc/en-us/articles/204369073 How to escalate a ticket: https://support.pivotal.io/hc/en-us/articles/203809556 [image: support] <https://support.pivotal.io/> [image: twitter] <https://twitter.com/pivotal> [image: linkedin] <https://www.linkedin.com/company/3048967> [image: facebook] <https://www.facebook.com/pivotalsoftware> [image: google plus] <https://plus.google.com/+Pivotal> [image: youtube] <https://www.youtube.com/playlist?list=PLAdzTan_eSPScpj2J50ErtzR9ANSzv3kl>
