upthewaterspout commented on a change in pull request #7315:
URL: https://github.com/apache/geode/pull/7315#discussion_r805065991
##########
File path: geode-core/src/main/java/org/apache/geode/internal/net/SSLConfig.java
##########
@@ -67,26 +73,30 @@
@Immutable
private final SSLParameterExtension sslParameterExtension;
- private SSLConfig(boolean endpointIdentification,
- boolean useDefaultSSLContext,
- boolean enabled,
- String protocols,
- String ciphers,
- boolean requireAuth,
- String keystore,
- String keystoreType,
- String keystorePassword,
- String truststore,
- String truststorePassword,
- String truststoreType,
- String alias,
- SecurableCommunicationChannel securableCommunicationChannel,
- Properties properties,
- SSLParameterExtension sslParameterExtension) {
+ private SSLConfig(final boolean endpointIdentification,
+ final boolean useDefaultSSLContext,
+ final boolean enabled,
+ final @NotNull String protocols,
+ final @Nullable String clientProtocols,
+ final @Nullable String serverProtocols,
+ final String ciphers,
+ final boolean requireAuth,
+ final String keystore,
+ final String keystoreType,
+ final String keystorePassword,
+ final String truststore,
+ final String truststorePassword,
+ final String truststoreType,
+ final String alias,
+ final SecurableCommunicationChannel securableCommunicationChannel,
+ final Properties properties,
+ final SSLParameterExtension sslParameterExtension) {
this.endpointIdentification = endpointIdentification;
this.useDefaultSSLContext = useDefaultSSLContext;
this.enabled = enabled;
this.protocols = protocols;
Review comment:
Do we still need this protocols field, now that we have server and
client protocols?
##########
File path:
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
##########
@@ -2154,6 +2155,29 @@
* <U>Since</U>: Geode 1.0
*/
String SSL_PROTOCOLS = "ssl-protocols";
+
+ /**
+ * The static String definition of the <i>"ssl-client-protocols"</i>
property <a
+ * name="ssl-client-protocols"/a>
+ * </p>
+ * <U>Description</U>: A space separated list of the SSL protocols to enable
on the client side
+ * of the SSL connection. Those listed must be supported by the available
providers.
+ * </p>
+ * <U>Default</U>: "any"
+ */
+ String SSL_CLIENT_PROTOCOLS = "ssl-client-protocols";
+
+ /**
+ * The static String definition of the <i>"ssl-server-protocols"</i>
property <a
+ * name="ssl-server-protocols"/a>
+ * </p>
+ * <U>Description</U>: A space separated list of the SSL protocols to enable
on the server side
+ * of the SSL connection. Those listed must be supported by the available
providers.
+ * </p>
+ * <U>Default</U>: "any"
+ */
+ String SSL_SERVER_PROTOCOLS = "ssl-server-protocols";
Review comment:
These new properties need more documentation about how they relate and
which geode processes they apply to. For example, does ssl-server-protocols
really default to "any" or does it default to the value of ssl-protocols? What
happens if I sent all of these.
##########
File path:
geode-core/src/upgradeTest/java/org/apache/geode/internal/net/SocketCreatorUpgradeTest.java
##########
@@ -0,0 +1,429 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.internal.net;
+
+import static java.lang.Integer.parseInt;
+import static java.lang.String.format;
+import static java.lang.System.getenv;
+import static java.nio.charset.Charset.defaultCharset;
+import static org.apache.commons.io.FileUtils.readFileToString;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_CLIENT_PROTOCOLS;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_ENABLED_COMPONENTS;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_ENDPOINT_IDENTIFICATION_ENABLED;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_PASSWORD;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_TYPE;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_PROTOCOLS;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_REQUIRE_AUTHENTICATION;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_SERVER_PROTOCOLS;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_TYPE;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.GeneralSecurityException;
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+import org.apache.geode.cache.ssl.CertStores;
+import org.apache.geode.cache.ssl.CertificateBuilder;
+import org.apache.geode.cache.ssl.CertificateMaterial;
+import org.apache.geode.internal.UniquePortSupplier;
+import org.apache.geode.internal.shared.NativeCalls;
+import org.apache.geode.test.junit.rules.gfsh.GfshRule;
+import org.apache.geode.test.junit.rules.gfsh.GfshScript;
+import org.apache.geode.test.version.TestVersion;
+import org.apache.geode.test.version.VersionManager;
+
+@RunWith(Parameterized.class)
+public class SocketCreatorUpgradeTest {
+ public static final String ALGORITHM = "SHA256withRSA";
+ public static final int EXPIRATION = 1;
+ public static final String STORE_PASSWORD = "geode";
+ public static final String STORE_TYPE = "jks";
+ public static final String PROTOCOL_TLSv1_2 = "TLSv1.2";
+ public static final String PROTOCOL_TLSv1_2_SSLv2Hello =
"TLSv1.2,SSLv2Hello";
+ public static final String PROTOCOL_ANY = "any";
+ public static final String LOCATOR_1 = "locator1";
+ public static final String LOCATOR_2 = "locator2";
+
+ private final String startLocator1;
+ private final String startLocator2;
+ private final String startLocator1New;
+ private final String startLocator2New;
+ private final String stopLocator1;
+ private final String stopLocator2;
+
+ @Rule
+ public TemporaryFolder tempFolder = new TemporaryFolder();
+
+ @Rule
+ public final GfshRule gfshOldGeodeOldJava;
+
+ @Rule
+ public final GfshRule gfshOldGeodeNewJava;
+
+ @Rule
+ public final GfshRule gfshNewGeodeOldJava;
+
+ @Rule
+ public final GfshRule gfshNewGeodeNewJava;
+
+ private final File root;
+ private final File keyStoreFile;
+ private final File trustStoreFile;
+ private final File securityPropertiesFile;
+ private final File newSecurityPropertiesFile;
+
+ @Parameters(name = "{0}")
+ public static Collection<String> data() {
+ final List<String> result =
VersionManager.getInstance().getVersionsWithoutCurrent();
+ result.removeIf(s -> TestVersion.compare(s, "1.13.0") < 0);
Review comment:
Did you mean to not test upgrades from older versions of Geode?
##########
File path:
geode-core/src/upgradeTest/java/org/apache/geode/internal/net/SocketCreatorUpgradeTest.java
##########
@@ -0,0 +1,429 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.internal.net;
+
+import static java.lang.Integer.parseInt;
+import static java.lang.String.format;
+import static java.lang.System.getenv;
+import static java.nio.charset.Charset.defaultCharset;
+import static org.apache.commons.io.FileUtils.readFileToString;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_CLIENT_PROTOCOLS;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_ENABLED_COMPONENTS;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_ENDPOINT_IDENTIFICATION_ENABLED;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_PASSWORD;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_TYPE;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_PROTOCOLS;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_REQUIRE_AUTHENTICATION;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_SERVER_PROTOCOLS;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD;
+import static
org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_TYPE;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.GeneralSecurityException;
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+import org.apache.geode.cache.ssl.CertStores;
+import org.apache.geode.cache.ssl.CertificateBuilder;
+import org.apache.geode.cache.ssl.CertificateMaterial;
+import org.apache.geode.internal.UniquePortSupplier;
+import org.apache.geode.internal.shared.NativeCalls;
+import org.apache.geode.test.junit.rules.gfsh.GfshRule;
+import org.apache.geode.test.junit.rules.gfsh.GfshScript;
+import org.apache.geode.test.version.TestVersion;
+import org.apache.geode.test.version.VersionManager;
+
+@RunWith(Parameterized.class)
+public class SocketCreatorUpgradeTest {
Review comment:
Is this the right name for this test? Seems like this is maybe an
upgrade with TLS enabled test, or some such?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
