[GEODE-2053] Improve Geode Security Framework

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Invalid
• Component/s: management, security
• Assignee: Unassigned
• Reporter: Jinmei Liao
• Created: 02/Nov/16 16:43
• Updated: 29/Mar/19 23:29
• Resolved: 02/Apr/18 18:17

Description

This is based on the feedback that John provided by using Geode Security in SDG.

Below is his email:
This is will be my final feedback (for now) regarding Geode's Integrated Security framework/feature. My following recommendations are based on extensive testing and experience trying to configure and enable Geode's security services.

As you know, my goal was to be able to quickly, easily and conveniently configure Geode's security framework services and features using Spring. However, my techniques could equally apply in any context (e.g. PCF, or a simple test environment without any framework/container).

To make it quick, easy and convenient, I added support for Geode Integrated Security in SD Geode by way of the new Annotation configuration model, specifically with the addition of the new @EnableSecurity annotation [1].

The new @EnableSecurity annotation can be seen in action in the SDG Contacts Application RI [2] (apache-geode branch [3]), security-example [4], GeodeSecurityIntegrationTests class [5]. In this test class, I demonstrate 4 different ways, each employing different methods an application developer might use to configure and enable Geode's Integrated Security feature to secure Apache Geode operationally:

1. I use Geode's security-manager (System) property to refer to an application implementation of "Geode's" SecurityManager interface. See here [6].

As you may recall, the SimpleSecurityManager [7] implementation is a custom application implementation of Geode's SecurityManager interface [8] (not to be confused with Shiro's [20], or even Java's [21] SecurityManager, for that matter) employing the Repository (DAO) Design Pattern (via the SecurityRepository interface [9]) to load security configuration meta-data (e.g. Users, Roles and Permissions) from a variety of data sources. This is not at all unlike how the Apache Shiro framework, itself, is organized [10] (i.e. SecurityManager -> (pluggable) Realms).

2. Next [11], I use Geode's security-shiro-init (System) property to refer to a Apache Shiro INI security configuration file (e.g. geode-security-shiro.ini [12]).

NOTE: the Users, Roles and Permissions I define are configured consistently throughout all my security configurations and data source examples (all based on my SecurityRepository implementations [13]).

3. Then [14], I create a custom Apache Shiro Realm based on my SecurityRepository interface (the SecurityRepositoryAuthorizingRealm [15]), enabling me, as a application developer, to plug in 1 of my implementations (i.e. JDBC or XML).

NOTE:, I use XML since Apache Shiro provides no, OOTB implementation for XML, ironically (Shiro only supports Active Directory, JDBC, JNDI, LDAP and TEXT; see sub-packages below org.apache.shiro.realm [16]).

4. Finally [17], I use a canned, OOTB, Apache Shiro provided Realm implementation (PropertiesRealm [18]), that, as the name suggests, is based on the Java Properties file format (e.g. geode-security-shiro.properties [19]).

This may seem like a lot of work, even overkill, but all these were pertinent in finding a number of bugs not only in SDG's @EnableSecurity annotation and supporting classes, but with Geode's own Integrated Security framework, and specifically the API [22]. Individually, or if I had stopped my testing efforts prematurely with only 1 or 2 examples, I definitely would not have uncovered all the problems I found.

1. First, the GeodePermissionResolver [23] is necessary to configure Apache Shiro's provided (OOTB) Realms correctly. Otherwise, the security Permissions are not enforced properly (in a hierarchical fashion as advertised [24], i.e. in section "3. Introduction of ResourcePermission").

I used [25] the GeodePermissionResolver class to configure the Apache Shiro provided (OOTB) PropertiesRealm implementation [18].

Therefore, the GeodePermissionResolver class must NOT be internal. This is particularly important if the user is using Apache Shiro to the fullest extent to configure and secure Apache Geode.

Of course, I could have provided my own implementation of the Apache Shiro PermissionResolver interface [26] (especially given the simplicity of the GeodePermissionResolver implementation) but if that implementation every involves more logic behind the scenes, better to "reuse" then "reinvent" in this case.

2. I also think it is pertinent that the SecurityService interface [27] be in Geode's public API. Given this is just an interface, I am not sure why it was decided to make it "internal" anyway? I see no good reason NOT to expose this interface, especially since it would be particularly useful for both API/Framework (e.g. SDG) as well as tools developers developing extensions to Apache Geode.

