GEODE-1571: revert previous two changes
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/f4fdfa91 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/f4fdfa91 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/f4fdfa91 Branch: refs/heads/master Commit: f4fdfa915198d91beff3ce44e939aa2f943e971b Parents: 5cbaa7d Author: Jinmei Liao <[email protected]> Authored: Mon Jul 18 11:20:45 2016 -0700 Committer: Jinmei Liao <[email protected]> Committed: Mon Jul 18 11:21:54 2016 -0700 ---------------------------------------------------------------------- .../membership/gms/auth/GMSAuthenticator.java | 24 ++-- .../cache/tier/sockets/AcceptorImpl.java | 4 +- .../internal/cache/tier/sockets/HandShake.java | 26 +++-- .../internal/security/GeodeSecurityUtil.java | 111 ++++++------------- .../security/shiro/CustomAuthRealm.java | 6 +- .../gemfire/security/AuthInitialize.java | 12 +- .../apache/geode/security/GeodePermission.java | 21 ---- .../apache/geode/security/PostProcessor.java | 2 +- .../security/templates/SamplePostProcessor.java | 16 +-- 9 files changed, 69 insertions(+), 153 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f4fdfa91/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/auth/GMSAuthenticator.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/auth/GMSAuthenticator.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/auth/GMSAuthenticator.java index f16a722..b82fdb1 100755 --- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/auth/GMSAuthenticator.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/auth/GMSAuthenticator.java @@ -16,14 +16,6 @@ */ package com.gemstone.gemfire.distributed.internal.membership.gms.auth; -import static com.gemstone.gemfire.distributed.ConfigurationProperties.*; -import static com.gemstone.gemfire.internal.i18n.LocalizedStrings.*; - -import java.lang.reflect.Method; -import java.security.Principal; -import java.util.Properties; -import java.util.Set; - import com.gemstone.gemfire.LogWriter; import com.gemstone.gemfire.distributed.DistributedMember; import com.gemstone.gemfire.distributed.internal.DistributionConfig; @@ -34,12 +26,19 @@ import com.gemstone.gemfire.distributed.internal.membership.gms.interfaces.Authe import com.gemstone.gemfire.internal.ClassLoadUtil; import com.gemstone.gemfire.internal.i18n.LocalizedStrings; import com.gemstone.gemfire.internal.logging.InternalLogWriter; -import com.gemstone.gemfire.internal.security.GeodeSecurityUtil; import com.gemstone.gemfire.security.AuthInitialize; import com.gemstone.gemfire.security.AuthenticationFailedException; import com.gemstone.gemfire.security.AuthenticationRequiredException; import com.gemstone.gemfire.security.GemFireSecurityException; +import java.lang.reflect.Method; +import java.security.Principal; +import java.util.Properties; +import java.util.Set; + +import static com.gemstone.gemfire.internal.i18n.LocalizedStrings.*; +import static com.gemstone.gemfire.distributed.ConfigurationProperties.*; + // static messages public class GMSAuthenticator implements Authenticator { @@ -196,7 +195,12 @@ public class GMSAuthenticator implements Authenticator { try { if (authMethod != null && authMethod.length() > 0) { - AuthInitialize auth = GeodeSecurityUtil.getObjectOfType(authMethod, AuthInitialize.class); + Method getter = ClassLoadUtil.methodFromName(authMethod); + AuthInitialize auth = (AuthInitialize)getter.invoke(null, (Object[]) null); + if (auth == null) { + throw new AuthenticationRequiredException(AUTH_FAILED_TO_ACQUIRE_AUTHINITIALIZE_INSTANCE.toLocalizedString(authMethod)); + } + try { LogWriter logWriter = services.getLogWriter(); LogWriter securityLogWriter = services.getSecurityLogWriter(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f4fdfa91/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java index 43f90d5..e93faf8 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java @@ -638,9 +638,9 @@ public class AcceptorImpl extends Acceptor implements Runnable this.hsPool = tmp_hsPool; } - isAuthenticationRequired = GeodeSecurityUtil.isSecurityRequired(); + isAuthenticationRequired = GeodeSecurityUtil.isSecurityRequired(this.cache.getDistributedSystem().getSecurityProperties()); - isIntegratedSecurity = GeodeSecurityUtil.isIntegratedSecurity(); + isIntegratedSecurity = GeodeSecurityUtil.isIntegratedSecurity(this.cache.getDistributedSystem().getSecurityProperties()); String postAuthzFactoryName = this.cache.getDistributedSystem() .getProperties().getProperty(SECURITY_CLIENT_ACCESSOR_PP); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f4fdfa91/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java index 2dcf8e7..a2951f5 100755 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java @@ -899,7 +899,7 @@ public class HandShake implements ClientHandShake throws GemFireSecurityException, IOException { Properties credentials = null; - boolean requireAuthentication = GeodeSecurityUtil.isSecurityRequired(); + boolean requireAuthentication = GeodeSecurityUtil.isSecurityRequired(system.getSecurityProperties()); try { byte secureMode = dis.readByte(); if (secureMode == CREDENTIALS_NONE) { @@ -1161,7 +1161,7 @@ public class HandShake implements ClientHandShake // non-blank setting for DH symmetric algo, or this is a server // that has authenticator defined. if ((dhSKAlgo != null && dhSKAlgo.length() > 0) - || GeodeSecurityUtil.isSecurityRequired()) { + || GeodeSecurityUtil.isSecurityRequired(config.getSecurityProps())) { KeyPairGenerator keyGen = KeyPairGenerator.getInstance("DH"); DHParameterSpec dhSpec = new DHParameterSpec(dhP, dhG, dhL); keyGen.initialize(dhSpec); @@ -1599,13 +1599,19 @@ public class HandShake implements ClientHandShake Properties credentials = null; try { if (authInitMethod != null && authInitMethod.length() > 0) { - AuthInitialize auth = GeodeSecurityUtil.getObjectOfType(authInitMethod, AuthInitialize.class); - auth.init(logWriter, securityLogWriter); - try { - credentials = auth.getCredentials(securityProperties, server, isPeer); - } - finally { - auth.close(); + Method instanceGetter = ClassLoadUtil.methodFromName(authInitMethod); + AuthInitialize auth = (AuthInitialize)instanceGetter.invoke(null, + (Object[])null); + if (auth != null) { + auth.init(logWriter, + securityLogWriter); + try { + credentials = auth.getCredentials(securityProperties, server, + isPeer); + } + finally { + auth.close(); + } } } } @@ -1632,7 +1638,7 @@ public class HandShake implements ClientHandShake DataOutputStream dos, DistributedSystem system) throws GemFireSecurityException, IOException { - boolean requireAuthentication = GeodeSecurityUtil.isSecurityRequired(); + boolean requireAuthentication = GeodeSecurityUtil.isSecurityRequired(system.getSecurityProperties()); Properties credentials = null; try { byte secureMode = dis.readByte(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f4fdfa91/geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java index 9455166..8707a78 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java @@ -19,23 +19,17 @@ package com.gemstone.gemfire.internal.security; import static com.gemstone.gemfire.distributed.ConfigurationProperties.*; -import java.lang.reflect.Method; import java.security.AccessController; import java.security.Principal; import java.util.Properties; import java.util.Set; import java.util.concurrent.Callable; -import org.apache.commons.lang.NullArgumentException; import org.apache.commons.lang.StringUtils; -import org.apache.geode.security.GeodePermission; -import org.apache.geode.security.GeodePermission.Operation; -import org.apache.geode.security.GeodePermission.Resource; -import org.apache.geode.security.PostProcessor; -import org.apache.geode.security.SecurityManager; import org.apache.logging.log4j.Logger; import org.apache.shiro.SecurityUtils; import org.apache.shiro.ShiroException; +import org.apache.shiro.UnavailableSecurityManagerException; import org.apache.shiro.authc.UsernamePasswordToken; import org.apache.shiro.config.Ini.Section; import org.apache.shiro.config.IniSecurityManagerFactory; @@ -53,7 +47,12 @@ import com.gemstone.gemfire.internal.security.shiro.ShiroPrincipal; import com.gemstone.gemfire.management.internal.security.ResourceOperation; import com.gemstone.gemfire.security.AuthenticationFailedException; import com.gemstone.gemfire.security.GemFireSecurityException; +import org.apache.geode.security.GeodePermission; +import org.apache.geode.security.GeodePermission.Operation; +import org.apache.geode.security.GeodePermission.Resource; import com.gemstone.gemfire.security.NotAuthorizedException; +import org.apache.geode.security.PostProcessor; +import org.apache.geode.security.SecurityManager; public class GeodeSecurityUtil { @@ -66,7 +65,7 @@ public class GeodeSecurityUtil { * @return the shiro subject, null if security is not enabled */ public static Subject getSubject() { - if (!isIntegratedSecure) { + if (!isSecured()) { return null; } @@ -103,7 +102,7 @@ public class GeodeSecurityUtil { * @return null if security is not enabled, otherwise return a shiro subject */ public static Subject login(String username, String password) { - if (!isIntegratedSecure) { + if (!isSecured()) { return null; } @@ -270,10 +269,18 @@ public class GeodeSecurityUtil { } } + private static boolean isSecured() { + try { + SecurityUtils.getSecurityManager(); + } + catch (UnavailableSecurityManagerException e) { + return false; + } + return true; + } + private static PostProcessor postProcessor; private static SecurityManager securityManager; - private static boolean isSecure; - private static boolean isIntegratedSecure; /** * initialize Shiro's Security Manager and Security Utilities @@ -286,7 +293,6 @@ public class GeodeSecurityUtil { String shiroConfig = securityProps.getProperty(SECURITY_SHIRO_INIT); String securityConfig = securityProps.getProperty(SECURITY_MANAGER); - String clientAuthenticatorConfig = securityProps.getProperty(SECURITY_CLIENT_AUTHENTICATOR); if (!StringUtils.isBlank(shiroConfig)) { IniSecurityManagerFactory factory = new IniSecurityManagerFactory("classpath:" + shiroConfig); @@ -300,33 +306,24 @@ public class GeodeSecurityUtil { org.apache.shiro.mgt.SecurityManager securityManager = factory.getInstance(); SecurityUtils.setSecurityManager(securityManager); - isSecure = true; - isIntegratedSecure = true; } + // only set up shiro realm if user has implemented SecurityManager else if (!StringUtils.isBlank(securityConfig)) { - securityManager = getObjectOfTypeFromClassName(securityConfig, SecurityManager.class); + securityManager = getObjectOfType(securityConfig, SecurityManager.class); securityManager.init(securityProps); Realm realm = new CustomAuthRealm(securityManager); org.apache.shiro.mgt.SecurityManager shiroManager = new DefaultSecurityManager(realm); SecurityUtils.setSecurityManager(shiroManager); - isSecure = true; - isIntegratedSecure = true; - } - else if( !StringUtils.isBlank(clientAuthenticatorConfig)) { - isSecure = true; - isIntegratedSecure = false; } else { SecurityUtils.setSecurityManager(null); - isSecure = false; - isIntegratedSecure = false; } // this initializes the post processor String customPostProcessor = securityProps.getProperty(SECURITY_POST_PROCESSOR); if( !StringUtils.isBlank(customPostProcessor)) { - postProcessor = getObjectOfTypeFromClassName(customPostProcessor, PostProcessor.class); + postProcessor = getObjectOfType(customPostProcessor, PostProcessor.class); postProcessor.init(securityProps); } else{ @@ -345,8 +342,6 @@ public class GeodeSecurityUtil { postProcessor = null; } ThreadContext.remove(); - isSecure = false; - isIntegratedSecure = false; } /** @@ -354,7 +349,8 @@ public class GeodeSecurityUtil { * But if your postProcess is pretty involved with preparations and you need to bypass it entirely, call this first. */ public static boolean needPostProcess(){ - return (isIntegratedSecure && postProcessor != null); + Subject subject = getSubject(); + return (subject != null && postProcessor != null); } public static Object postProcess(String regionPath, Object key, Object result){ @@ -371,14 +367,7 @@ public class GeodeSecurityUtil { } - /** - * this method would never return null, it either throws an exception or returns an object - * @param className - * @param expectedClazz - * @param <T> - * @return - */ - public static <T> T getObjectOfTypeFromClassName(String className, Class<T> expectedClazz) { + public static <T> T getObjectOfType(String className, Class<T> expectedClazz) { Class actualClass = null; try { actualClass = ClassLoadUtil.classFromName(className); @@ -400,58 +389,20 @@ public class GeodeSecurityUtil { return actualObject; } - /** - * this method would never return null, it either throws an exception or returns an object - * @param factoryMethodName - * @param expectedClazz - * @param <T> - * @return - */ - public static <T> T getObjectOfTypeFromFactoryMethod(String factoryMethodName, Class<T> expectedClazz){ - try { - Method factoryMethod = ClassLoadUtil.methodFromName(factoryMethodName); - T actualObject = (T)factoryMethod.invoke(null, (Object[])null); - - if(actualObject == null){ - throw new NullArgumentException("Factory method "+ factoryMethodName + " should not return null."); - } - - return actualObject; - } catch (Exception e) { - throw new GemFireSecurityException(e.toString(), e); - } - } - - /** - * this method would never return null, it either throws an exception or returns an object - * @param classOrMethod - * @param expectedClazz - * @param <T> - * @return an object of type expectedClazz. This method would never return null. It either returns an non-null - * object or throws exception. - */ - public static <T> T getObjectOfType(String classOrMethod, Class<T> expectedClazz) { - T object = null; - try{ - object = getObjectOfTypeFromClassName(classOrMethod, expectedClazz); - } - catch (Exception e){ - object = getObjectOfTypeFromFactoryMethod(classOrMethod, expectedClazz); - } - return object; - } - public static SecurityManager getSecurityManager(){ return securityManager; } - public static boolean isSecurityRequired(){ - return isSecure; + public static boolean isSecurityRequired(Properties securityProps){ + String authenticator = securityProps.getProperty(SECURITY_CLIENT_AUTHENTICATOR); + String securityManager = securityProps.getProperty(SECURITY_MANAGER); + return !StringUtils.isEmpty(authenticator) || !StringUtils.isEmpty(securityManager); } - public static boolean isIntegratedSecurity(){ - return isIntegratedSecure; + public static boolean isIntegratedSecurity(Properties securityProps){ + String securityManager = securityProps.getProperty(SECURITY_MANAGER); + return !StringUtils.isEmpty(securityManager); } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f4fdfa91/geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/CustomAuthRealm.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/CustomAuthRealm.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/CustomAuthRealm.java index db07fe0..3d6275b 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/CustomAuthRealm.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/CustomAuthRealm.java @@ -19,8 +19,6 @@ package com.gemstone.gemfire.internal.security.shiro; import java.security.Principal; import java.util.Properties; -import org.apache.geode.security.GeodePermission; -import org.apache.geode.security.SecurityManager; import org.apache.shiro.authc.AuthenticationException; import org.apache.shiro.authc.AuthenticationInfo; import org.apache.shiro.authc.AuthenticationToken; @@ -33,6 +31,8 @@ import org.apache.shiro.subject.PrincipalCollection; import com.gemstone.gemfire.internal.security.GeodeSecurityUtil; import com.gemstone.gemfire.management.internal.security.ResourceConstants; +import org.apache.geode.security.SecurityManager; +import org.apache.geode.security.GeodePermission; public class CustomAuthRealm extends AuthorizingRealm{ @@ -45,7 +45,7 @@ public class CustomAuthRealm extends AuthorizingRealm{ } public CustomAuthRealm (String authenticatorFactory) { - this.securityManager = GeodeSecurityUtil.getObjectOfTypeFromClassName(authenticatorFactory, SecurityManager.class); + this.securityManager = GeodeSecurityUtil.getObjectOfType(authenticatorFactory, SecurityManager.class); } @Override http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f4fdfa91/geode-core/src/main/java/com/gemstone/gemfire/security/AuthInitialize.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/security/AuthInitialize.java b/geode-core/src/main/java/com/gemstone/gemfire/security/AuthInitialize.java index e92772b..400c665 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/security/AuthInitialize.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/security/AuthInitialize.java @@ -23,7 +23,6 @@ import com.gemstone.gemfire.LogWriter; import com.gemstone.gemfire.cache.CacheCallback; import com.gemstone.gemfire.distributed.DistributedMember; import com.gemstone.gemfire.distributed.DistributedSystem; -import com.gemstone.gemfire.internal.cache.GemFireCacheImpl; // TODO Add example usage of this interface and configuration details /** @@ -50,21 +49,11 @@ public interface AuthInitialize extends CacheCallback { * * @throws AuthenticationFailedException * if some exception occurs during the initialization - * - * @deprecated since Geode 1.0, use init() */ public void init(LogWriter systemLogger, LogWriter securityLogger) throws AuthenticationFailedException; /** - * @since Geode 1.0. implement this method instead of init with logwriters. - * Implementation should use log4j instead of these loggers. - */ - default public void init(){ - GemFireCacheImpl cache = GemFireCacheImpl.getInstance(); - init(cache.getLogger(), cache.getSecurityLogger()); - } - /** * Initialize with the given set of security properties and return the * credentials for the peer/client as properties. * @@ -94,4 +83,5 @@ public interface AuthInitialize extends CacheCallback { public Properties getCredentials(Properties securityProps, DistributedMember server, boolean isPeer) throws AuthenticationFailedException; + } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f4fdfa91/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java b/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java index 0a777a8..866b14e 100644 --- a/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java +++ b/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java @@ -19,11 +19,6 @@ package org.apache.geode.security; import org.apache.shiro.authz.permission.WildcardPermission; -/** - * GeodePermission defines the resource, the operation, the region and the key involved in the action to be authorized. - * - * It is passed to the SecurityManager for the implementation to decide whether to grant a user this permission or not. - */ public class GeodePermission extends WildcardPermission { public static String ALL_REGIONS = "*"; @@ -42,34 +37,18 @@ public class GeodePermission extends WildcardPermission { READ } - /** - * Returns the resource, could be either DATA or CLUSTER - * @return - */ public Resource getResource() { return resource; } - /** - * Returns the operation, could be either MANAGE, WRITE or READ - * @return - */ public Operation getOperation() { return operation; } - /** - * returns the regionName, could be "*", meaning all regions - * @return - */ public String getRegionName() { return regionName; } - /** - * returns the key, could be "*" meaning all keys. - * @return - */ public String getKey() { return key; } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f4fdfa91/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java b/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java index 1a0e5de..0f13b47 100644 --- a/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java +++ b/geode-core/src/main/java/org/apache/geode/security/PostProcessor.java @@ -44,7 +44,7 @@ public interface PostProcessor { * @param key * the key of the value that's been accessed. This could be null. * @param value - * the original value. The orginal value could be null as well. + * the value, this could be null. * @return * the value that will be returned to the requester */ http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f4fdfa91/geode-core/src/main/java/org/apache/geode/security/templates/SamplePostProcessor.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/security/templates/SamplePostProcessor.java b/geode-core/src/main/java/org/apache/geode/security/templates/SamplePostProcessor.java index 8f61db7..7e078da 100644 --- a/geode-core/src/main/java/org/apache/geode/security/templates/SamplePostProcessor.java +++ b/geode-core/src/main/java/org/apache/geode/security/templates/SamplePostProcessor.java @@ -22,28 +22,14 @@ import java.util.Properties; import org.apache.geode.security.PostProcessor; -/** - * This is example that implements PostProcessor - */ public class SamplePostProcessor implements PostProcessor{ + public static String MASK = "****"; @Override public void init(final Properties securityProps) { } - /** - * this simply modifies the value with all the parameter values - * @param principal - * The principal that's accessing the value - * @param regionName - * The region that's been accessed. This could be null. - * @param key - * the key of the value that's been accessed. This could be null. - * @param value - * the value, this could be null. - * @return - */ @Override public Object processRegionValue(Principal principal, String regionName,
