GEODE-417: deprecated ssl-* gemfire properties should not be allowed with cluster-ssl-* properties
Modified the verification code to only complain if corresponding properties are both set and are not equal. Modified the exception text to be (I believe) grammatically correct. Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/6c74e5ab Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/6c74e5ab Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/6c74e5ab Branch: refs/heads/feature/GEODE-949-2 Commit: 6c74e5ab10295c4420c3d75694ab5e077c46e16e Parents: a42a2f3 Author: Bruce Schuchardt <[email protected]> Authored: Wed Mar 9 08:53:44 2016 -0800 Committer: Bruce Schuchardt <[email protected]> Committed: Wed Mar 9 08:56:26 2016 -0800 ---------------------------------------------------------------------- .../internal/DistributionConfigImpl.java | 33 +++++---- .../gemfire/internal/AbstractConfig.java | 2 + .../internal/SSLConfigIntegrationJUnitTest.java | 2 +- .../gemfire/internal/SSLConfigJUnitTest.java | 73 ++++++++++++++++---- 4 files changed, 77 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6c74e5ab/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java index fc2fca7..93b59f5 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java @@ -746,27 +746,28 @@ public class DistributionConfigImpl if(sslEnabledString != null && clusterSSLEnabledString != null){ boolean sslEnabled = new Boolean(sslEnabledString).booleanValue(); boolean clusterSSLEnabled =new Boolean(clusterSSLEnabledString).booleanValue(); - if (sslEnabled != DEFAULT_SSL_ENABLED - && clusterSSLEnabled != DEFAULT_CLUSTER_SSL_ENABLED) { + if (sslEnabled != clusterSSLEnabled) { throw new IllegalArgumentException( - "Gemfire property \'ssl-enabled\' and \'cluster-ssl-enabled\' can not be used at the same time. Prefer way is to use all \'cluster-ssl*\' properties instead of \'ssl-*\'."); + "GemFire properties \'ssl-enabled\' and \'cluster-ssl-enabled\' can not be used at the same time and have different values. The preferred way is to use all \'cluster-ssl*\' properties instead of \'ssl-*\'."); } } String sslCipher = (String)props.get(SSL_CIPHERS_NAME); String clusterSSLCipher = (String)props.get(CLUSTER_SSL_CIPHERS_NAME); - if (sslCipher != null && sslCipher != DEFAULT_SSL_CIPHERS - && clusterSSLCipher != null && clusterSSLCipher != DEFAULT_CLUSTER_SSL_CIPHERS) { - throw new IllegalArgumentException( - "Gemfire property \'ssl-cipher\' and \'cluster-ssl-cipher\' can not be used at the same time. Prefer way is to use all \'cluster-ssl*\' properties instead of \'ssl-*\'."); + if (sslCipher != null && clusterSSLCipher != null) { + if ( !sslCipher.equals(clusterSSLCipher) ) { + throw new IllegalArgumentException( + "GemFire properties \'ssl-cipher\' and \'cluster-ssl-cipher\' can not be used at the same time and have different values. The preferred way is to use all \'cluster-ssl*\' properties instead of \'ssl-*\'."); + } } String sslProtocol = (String)props.get(SSL_PROTOCOLS_NAME); String clusterSSLProtocol = (String)props.get(CLUSTER_SSL_PROTOCOLS_NAME); - if (sslProtocol != null && sslProtocol != DEFAULT_SSL_PROTOCOLS - && clusterSSLProtocol != null && clusterSSLProtocol != DEFAULT_CLUSTER_SSL_PROTOCOLS ) { - throw new IllegalArgumentException( - "Gemfire property \'ssl-protocols\' and \'cluster-ssl-protocols\' can not be used at the same time. Prefer way is to use all \'cluster-ssl*\' properties instead of \'ssl-*\'."); + if (sslProtocol != null && clusterSSLProtocol != null) { + if ( !sslProtocol.equals(clusterSSLProtocol) ) { + throw new IllegalArgumentException( + "GemFire properties \'ssl-protocols\' and \'cluster-ssl-protocols\' can not be used at the same time and have different values. The preferred way is to use all \'cluster-ssl*\' properties instead of \'ssl-*\'."); + } } String sslReqAuthString = (String)props.get(SSL_REQUIRE_AUTHENTICATION_NAME); @@ -774,10 +775,9 @@ public class DistributionConfigImpl if(sslReqAuthString != null && clusterReqAuthString != null){ boolean sslReqAuth = new Boolean(sslReqAuthString).booleanValue(); boolean clusterSSLReqAuth =new Boolean(clusterReqAuthString).booleanValue(); - if (sslReqAuth != DEFAULT_SSL_REQUIRE_AUTHENTICATION - && clusterSSLReqAuth != DEFAULT_CLUSTER_SSL_REQUIRE_AUTHENTICATION) { + if (sslReqAuth != clusterSSLReqAuth) { throw new IllegalArgumentException( - "Gemfire property \'ssl-require-authentication\' and \'cluster-ssl-require-authentication\' can not be used at the same time. Prefer way is to use all \'cluster-ssl*\' properties instead of \'ssl-*\'."); + "GemFire properties \'ssl-require-authentication\' and \'cluster-ssl-require-authentication\' can not be used at the same time and have different values. The preferred way is to use all \'cluster-ssl*\' properties instead of \'ssl-*\'."); } } @@ -786,10 +786,9 @@ public class DistributionConfigImpl if(jmxSSLString != null && jmxSSLEnabledString != null){ boolean jmxSSL = new Boolean(jmxSSLString).booleanValue(); boolean jmxSSLEnabled =new Boolean(jmxSSLEnabledString).booleanValue(); - if (jmxSSL != DEFAULT_SSL_ENABLED - && jmxSSLEnabled != DEFAULT_CLUSTER_SSL_ENABLED) { + if (jmxSSL != jmxSSLEnabled) { throw new IllegalArgumentException( - "Gemfire property \'jmx-manager-ssl\' and \'jmx-manager-ssl-enabled\' can not be used at the same time. Prefer way is to use \'jmx-manager-ssl-enabled\' instead of \'jmx-manager-ssl\'."); + "GemFire properties \'jmx-manager-ssl\' and \'jmx-manager-ssl-enabled\' can not be used at the same time and have different values and have different values. The preferred way is to use \'jmx-manager-ssl-enabled\' instead of \'jmx-manager-ssl\'."); } } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6c74e5ab/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java index ee4aceb..09fc61c 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java @@ -211,6 +211,8 @@ public abstract class AbstractConfig implements Config { return true; } else if (attName.equals(DistributionConfig.SSL_REQUIRE_AUTHENTICATION_NAME)) { return true; + } else if (attName.equals(DistributionConfig.JMX_MANAGER_SSL_NAME)) { + return true; } return false; } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6c74e5ab/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigIntegrationJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigIntegrationJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigIntegrationJUnitTest.java index 148004e..c4d643d 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigIntegrationJUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigIntegrationJUnitTest.java @@ -36,7 +36,7 @@ import com.gemstone.gemfire.test.junit.categories.IntegrationTest; public class SSLConfigIntegrationJUnitTest { @Test - public void test51531() { + public void testIsClusterSSLRequireAuthentication() { Cache mCache = new CacheFactory().set("mcast-port", "0").set("jmx-manager", "true").create(); ManagementService mService = ManagementService.getManagementService(mCache); MemberMXBean mMemberBean = mService.getMemberMXBean(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6c74e5ab/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigJUnitTest.java index 99eb949..543574a 100755 --- a/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigJUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/internal/SSLConfigJUnitTest.java @@ -355,7 +355,7 @@ public class SSLConfigJUnitTest { try{ DistributionConfigImpl config = new DistributionConfigImpl( gemFireProps ); }catch(IllegalArgumentException e){ - if (! e.toString().contains( "Gemfire property \'jmx-manager-ssl\' and \'jmx-manager-ssl-enabled\' can not be used at the same time")) { + if (! e.toString().contains( "GemFire properties \'jmx-manager-ssl\' and \'jmx-manager-ssl-enabled\' can not be used at the same time")) { throw new Exception( "did not get expected exception, got this instead...", e ); } } @@ -471,61 +471,104 @@ public class SSLConfigJUnitTest { Properties gemFireProps = new Properties(); gemFireProps.setProperty( "mcast-port", "0" ); gemFireProps.put(DistributionConfig.SSL_ENABLED_NAME, "true"); - gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "true"); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "false"); DistributionConfigImpl config = null; try{ config = new DistributionConfigImpl( gemFireProps ); + throw new Exception(); }catch(IllegalArgumentException e){ - if (! e.toString().contains( "Gemfire property \'ssl-enabled\' and \'cluster-ssl-enabled\' can not be used at the same time")) { + if (! e.toString().contains( "GemFire properties \'ssl-enabled\' and \'cluster-ssl-enabled\' can not be used at the same time")) { throw new Exception( "did not get expected exception, got this instead...", e ); } } - //ssl-protocol and clsuter-ssl-protocol set at the same time + //ssl-protocol and cluster-ssl-protocol set at the same time gemFireProps = new Properties(); gemFireProps.setProperty( "mcast-port", "0" ); gemFireProps.put(DistributionConfig.SSL_ENABLED_NAME, "true"); gemFireProps.put(DistributionConfig.SSL_PROTOCOLS_NAME, sslprotocols); - gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "false"); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "true"); gemFireProps.put(DistributionConfig.CLUSTER_SSL_PROTOCOLS_NAME, clusterSslprotocols); try{ config = new DistributionConfigImpl( gemFireProps ); + throw new Exception(); }catch(IllegalArgumentException e){ - if (! e.toString().contains( "Gemfire property \'ssl-protocols\' and \'cluster-ssl-protocols\' can not be used at the same time") ) { + if (! e.toString().contains( "GemFire properties \'ssl-protocols\' and \'cluster-ssl-protocols\' can not be used at the same time") ) { throw new Exception( "did not get expected exception, got this instead...", e ); } } - //ssl-cipher and clsuter-ssl-cipher set at the same time + //ssl-protocol and cluster-ssl-protocol set at the same time with same value + gemFireProps = new Properties(); + gemFireProps.setProperty( "mcast-port", "0" ); + gemFireProps.put(DistributionConfig.SSL_ENABLED_NAME, "true"); + gemFireProps.put(DistributionConfig.SSL_PROTOCOLS_NAME, sslprotocols); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "true"); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_PROTOCOLS_NAME, sslprotocols); + try{ + config = new DistributionConfigImpl( gemFireProps ); + } catch(IllegalArgumentException e){ + throw new Exception(); + } + + //ssl-cipher and cluster-ssl-cipher set at the same time gemFireProps = new Properties(); gemFireProps.setProperty( "mcast-port", "0" ); gemFireProps.put(DistributionConfig.SSL_ENABLED_NAME, "true"); gemFireProps.put(DistributionConfig.SSL_CIPHERS_NAME, sslciphers); - gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "false"); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "true"); gemFireProps.put(DistributionConfig.CLUSTER_SSL_CIPHERS_NAME, clusterSslciphers); try{ config = new DistributionConfigImpl( gemFireProps ); - }catch(IllegalArgumentException e){ - if (! e.toString().contains( "Gemfire property \'ssl-cipher\' and \'cluster-ssl-cipher\' can not be used at the same time") ) { + throw new Exception(); + } catch(IllegalArgumentException e){ + if (! e.toString().contains( "GemFire properties \'ssl-cipher\' and \'cluster-ssl-cipher\' can not be used at the same time") ) { throw new Exception( "did not get expected exception, got this instead...", e ); } } - //ssl-require-authentication and clsuter-ssl-require-authentication set at the same time + //ssl-cipher and cluster-ssl-cipher set at the same time with same value + gemFireProps = new Properties(); + gemFireProps.setProperty( "mcast-port", "0" ); + gemFireProps.put(DistributionConfig.SSL_ENABLED_NAME, "true"); + gemFireProps.put(DistributionConfig.SSL_CIPHERS_NAME, sslciphers); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "true"); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_CIPHERS_NAME, sslciphers); + try{ + config = new DistributionConfigImpl( gemFireProps ); + } catch(IllegalArgumentException e){ + throw new Exception(); + } + + //ssl-require-authentication and cluster-ssl-require-authentication set at the same time gemFireProps = new Properties(); gemFireProps.setProperty( "mcast-port", "0" ); gemFireProps.put(DistributionConfig.SSL_ENABLED_NAME, "true"); gemFireProps.put(DistributionConfig.SSL_REQUIRE_AUTHENTICATION_NAME, "true"); - gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "false"); - gemFireProps.put(DistributionConfig.CLUSTER_SSL_REQUIRE_AUTHENTICATION_NAME, "true"); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "true"); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_REQUIRE_AUTHENTICATION_NAME, "false"); try{ config = new DistributionConfigImpl( gemFireProps ); - }catch(IllegalArgumentException e){ - if (! e.toString().contains( "Gemfire property \'ssl-require-authentication\' and \'cluster-ssl-require-authentication\' can not be used at the same time") ) { + throw new Exception(); + } catch(IllegalArgumentException e){ + if (! e.toString().contains( "GemFire properties \'ssl-require-authentication\' and \'cluster-ssl-require-authentication\' can not be used at the same time") ) { throw new Exception( "did not get expected exception, got this instead...", e ); } } + //ssl-require-authentication and cluster-ssl-require-authentication set at the same time and have the same value + gemFireProps = new Properties(); + gemFireProps.setProperty( "mcast-port", "0" ); + gemFireProps.put(DistributionConfig.SSL_ENABLED_NAME, "true"); + gemFireProps.put(DistributionConfig.SSL_REQUIRE_AUTHENTICATION_NAME, "true"); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_ENABLED_NAME, "true"); + gemFireProps.put(DistributionConfig.CLUSTER_SSL_REQUIRE_AUTHENTICATION_NAME, "true"); + try{ + config = new DistributionConfigImpl( gemFireProps ); + } catch(IllegalArgumentException e){ + throw new Exception(); + } + // only ssl-* properties provided. same should reflect in cluster-ssl properties gemFireProps = new Properties(); gemFireProps.setProperty("mcast-port", "0");
