Re: [PROPOSAL]: Improve OQL Method Invocation Security
Hello Jake,
*>> You asked people to comment in both the wiki and the emails but you
didn’t include comments from the wiki below.*
I never said I was going to reply to comments in the wiki and to the email
thread at the same time on the same day. I didn't forget your comments,
BTW, I was going to answer them today on the WIKI directly.
Either way, now that the *Lightweight RFC Process [1]* has been released,
I'll go ahead and address all comments here in the email thread.
*>> Can we safely assume that some caching of authorization requests will
be performed? What will the scope and lifetime of this caching be? Are the
authentication rules and modules assumed to be immutable at runtime? All of
this will have significant implications on performance.*My assumption right
now is that there will be some caching performed and will certainly try to
keep the authorizers immutable but, however, no PoC has been done and the
implementations details shown in the proposal are just a draft, so I'm not
in a position to accurately answer these questions. I also read that
*"premature
optimization is the root of all evil", *so I guess we can benchmark the
solution with and without caching, and decide what's better based on some
real data (I certainly see some gains using caching on the
*RegexBasedMethodAuthorizer*, not sure about the
*JavaBeanAccessorBasedMethodAuthorizer*).
*>> The second issue is how does this differ, augment are compete with
Java’s built in Security Manager / Policy system. It was designed for a lot
of these same reasons, restricting application access to specific OS level
operations that can be dangerous if executed by malicious code. Why is such
a system not sufficient to handle our concerns in OQL? Beyond creating
sockets, files, threads, forks, etc. what are we intending to prevent the
OQL user executing?*
Thanks for pointing this out, I guess I should have included these examples
in the proposal (will certainly do it in the next iteration) as the
starting point so readers could see what we wanted to prevent when *GEODE-3247
[2]* was implemented.
*1. Reflection (This allows the user to do everything)*
select * from /region r where
r.getClass().forName('java.lang.Runtime').getDeclaredMethods()[0].invoke()
*2. Doing anything with the cache (closing, accessing all regions, etc.)*
select * from /region.getCache().close()
*3. Destroying, adding or invalidating the entire region or specific
elements*
select * from /region.destroyRegion()
select * from /region.invalidate('xyz')
select * from /region.put('xyz','abc')
*4. Modifying an object in place*
select r.setName('Zaraza') from /region r
I think the authorizers from the proposal covers items 1, 2 and 3; item 4
is more complicated and I currently don't see a way to prevent in place
modifications (other than trusting that accessor methods - get/is - don't
modify the object or denying everything but let users customize exactly
what should be allowed through regex).
I honestly don't know much about the *Java Security Manager *and didn't
consider its usage due to the fact that it was't considered in the first
place when GEODE-3247 was implemented. I'll spend some time looking at it
today, however, and see if it's applicable to our use case. If you (or
anyone reading this) already has enough knowledge about the *Java Security
Manager* and knows whether we can apply it to our scenario, please shout
and let's have a talk :-).
Best regards.
[1]:
https://cwiki.apache.org/confluence/display/GEODE/Lightweight+RFC+Process
[2]: https://issues.apache.org/jira/browse/GEODE-3247
On Fri, Jun 28, 2019 at 6:29 PM Jacob Barrett wrote:
> Juan,
>
> You asked people to comment in both the wiki and the emails but you didn’t
> include comments from the wiki below.
>
>
> I have two issues, the first I raised in the wiki is what about caching
> the authentication lookups:
> > Can we safely assume that some caching of authorization requests will be
> performed? What will the scope and lifetime of this caching be? Are the
> authentication rules and modules assumed to be immutable at runtime? All of
> this will have significant implications on performance.
>
> The second issue is how does this differ, augment are compete with Java’s
> built in Security Manager / Policy system. It was designed for a lot of
> these same reasons, restricting application access to specific OS level
> operations that can be dangerous if executed by malicious code. Why is such
> a system not sufficient to handle our concerns in OQL? Beyond creating
> sockets, files, threads, forks, etc. what are we intending to prevent the
> OQL user executing?
>
> Thanks,
> Jake
>
>
> > On Jun 28, 2019, at 10:36 AM, Juan José Ramos wrote:
> >
> > Hello all,
> >
> > Below are some answers/comments to the questions and feedback gathered
> > during the last round, along with some final ideas at the end of the
> email.
> >
>
>
--
Juan José Ramos Cassella
Senior Technical Support Engineer
Email: jra...@pi
Re: [PROPOSAL]: Improve OQL Method Invocation Security
> On Jul 1, 2019, at 6:55 AM, Juan José Ramos wrote:
>
>> Can we safely assume that some caching of authorization requests will
>> be performed? What will the scope and lifetime of this caching be? Are the
>> authentication rules and modules assumed to be immutable at runtime? All of
>> this will have significant implications on performance.
> My assumption right
> now is that there will be some caching performed and will certainly try to
> keep the authorizers immutable but, however, no PoC has been done and the
> implementations details shown in the proposal are just a draft, so I'm not
> in a position to accurately answer these questions. I also read that
> *"premature
> optimization is the root of all evil", *so I guess we can benchmark the
> solution with and without caching, and decide what's better based on some
> real data (I certainly see some gains using caching on the
> *RegexBasedMethodAuthorizer*, not sure about the
> *JavaBeanAccessorBasedMethodAuthorizer*).
Premature optimization can be bad but so can ignoring it. Most importantly here
is to define the scope of an authorization. This will need to be defined as
part of the API/SPI. If I was to implement one of these authorizers I would
need to know under what conditions my authorize method going to be called. Is
it going to be for every object that is scanned by this method? The API
suggests this might be the case given that target is an Object and not a Class.
We can do some profiling in our head an know that if we invoke this method for
every potential object we scan then performance is going to suffer. The regex
matcher will destroy the CPU. What if my implementation was to use LDAP, ouch!
If we define scope to be per query this is orders of magnitude better. The
API/SPI should probably change to reflect this. Maybe target should be the
Class? Should the query itself be included? Now if the target is a Class, and
classes are immutable, perhaps now the authorize method could be invoked once
for the life of the application only when a new Class is touched in our object
scan. Such scope would require that either the API/SPI define that rules are
also immutable or that there is some invalidation mechanism to tell the system
the rules have been mutated. Again, this has API/SPI ramifications that all
need to be discussed in this proposal.
> The second issue is how does this differ, augment are compete with
> Java’s built in Security Manager / Policy system. It was designed for a lot
> of these same reasons, restricting application access to specific OS level
> operations that can be dangerous if executed by malicious code. Why is such
> a system not sufficient to handle our concerns in OQL? Beyond creating
> sockets, files, threads, forks, etc. what are we intending to prevent the
> OQL user executing?*
>> Thanks for pointing this out, I guess I should have included these examples
>> in the proposal (will certainly do it in the next iteration) as the
>> starting point so readers could see what we wanted to prevent when
>> *GEODE-3247
>> [2]* was implemented.
>> *1. Reflection (This allows the user to do everything)*
>> select * from /region r where
>> r.getClass().forName('java.lang.Runtime').getDeclaredMethods()[0].invoke()
All of the methods on Runtime use the SecurityManager to prevent invocations by
unauthorized contexts. If we executed queries with the correct context and a
property configure SecurityManager we are protected.
> *2. Doing anything with the cache (closing, accessing all regions, etc.)*
> select * from /region.getCache().close()
> *3. Destroying, adding or invalidating the entire region or specific
> elements*
> select * from /region.destroyRegion()
> select * from /region.invalidate('xyz')
> select * from /region.put('xyz','abc’)
We could add SecurityManager checks to our protected calls.
> *4. Modifying an object in place*
> select r.setName('Zaraza') from /region r
How far do we go to prevent the user from shooting themselves in the foot? Here
we could change OQL to any support the bean interface and disallow all set
operations.
> I honestly don't know much about the *Java Security Manager *and didn't
> consider its usage due to the fact that it was't considered in the first
> place when GEODE-3247 was implemented. I'll spend some time looking at it
> today, however, and see if it's applicable to our use case. If you (or
> anyone reading this) already has enough knowledge about the *Java Security
> Manager* and knows whether we can apply it to our scenario, please shout
> and let's have a talk :-).
I have spent plenty of time in the guts of SecurityManager in previous lives.
I want us to consider systems that exist before we spend time inventing our
own. There is cost in maintain our own solution in implementation time and bug
squashing. There is also the cost in training on our own solution.
SecurityManger is discussed in numerous books and blogs, our solution would not
have
Re: [PROPOSAL]: Improve OQL Method Invocation Security
Hello Jake, Thanks for your reply. Some comments below, inline. *>> Premature optimization can be bad but so can ignoring it. Most importantly here is to define the scope of an authorization. This will need to be defined as part of the API/SPI. If I was to implement one of these authorizers I would need to know under what conditions my authorize method going to be called. Is it going to be for every object that is scanned by this method? The API suggests this might be the case given that target is an Object and not a Class. We can do some profiling in our head an know that if we invoke this method for every potential object we scan then performance is going to suffer. The regex matcher will destroy the CPU. What if my implementation was to use LDAP, ouch! If we define scope to be per query this is orders of magnitude better. The API/SPI should probably change to reflect this. Maybe target should be the Class? Should the query itself be included? Now if the target is a Class, and classes are immutable, perhaps now the authorize method could be invoked once for the life of the application only when a new Class is touched in our object scan. Such scope would require that either the API/SPI define that rules are also immutable or that there is some invalidation mechanism to tell the system the rules have been mutated. Again, this has API/SPI ramifications that all need to be discussed in this proposal.* [Juan]: Agreed, I will keep this in mind when modifying the proposal. *>> How far do we go to prevent the user from shooting themselves in the foot?. Here we could change OQL to any support the bean interface and disallow all set operations.* [Juan]: I'm not sure about this one... how far do we go?, I guess it's a decision the community needs to make. I certainly believe that we can't prevent everything, but I also think that we can't force our uses to design their model in terms of our one-direction decisions. As an example, why should I call my accessor only method get*/is* to be able to invoke it on a query?, what if the company I work for wants to use the local native tongue and the equivalent for *get*/is** instead of using english all together?, etc. *>> I want us to consider systems that exist before we spend time inventing our own. There is cost in maintain our own solution in implementation time and bug squashing. There is also the cost in training on our own solution. SecurityManger is discussed in numerous books and blogs, our solution would not have that kind of coverage. Let’s think really critically there. What are we trying to prevent? Is there a solution in the Java echo system that already does what we really need to do? Can we use a preexisting solution and avoid the risk of implementing our own?* [Juan]: Agreed!. *>> I have spent plenty of time in the guts of SecurityManager in previous lives. I think, based on the intended use of this proposal, that Java SecurityManager more than meets our needs.*[Juan]: Fair enough, I'll go ahead and do some reading about the *Security Manager *on my own, will probably ping you once I have enough context to discuss options (or even ask you some questions :-)). One concern I have, however, is regarding *How far do we go to prevent the user from shooting themselves in the foot?*. I think this needs to be decided by the community, do we have any formal *poll* mechanism or something like that?. As a matter of fact, if we decide to prevent OQL method invocation only on our own internal/protected classes (and some known dangerous things like getClass), the implementation would be wy easier... Cheers. On Mon, Jul 1, 2019 at 12:07 PM Jacob Barrett wrote: > > > > On Jul 1, 2019, at 6:55 AM, Juan José Ramos wrote: > > > >> Can we safely assume that some caching of authorization requests will > >> be performed? What will the scope and lifetime of this caching be? Are > the > >> authentication rules and modules assumed to be immutable at runtime? > All of > >> this will have significant implications on performance. > > My assumption right > > now is that there will be some caching performed and will certainly try > to > > keep the authorizers immutable but, however, no PoC has been done and the > > implementations details shown in the proposal are just a draft, so I'm > not > > in a position to accurately answer these questions. I also read that > > *"premature > > optimization is the root of all evil", *so I guess we can benchmark the > > solution with and without caching, and decide what's better based on some > > real data (I certainly see some gains using caching on the > > *RegexBasedMethodAuthorizer*, not sure about the > > *JavaBeanAccessorBasedMethodAuthorizer*). > > Premature optimization can be bad but so can ignoring it. Most importantly > here is to define the scope of an authorization. This will need to be > defined as part of the API/SPI. If I was to implement one of these > authorizers I would need to know under what conditions my authori
