Re: [PROPOSAL]: Improve OQL Method Invocation Security
Hello Jake, I've been doing some reading about the *Java Security Manager* and, even when it might work for our use case, I don't think is a good fit due to the following reasons: 1). We already have chosen *Shiro* for authentication and authorization, adding yet another security framework (and mapping between roles and permissions between the two of them) for OQL method invocation security seems overkilling to me. I'd rather spend some time improving the *ResourcePermissionBasedMethodAuthorizer [1] *and/or adding *Permissions*/*Roles* to our current security framework than introducing a new security framework altogether. 2). There can only be one *Security Manager* per JVM at a given time. 3). Customers already using a custom *Security Manager* on their own will be in trouble... we can certainly wrote our own implementation as a composite and parse multiple policy files, but this will probably break the backward compatibility for these customers (and requires yet more configuration and things to keep in mind when upgrading). 4). The current set of default *Permissions* (*PropertyPermission*, *FilePermission*, etc.) don't apply to our use case, so we'll need to create our own implementations and do some plumbing to map to what we currently have in terms of principals and roles (more boilerplate). 5). In order to check a permission we basically need to check whether a Security Manager is installed (*System.getSecurityManager() != null*) and execute the check afterwards (*securityManager**.checkPermission(perm)*); the *Security Manager* then looks at the classes of all methods currently within the call stack (roughly speaking) and returns or throws an exception... a lookup through a simple Map is probably faster, and easier to maintain and read. 6). Overall, it feels like killing mosquitoes with a bazooka to me: it's certainly a powerful framework and we might certainly use it to achieve our goal, but it just seems too complicated for what we are trying to do. My expertise with the *Java Security Manager* has came from what I've recently read, anyway, so chances are high that I might be missing something :-). That said, if we all agree in that *we can't prevent users from shooting themselves in the foot* (thanks for that phrase!!) then things become somewhat trivial: we known which internal methods can compromise the integrity of the system and we also know which general methods are dangerous, so we just need to have an immutable structure within our default authorizer and check if the method belongs to it or not to decide whether it should be allowed/denied. We can certainly provide the other 2 or 3 implementations out of the box and allow customers to provide their own in case they want to restrict things further but, as far as this proposal goes, I think we're good to incorporate the comments/suggestions gathered and go for another feedback round (it's what I plan to do now). Cheers. [1]: https://cwiki.apache.org/confluence/display/GEODE/OQL+Method+Invocation+Security#OQLMethodInvocationSecurity-ResourcePermissionBasedMethodAuthorizer On Mon, Jul 1, 2019 at 12:57 PM Juan José Ramos wrote: > 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 certai
Re: [PROPOSAL]: Improve OQL Method Invocation Security
> On Jul 2, 2019, at 11:58 AM, Juan José Ramos wrote: > > Hello Jake, > > I've been doing some reading about the *Java Security Manager* and, even > when it might work for our use case, I don't think is a good fit due to the > following reasons: > 1). We already have chosen *Shiro* for authentication and authorization, > adding yet another security framework (and mapping between roles and > permissions between the two of them) for OQL method invocation security > seems overkilling to me. I'd rather spend some time improving the > *ResourcePermissionBasedMethodAuthorizer The security manager doesn’t have to be as fined grained as individual users. Do we really intend to support actions on objects base on current user too? > [1] *and/or adding *Permissions*/*Roles* to our current security framework > than introducing a new security framework altogether. Again, we don’t have to add anything. If the user wants to restrict access to certain activities they could add a policy file to the JVM to do so. It doesn’t have to be user based. We could simply have two hard coded roles, UserCode and SystemCode. SystemCode has AllPermissions, UserCode has none. Now we run all queries under the context of UserCode and it can’t execute any File, Runtime, Socket, etc. > 2). There can only be one *Security Manager* per JVM at a given time Eh, sorta but no, you can use thread contexts and class loader isolation, but there really doesn’t need to be more than one. . > 3). Customers already using a custom *Security Manager* on their own will > be in trouble... we can certainly wrote our own implementation as a > composite and parse multiple policy files, but this will probably break the > backward compatibility for these customers (and requires yet more > configuration and things to keep in mind when upgrading). Their custom SecurityManager would conform to the same rules as the default file based one in the JRE. They would be able to control code access using their existing framework. Seems like win to me. > 4). The current set of default *Permissions* (*PropertyPermission*, > *FilePermission*, etc.) don't apply to our use case, so we'll need to > create our own implementations and do some plumbing to map to what we > currently have in terms of principals and roles (more boilerplate). How do they not apply? > 5). In order to check a permission we basically need to check whether a > Security Manager is installed (*System.getSecurityManager() != null*) and > execute the check afterwards (*securityManager**.checkPermission(perm)*); > the *Security Manager* then looks at the classes of all methods currently > within the call stack (roughly speaking) and returns or throws an > exception... a lookup through a simple Map is probably faster, and easier > to maintain and read. All the things we need to be protected from have already been coded with these checks in the JRE. Do you think we need to protect other activities? -Jake
Re: [PROPOSAL]: Improve OQL Method Invocation Security
Are security manager policies modifiable on the fly? Just wondering if someone decides they want to disallow or allow something, will they need to restart their vms/geode node? I think Dan pointed this out earlier in the thread, but just wanted to have us consider the original cve that led to the heavy handed deny all method invocations: CVE-2017-9795 Apache Geode OQL method invocation vulnerability Description: A malicious user with read access to specific regions within a Geode cluster may execute OQL queries that allow read and write access to objects within unauthorized regions. In addition a user could invoke methods that allow remote code execution I think Juan's proposal would still allow us to provide multiple solutions that may or may not reopen that hole, but it would be up to the user to decide what they are willing to accept. The choice for what should be default would still be up for debate... On Tue, Jul 2, 2019 at 1:07 PM Jacob Barrett wrote: > > > > On Jul 2, 2019, at 11:58 AM, Juan José Ramos wrote: > > > > Hello Jake, > > > > I've been doing some reading about the *Java Security Manager* and, even > > when it might work for our use case, I don't think is a good fit due to > the > > following reasons: > > 1). We already have chosen *Shiro* for authentication and authorization, > > adding yet another security framework (and mapping between roles and > > permissions between the two of them) for OQL method invocation security > > seems overkilling to me. I'd rather spend some time improving the > > *ResourcePermissionBasedMethodAuthorizer > > The security manager doesn’t have to be as fined grained as individual > users. Do we really intend to support actions on objects base on current > user too? > > > [1] *and/or adding *Permissions*/*Roles* to our current security > framework > > than introducing a new security framework altogether. > > Again, we don’t have to add anything. If the user wants to restrict access > to certain activities they could add a policy file to the JVM to do so. It > doesn’t have to be user based. We could simply have two hard coded roles, > UserCode and SystemCode. SystemCode has AllPermissions, UserCode has none. > Now we run all queries under the context of UserCode and it can’t execute > any File, Runtime, Socket, etc. > > > 2). There can only be one *Security Manager* per JVM at a given time > Eh, sorta but no, you can use thread contexts and class loader isolation, > but there really doesn’t need to be more than one. > . > > 3). Customers already using a custom *Security Manager* on their own will > > be in trouble... we can certainly wrote our own implementation as a > > composite and parse multiple policy files, but this will probably break > the > > backward compatibility for these customers (and requires yet more > > configuration and things to keep in mind when upgrading). > Their custom SecurityManager would conform to the same rules as the > default file based one in the JRE. They would be able to control code > access using their existing framework. Seems like win to me. > > > 4). The current set of default *Permissions* (*PropertyPermission*, > > *FilePermission*, etc.) don't apply to our use case, so we'll need to > > create our own implementations and do some plumbing to map to what we > > currently have in terms of principals and roles (more boilerplate). > > How do they not apply? > > > 5). In order to check a permission we basically need to check whether a > > Security Manager is installed (*System.getSecurityManager() != null*) and > > execute the check afterwards (*securityManager**.checkPermission(perm)*); > > the *Security Manager* then looks at the classes of all methods currently > > within the call stack (roughly speaking) and returns or throws an > > exception... a lookup through a simple Map is probably faster, and easier > > to maintain and read. > > All the things we need to be protected from have already been coded with > these checks in the JRE. Do you think we need to protect other activities? > > -Jake > >