I actually did make use of the SecurityService interface [28] in SDG, despite my (usually) hard rule of NOT ever using any GemFire/Geode internal classes at all in SDG. Unfortunately, and all too often, in certain cases, such as the currently released version of Apache Geode, 1.0.0-incubating GA, there is simply no other way around it when providing support for securing Apache Geode, especially for making Apache Shiro first-class (something I want to similarly do for Spring Security).

The usage in [28] is, well, a hack! I can certainly think of better uses of the SecurityService interface as alluded to above.

3. That brings me to a more general problem I have with the Geode (and GemFires) APIs and organization, which surfaces many anti-patterns. There is only 1 thing I will focus on here...

We seem to keep propagating the broken "global", internal package organization [29] that has many, many, cross-cutting concerns in it, and recently adds "security" [30] into the fold.

While Security IS a "cross-cutting" concern, it is definitely NOT a recommended way to organize a modular codebase. It will only make it more difficult to organize and modularize Geode into logical, (hopefully, "pluggable") functional units in the future. It would be better to see the org.apache.geode.internal.security package move under org.apache.geode.security. If Security is truly cross-cutting, then it ought to be a separate "module", independent of the other geode modules which can depend on it.

While OSGi has lost it steam in recent years, the current organization would be problematic in this environment, especially where some of these classes/interface should have been made public.

Still, given Java 9 is moving to a modular architecture, you might want to be a bit more forward thinking in how the codebase organization is going to affect modularity. You can bet users are going to want to leverage Geode in a very modular way once Java 9 is readily available.

4. Rather than having a org.apache.geode.security.templates package [31] in the public API, I might consider moving these classes to the examples. I would not want users thinking these Security classes/components are actually sufficient to securing Geode in a production environment (yikes)!

Additionally, I would probably even package the classes under org.apache.geode.security [32] a bit differently, having more organization/cleanliness with sub-packages. The security packages feels like a dumping ground for anything and everything, all handling related but slightly different things, functionally... client vs. the server, authentication vs. authorization, securing communication channels, managing over all security, etc, etc. Organization is the key to architecture and making things easy to consume and understand by users.

You could make a SecurableCommunicationChannels [33] an enum, too.

5. One more, and I will leave it at that for now... so I am thinking ahead, and... I really want to build support for Spring Security. This support can come by way of SDG to auto-enable this provider.

Anyway, I think it is of paramount importance that we nail down the Geode Integrated Security SPI, allowing different "security providers" to be "plugged" in to secure Apache Geode in different contexts.

Initially I was thinking the Geode SecurityManager interface would suffice, but I don't think so, not now. It could also be Geode's SecurityService interface and/or combination of Geode's SecurityManager, especially given that the Geode (Integrate)SecurityService seems to be used throughout the Geode codebase. Still this does not even feel quite right.

I particularly like Apache Shiro's concept of and use of the Subject class [34]. This seems to be the focal point for all security related concerns using the Apache Shiro security framework. Unfortunately, they have their "own" implementation <sigh>.

Apache Geode could adhere to the Java Subject API [35], though even the Java Subject is a final class (and not an interface, <sigh> ).

The idea is that different adapters could be provided to different underlying security provider classes and interfaces (e.g. like Apache Shiro's Subject; SDG could provide an implementation, err.. Adapter, for Spring Security; the community might contribute additional ones for other security providers, and so on).

So, maybe Apache Geode needs/provides a Subject interface (API/SPI) of it's "own" that is adapted for each security provider supported by Apache Geode or contributed by the community.

Anyway, I am thinking out loud here, but this SPI and it's API will be crucially important to get more right than not in order to allow Geode to leverage the vast array of security providers available, making Geode as widely accepted in as many context's as possible.

Phew, many thoughts to share; sorry for the length. Hopefully, by now, you realize the importance of, and understand the careful considerations necessary when designing an API, well, any feature for that matter.

That's all for now. Hope this helps!

Cheers,
John

Sub-Tasks

• Do not use classpath: when looking for security-shiro-ini files: Open
• Expose GeodePermissionResolver: Open
• Expose GeodeSecurityService as needed: Open
• reorganize security packages: Open
• reorganize security examples: Closed
• Log UnauthorizedException message at INFO and stack at DEBUG: Open
• Refactor SecurableCommunicationChannels and SecurableCommunicationChannel: Open
• GEODE-2083: Closed