Jinmei Liao created GEODE-2053:
----------------------------------
Summary: Improve Geode Security Framework
Key: GEODE-2053
URL: https://issues.apache.org/jira/browse/GEODE-2053
Project: Geode
Issue Type: Improvement
Reporter: Jinmei Liao
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
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
