From 23ef9c3040cd7e283b92e7f5d2bde9ecfe8c7a66 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Fri, 27 Sep 2024 01:03:26 +0600 Subject: [PATCH 01/46] Add SslOptions from Lettuce library --- .../java/redis/clients/jedis/SslOptions.java | 389 ++++++++++++++++++ 1 file changed, 389 insertions(+) create mode 100644 src/main/java/redis/clients/jedis/SslOptions.java diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java new file mode 100644 index 0000000000..b1f3ef97b4 --- /dev/null +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -0,0 +1,389 @@ +/* + * Copyright 2011-Present, Redis Ltd. and Contributors + * All rights reserved. + * + * Licensed under the MIT License. + * + * This file contains contributions from third-party contributors + * licensed 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 + * + * https://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 redis.clients.jedis; + +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.util.Arrays; +import java.util.Objects; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.TrustManagerFactory; + +/** + * Options to configure SSL options for the connections kept to Redis servers. + * + * @author Mark Paluch + */ +public class SslOptions { + + private final String keyManagerAlgorithm; + + private final String trustManagerAlgorithm; + + private final String keyStoreType; + + private final String trustStoreType; + + private final Resource keystoreResource; + + private final char[] keystorePassword; + + private final Resource truststoreResource; + + private final char[] truststorePassword; + + private SslOptions(Builder builder) { + this.keyManagerAlgorithm = builder.keyManagerAlgorithm; + this.trustManagerAlgorithm = builder.trustManagerAlgorithm; + this.keyStoreType = builder.keyStoreType; + this.trustStoreType = builder.trustStoreType; + this.keystoreResource = builder.keystoreResource; + this.keystorePassword = builder.keystorePassword; + this.truststoreResource = builder.truststoreResource; + this.truststorePassword = builder.truststorePassword; + } + + /** + * Returns a new {@link SslOptions.Builder} to construct {@link SslOptions}. + * + * @return a new {@link SslOptions.Builder} to construct {@link SslOptions}. + */ + public static SslOptions.Builder builder() { + return new SslOptions.Builder(); + } + + /** + * Builder for {@link SslOptions}. + */ + public static class Builder { + + private String keyManagerAlgorithm = KeyManagerFactory.getDefaultAlgorithm(); + //private String keyManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); // Lettuce + + private String trustManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); + + private String keyStoreType; + + private String trustStoreType; + + private Resource keystoreResource; + + private char[] keystorePassword = new char[0]; + + private Resource truststoreResource; + + private char[] truststorePassword = new char[0]; + + private Builder() { + } + + public Builder keyManagerAlgorithm(String keyManagerAlgorithm) { + this.keyManagerAlgorithm = Objects.requireNonNull(keyManagerAlgorithm, "KeyManagerAlgorithm must not be null"); + return this; + } + + public Builder trustManagerAlgorithm(String trustManagerAlgorithm) { + this.trustManagerAlgorithm = Objects.requireNonNull(trustManagerAlgorithm, "TrustManagerAlgorithm must not be null"); + return this; + } + + /** + * Sets the KeyStore type. Defaults to {@link KeyStore#getDefaultType()} if not set. + * + * @param keyStoreType the keystore type to use, must not be {@code null}. + * @return {@code this} + */ + public Builder keyStoreType(String keyStoreType) { + this.keyStoreType = Objects.requireNonNull(keyStoreType, "KeyStoreType must not be null"); + return this; + } + + /** + * Sets the KeyStore type. Defaults to {@link KeyStore#getDefaultType()} if not set. + * + * @param trustStoreType the keystore type to use, must not be {@code null}. + * @return {@code this} + */ + public Builder trustStoreType(String trustStoreType) { + this.trustStoreType = Objects.requireNonNull(trustStoreType, "TrustStoreType must not be null"); + return this; + } + + /** + * Sets the Keystore file to load client certificates. The key store file must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The keystore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param keystore the keystore file, must not be {@code null}. + * @return {@code this} + */ + public Builder keystore(File keystore) { + return keystore(keystore, null); + //return keystore(keystore, new char[0]); // Lettuce + } + + /** + * Sets the Keystore file to load client certificates. The keystore file must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The keystore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param keystore the keystore file, must not be {@code null}. + * @param keystorePassword the keystore password. May be empty to omit password and the keystore integrity check. + * @return {@code this} + */ + public Builder keystore(File keystore, char[] keystorePassword) { + + Objects.requireNonNull(keystore, "Keystore must not be null"); +// LettuceAssert.isTrue(keystore.exists(), () -> String.format("Keystore file %s does not exist", truststore)); +// LettuceAssert.isTrue(keystore.isFile(), () -> String.format("Keystore %s is not a file", truststore)); + + return keystore(Resource.from(keystore), keystorePassword); + } + + /** + * Sets the Keystore resource to load client certificates. The keystore file must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The keystore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param keystore the keystore URL, must not be {@code null}. + * @return {@code this} + */ + public Builder keystore(URL keystore) { + return keystore(keystore, null); + } + + /** + * Sets the Keystore resource to load client certificates. The keystore file must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The keystore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param keystore the keystore file, must not be {@code null}. + * @param keystorePassword + * @return {@code this + */ + public Builder keystore(URL keystore, char[] keystorePassword) { + + Objects.requireNonNull(keystore, "Keystore must not be null"); + + return keystore(Resource.from(keystore), keystorePassword); + } + + /** + * Sets the Java Keystore resource to load client certificates. The keystore file must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The keystore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param resource the provider that opens a {@link InputStream} to the keystore file, must not be {@code null}. + * @param keystorePassword the keystore password. May be empty to omit password and the keystore integrity check. + * @return {@code this} + */ + public Builder keystore(Resource resource, char[] keystorePassword) { + + Objects.requireNonNull(resource, "Keystore InputStreamProvider must not be null"); + + this.keystorePassword = getPassword(keystorePassword); + this.keystoreResource = resource; + + return this; + } + + /** + * Sets the Truststore file to load trusted certificates. The truststore file must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The truststore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param truststore the truststore file, must not be {@code null}. + * @return {@code this} + */ + public Builder truststore(File truststore) { + return truststore(truststore, null); + } + + /** + * Sets the Truststore file to load trusted certificates. The truststore file must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The truststore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param truststore the truststore file, must not be {@code null}. + * @param truststorePassword the truststore password. May be empty to omit password and the truststore integrity check. + * @return {@code this} + */ + public Builder truststore(File truststore, String truststorePassword) { + + Objects.requireNonNull(truststore, "Truststore must not be null"); +// LettuceAssert.isTrue(truststore.exists(), () -> String.format("Truststore file %s does not exist", truststore)); +// LettuceAssert.isTrue(truststore.isFile(), () -> String.format("Truststore file %s is not a file", truststore)); + + return truststore(Resource.from(truststore), getPassword(truststorePassword)); + } + + /** + * Sets the Truststore resource to load trusted certificates. The truststore resource must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The truststore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param truststore the truststore file, must not be {@code null}. + * @return {@code this} + */ + public Builder truststore(URL truststore) { + return truststore(truststore, null); + } + + /** + * Sets the Truststore resource to load trusted certificates. The truststore resource must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The truststore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param truststore the truststore file, must not be {@code null}. + * @param truststorePassword the truststore password. May be empty to omit password and the truststore integrity check. + * @return {@code this} + */ + public Builder truststore(URL truststore, String truststorePassword) { + + Objects.requireNonNull(truststore, "Truststore must not be null"); + + return truststore(Resource.from(truststore), getPassword(truststorePassword)); + } + + /** + * Sets the Truststore resource to load trusted certificates. The truststore resource must be supported by + * {@link java.security.KeyStore} which is {@link KeyStore#getDefaultType()} by default. The truststore is reloaded on + * each connection attempt that allows to replace certificates during runtime. + * + * @param resource the provider that opens a {@link InputStream} to the keystore file, must not be {@code null}. + * @param truststorePassword the truststore password. May be empty to omit password and the truststore integrity check. + * @return {@code this} + */ + private Builder truststore(Resource resource, char[] truststorePassword) { + + Objects.requireNonNull(resource, "Truststore InputStreamProvider must not be null"); + + this.truststorePassword = getPassword(truststorePassword); + this.truststoreResource = resource; + + return this; + } + + /** + * Create a new instance of {@link SslOptions} + * + * @return new instance of {@link SslOptions} + */ + public SslOptions build() { + return new SslOptions(this); + } + + } + + /** + * A {@link SSLContext} object that is configured with values from this {@link SslOptions} object. + * + * @return a {@link SSLContext}. + * @throws IOException thrown when loading the keystore or the truststore fails. + * @throws GeneralSecurityException thrown when loading the keystore or the truststore fails. + */ + public SSLContext createSslContext() throws IOException, GeneralSecurityException { + + KeyStore keyStore = KeyStore.getInstance(keyStoreType); + try (InputStream keystoreStream = keystoreResource.get()) { + keyStore.load(keystoreStream, keystorePassword); + } + + KeyStore trustStore = KeyStore.getInstance(trustStoreType); + try (InputStream truststoreStream = truststoreResource.get()) { + trustStore.load(truststoreStream, truststorePassword); + } + + TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(trustManagerAlgorithm); + trustManagerFactory.init(trustStore); + + KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(keyManagerAlgorithm); + keyManagerFactory.init(keyStore, keystorePassword); + + SSLContext sslContext = SSLContext.getDefault(); + //SSLContext sslContext = SSLContext.getInstance("TLS"); // examples + + sslContext.init(keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), null); + + return sslContext; + } + + private static boolean isEmpty(String cs) { + return cs == null || cs.isEmpty(); + } + + private static char[] getPassword(String truststorePassword) { + return !isEmpty(truststorePassword) ? truststorePassword.toCharArray() : null; + } + + private static char[] getPassword(char[] chars) { + return chars != null ? Arrays.copyOf(chars, chars.length) : null; + } + + /** + * Supplier for a {@link InputStream} representing a resource. The resulting {@link InputStream} must be closed by + * the calling code. + */ + @FunctionalInterface + public interface Resource { + + /** + * Create a {@link Resource} that obtains a {@link InputStream} from a {@link URL}. + * + * @param url the URL to obtain the {@link InputStream} from. + * @return a {@link Resource} that opens a connection to the URL and obtains the {@link InputStream} for it. + */ + static Resource from(URL url) { + + Objects.requireNonNull(url, "URL must not be null"); + + return () -> url.openConnection().getInputStream(); + } + + /** + * Create a {@link Resource} that obtains a {@link InputStream} from a {@link File}. + * + * @param file the File to obtain the {@link InputStream} from. + * @return a {@link Resource} that obtains the {@link FileInputStream} for the given {@link File}. + */ + static Resource from(File file) { + + Objects.requireNonNull(file, "File must not be null"); + + return () -> new FileInputStream(file); + } + + /** + * Obtains the {@link InputStream}. + * + * @return the {@link InputStream}. + * @throws IOException + */ + InputStream get() throws IOException; + + } + +} From 375e1ca843e8e0133e789980b48eb01582ab48dc Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:34:34 +0600 Subject: [PATCH 02/46] Add InsecureTrustManagerFactory from Netty library --- .../util/InsecureTrustManagerFactory.java | 225 ++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 src/main/java/redis/clients/jedis/util/InsecureTrustManagerFactory.java diff --git a/src/main/java/redis/clients/jedis/util/InsecureTrustManagerFactory.java b/src/main/java/redis/clients/jedis/util/InsecureTrustManagerFactory.java new file mode 100644 index 0000000000..c5e4411340 --- /dev/null +++ b/src/main/java/redis/clients/jedis/util/InsecureTrustManagerFactory.java @@ -0,0 +1,225 @@ +/* + * Copyright 2014 The Netty Project + * + * The Netty Project 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: + * + * https://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 redis.clients.jedis.util; + +import java.net.Socket; +import javax.net.ssl.ManagerFactoryParameters; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; +import javax.net.ssl.TrustManagerFactorySpi; +import javax.net.ssl.X509ExtendedTrustManager; +import javax.net.ssl.X509TrustManager; +import java.security.InvalidAlgorithmParameterException; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.Provider; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; + +import java.util.Objects; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * An insecure {@link TrustManagerFactory} that trusts all X.509 certificates without any verification. + *

+ * NOTE: + * Never use this {@link TrustManagerFactory} in production. + * It is purely for testing purposes, and thus it is very insecure. + *

+ */ +public final class InsecureTrustManagerFactory extends TrustManagerFactory { + + private static final Logger logger = LoggerFactory.getLogger(InsecureTrustManagerFactory.class); + + public static final TrustManagerFactory INSTANCE = new InsecureTrustManagerFactory(); + + private static final X509Certificate[] EMPTY_X509_CERTIFICATES = {}; + + private static final Provider PROVIDER = new Provider("", 0.0, "") { + private static final long serialVersionUID = -2680540247105807895L; + }; + + /** + * {@link InsecureTrustManagerFactorySpi} must have a reference to {@link InsecureTrustManagerFactory} + * to delegate its callbacks back to {@link InsecureTrustManagerFactory}. However, it is impossible to do so, + * because {@link TrustManagerFactory} requires {@link TrustManagerFactorySpi} at construction time and + * does not provide a way to access it later. + * + * To work around this issue, we use an ugly hack which uses a {@link ThreadLocal}. + */ + private static final ThreadLocal CURRENT_SPI = + new ThreadLocal() { + @Override + protected InsecureTrustManagerFactorySpi initialValue() { + return new InsecureTrustManagerFactorySpi(); + } + }; + + private static final TrustManager tm = new X509TrustManager() { + @Override + public void checkClientTrusted(X509Certificate[] chain, String s) { + if (logger.isDebugEnabled()) { + logger.debug("Accepting a client certificate: " + chain[0].getSubjectDN()); + } + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String s) { + if (logger.isDebugEnabled()) { + logger.debug("Accepting a server certificate: " + chain[0].getSubjectDN()); + } + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return EMPTY_X509_CERTIFICATES; + } + }; + + private InsecureTrustManagerFactory() { + super(CURRENT_SPI.get(), PROVIDER, ""); + CURRENT_SPI.get().init(this); + CURRENT_SPI.remove(); + } + + /** + * Initializes this factory with a source of certificate authorities and related trust material. + * + * @see TrustManagerFactorySpi#engineInit(KeyStore) + */ + protected void engineInit(KeyStore keyStore) throws Exception { } + + /** + * Initializes this factory with a source of provider-specific key material. + * + * @see TrustManagerFactorySpi#engineInit(ManagerFactoryParameters) + */ + protected void engineInit(ManagerFactoryParameters managerFactoryParameters) throws Exception { } + + /** + * Returns one trust manager for each type of trust material. + * + * @see TrustManagerFactorySpi#engineGetTrustManagers() + */ + protected TrustManager[] engineGetTrustManagers() { + return new TrustManager[] { tm }; + } + + static final class InsecureTrustManagerFactorySpi extends TrustManagerFactorySpi { + + private InsecureTrustManagerFactory parent; + private volatile TrustManager[] trustManagers; + + void init(InsecureTrustManagerFactory parent) { + this.parent = parent; + } + + @Override + protected void engineInit(KeyStore keyStore) throws KeyStoreException { + try { + parent.engineInit(keyStore); + } catch (KeyStoreException e) { + throw e; + } catch (Exception e) { + throw new KeyStoreException(e); + } + } + + @Override + protected void engineInit(ManagerFactoryParameters managerFactoryParameters) + throws InvalidAlgorithmParameterException { + try { + parent.engineInit(managerFactoryParameters); + } catch (InvalidAlgorithmParameterException e) { + throw e; + } catch (Exception e) { + throw new InvalidAlgorithmParameterException(e); + } + } + + @Override + protected TrustManager[] engineGetTrustManagers() { + TrustManager[] trustManagers = this.trustManagers; + if (trustManagers == null) { + trustManagers = parent.engineGetTrustManagers(); + wrapForJava7Plus(trustManagers); + this.trustManagers = trustManagers; + } + return trustManagers.clone(); + } + + private static void wrapForJava7Plus(TrustManager[] trustManagers) { + for (int i = 0; i < trustManagers.length; i++) { + final TrustManager tm = trustManagers[i]; + if (tm instanceof X509TrustManager && !(tm instanceof X509ExtendedTrustManager)) { + trustManagers[i] = new X509TrustManagerWrapper((X509TrustManager) tm); + } + } + } + } + + static final class X509TrustManagerWrapper extends X509ExtendedTrustManager { + + private final X509TrustManager delegate; + + X509TrustManagerWrapper(X509TrustManager delegate) { + this.delegate = Objects.requireNonNull(delegate, "delegate"); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String s) throws CertificateException { + delegate.checkClientTrusted(chain, s); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String s, Socket socket) + throws CertificateException { + delegate.checkClientTrusted(chain, s); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String s, SSLEngine sslEngine) + throws CertificateException { + delegate.checkClientTrusted(chain, s); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String s) throws CertificateException { + delegate.checkServerTrusted(chain, s); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String s, Socket socket) + throws CertificateException { + delegate.checkServerTrusted(chain, s); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String s, SSLEngine sslEngine) + throws CertificateException { + delegate.checkServerTrusted(chain, s); + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return delegate.getAcceptedIssuers(); + } + } + +} From ed26f37541068db874565dc81810a6b5bf583bde Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 29 Sep 2024 18:58:31 +0600 Subject: [PATCH 03/46] Separate SSL socket creation in DefaultJedisSocketFactory --- .../jedis/DefaultJedisSocketFactory.java | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java index 0d41693d0f..32d6fbf9c0 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java @@ -1,5 +1,6 @@ package redis.clients.jedis; +import java.io.IOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Socket; @@ -89,6 +90,24 @@ public Socket createSocket() throws JedisConnectionException { socket = connectToFirstSuccessfulHost(_hostAndPort); socket.setSoTimeout(socketTimeout); + if (ssl) { + socket = createSslSocket(_hostAndPort, socket); + } + + return socket; + + } catch (Exception ex) { + IOUtils.closeQuietly(socket); + if (ex instanceof JedisConnectionException) { + throw (JedisConnectionException) ex; + } else { + throw new JedisConnectionException("Failed to create socket.", ex); + } + } + } + + private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws IOException { + if (ssl) { SSLSocketFactory _sslSocketFactory = this.sslSocketFactory; if (null == _sslSocketFactory) { @@ -111,15 +130,6 @@ public Socket createSocket() throws JedisConnectionException { } return socket; - - } catch (Exception ex) { - IOUtils.closeQuietly(socket); - if (ex instanceof JedisConnectionException) { - throw (JedisConnectionException) ex; - } else { - throw new JedisConnectionException("Failed to create socket.", ex); - } - } } public void updateHostAndPort(HostAndPort hostAndPort) { From 382fd8123f19bc7ae0888912a57598fd94b31295 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 29 Sep 2024 19:13:52 +0600 Subject: [PATCH 04/46] Add SslOptions in JedisClientConfig --- .../jedis/DefaultJedisClientConfig.java | 40 ++++++++++++-- .../jedis/DefaultJedisSocketFactory.java | 55 +++++++++++-------- .../clients/jedis/JedisClientConfig.java | 9 +++ 3 files changed, 76 insertions(+), 28 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index f26513fa58..95c127f1b4 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -20,6 +20,7 @@ public final class DefaultJedisClientConfig implements JedisClientConfig { private final boolean ssl; private final SSLSocketFactory sslSocketFactory; private final SSLParameters sslParameters; + private final SslOptions sslOptions; private final HostnameVerifier hostnameVerifier; private final HostAndPortMapper hostAndPortMapper; @@ -28,6 +29,7 @@ public final class DefaultJedisClientConfig implements JedisClientConfig { private final boolean readOnlyForRedisClusterReplicas; + // TODO: private DefaultJedisClientConfig(RedisProtocol protocol, int connectionTimeoutMillis, int soTimeoutMillis, int blockingSocketTimeoutMillis, Supplier credentialsProvider, int database, String clientName, boolean ssl, SSLSocketFactory sslSocketFactory, SSLParameters sslParameters, @@ -43,12 +45,31 @@ private DefaultJedisClientConfig(RedisProtocol protocol, int connectionTimeoutMi this.ssl = ssl; this.sslSocketFactory = sslSocketFactory; this.sslParameters = sslParameters; + this.sslOptions = null; this.hostnameVerifier = hostnameVerifier; this.hostAndPortMapper = hostAndPortMapper; this.clientSetInfoConfig = clientSetInfoConfig; this.readOnlyForRedisClusterReplicas = readOnlyForRedisClusterReplicas; } + private DefaultJedisClientConfig(DefaultJedisClientConfig.Builder builder) { + this.redisProtocol = builder.redisProtocol; + this.connectionTimeoutMillis = builder.connectionTimeoutMillis; + this.socketTimeoutMillis = builder.socketTimeoutMillis; + this.blockingSocketTimeoutMillis = builder.blockingSocketTimeoutMillis; + this.credentialsProvider = builder.credentialsProvider; + this.database = builder.database; + this.clientName = builder.clientName; + this.ssl = builder.ssl; + this.sslSocketFactory = builder.sslSocketFactory; + this.sslParameters = builder.sslParameters; + this.sslOptions = builder.sslOptions; + this.hostnameVerifier = builder.hostnameVerifier; + this.hostAndPortMapper = builder.hostAndPortMapper; + this.clientSetInfoConfig = builder.clientSetInfoConfig; + this.readOnlyForRedisClusterReplicas = builder.readOnlyForRedisClusterReplicas; + } + @Override public RedisProtocol getRedisProtocol() { return redisProtocol; @@ -110,6 +131,11 @@ public SSLParameters getSslParameters() { return sslParameters; } + @Override + public SslOptions getSslOptions() { + return sslOptions; + } + @Override public HostnameVerifier getHostnameVerifier() { return hostnameVerifier; @@ -151,6 +177,7 @@ public static class Builder { private boolean ssl = false; private SSLSocketFactory sslSocketFactory = null; private SSLParameters sslParameters = null; + private SslOptions sslOptions = null; private HostnameVerifier hostnameVerifier = null; private HostAndPortMapper hostAndPortMapper = null; @@ -168,14 +195,12 @@ public DefaultJedisClientConfig build() { new DefaultRedisCredentials(user, password)); } - return new DefaultJedisClientConfig(redisProtocol, connectionTimeoutMillis, socketTimeoutMillis, - blockingSocketTimeoutMillis, credentialsProvider, database, clientName, ssl, - sslSocketFactory, sslParameters, hostnameVerifier, hostAndPortMapper, clientSetInfoConfig, - readOnlyForRedisClusterReplicas); + return new DefaultJedisClientConfig(this); } /** * Shortcut to {@link Builder#protocol(redis.clients.jedis.RedisProtocol)} with {@link RedisProtocol#RESP3}. + * @return this */ public Builder resp3() { return protocol(RedisProtocol.RESP3); @@ -252,6 +277,11 @@ public Builder sslParameters(SSLParameters sslParameters) { return this; } + public Builder sslOptions(SslOptions sslOptions) { + this.sslOptions = sslOptions; + return this; + } + public Builder hostnameVerifier(HostnameVerifier hostnameVerifier) { this.hostnameVerifier = hostnameVerifier; return this; @@ -273,6 +303,7 @@ public Builder readOnlyForRedisClusterReplicas() { } } + // TOOD: public static DefaultJedisClientConfig create(int connectionTimeoutMillis, int soTimeoutMillis, int blockingSocketTimeoutMillis, String user, String password, int database, String clientName, boolean ssl, SSLSocketFactory sslSocketFactory, SSLParameters sslParameters, @@ -284,6 +315,7 @@ public static DefaultJedisClientConfig create(int connectionTimeoutMillis, int s false); } + // TODO: public static DefaultJedisClientConfig copyConfig(JedisClientConfig copy) { return new DefaultJedisClientConfig(copy.getRedisProtocol(), copy.getConnectionTimeoutMillis(), copy.getSocketTimeoutMillis(), diff --git a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java index 32d6fbf9c0..7ff1d0a6dd 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java @@ -4,6 +4,7 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Socket; +import java.security.GeneralSecurityException; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -25,6 +26,7 @@ public class DefaultJedisSocketFactory implements JedisSocketFactory { private int socketTimeout = Protocol.DEFAULT_TIMEOUT; private boolean ssl = false; private SSLSocketFactory sslSocketFactory = null; + private SslOptions sslOptions = null; private SSLParameters sslParameters = null; private HostnameVerifier hostnameVerifier = null; private HostAndPortMapper hostAndPortMapper = null; @@ -50,6 +52,7 @@ public DefaultJedisSocketFactory(HostAndPort hostAndPort, JedisClientConfig conf this.ssl = config.isSsl(); this.sslSocketFactory = config.getSslSocketFactory(); this.sslParameters = config.getSslParameters(); + this.sslOptions = config.getSslOptions(); this.hostnameVerifier = config.getHostnameVerifier(); this.hostAndPortMapper = config.getHostAndPortMapper(); } @@ -90,7 +93,7 @@ public Socket createSocket() throws JedisConnectionException { socket = connectToFirstSuccessfulHost(_hostAndPort); socket.setSoTimeout(socketTimeout); - if (ssl) { + if (ssl || sslOptions != null) { socket = createSslSocket(_hostAndPort, socket); } @@ -106,30 +109,34 @@ public Socket createSocket() throws JedisConnectionException { } } - private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws IOException { - - if (ssl) { - SSLSocketFactory _sslSocketFactory = this.sslSocketFactory; - if (null == _sslSocketFactory) { - _sslSocketFactory = (SSLSocketFactory) SSLSocketFactory.getDefault(); - } - Socket plainSocket = socket; - socket = _sslSocketFactory.createSocket(socket, _hostAndPort.getHost(), _hostAndPort.getPort(), true); - - if (null != sslParameters) { - ((SSLSocket) socket).setSSLParameters(sslParameters); - } - socket = new SSLSocketWrapper((SSLSocket) socket, plainSocket); - - if (null != hostnameVerifier - && !hostnameVerifier.verify(_hostAndPort.getHost(), ((SSLSocket) socket).getSession())) { - String message = String.format( - "The connection to '%s' failed ssl/tls hostname verification.", _hostAndPort.getHost()); - throw new JedisConnectionException(message); - } - } + private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws IOException, GeneralSecurityException { - return socket; + Socket plainSocket = socket; + + SSLSocketFactory _sslSocketFactory; + if (sslOptions != null) { + _sslSocketFactory = sslOptions.createSslContext().getSocketFactory(); + } else { + _sslSocketFactory = this.sslSocketFactory; + } + if (null == _sslSocketFactory) { + _sslSocketFactory = (SSLSocketFactory) SSLSocketFactory.getDefault(); + } + + SSLSocket sslSocket = (SSLSocket) _sslSocketFactory.createSocket(socket, _hostAndPort.getHost(), _hostAndPort.getPort(), true); + + if (null != sslParameters) { + sslSocket.setSSLParameters(sslParameters); + } + + if (null != hostnameVerifier + && !hostnameVerifier.verify(_hostAndPort.getHost(), sslSocket.getSession())) { + String message = String.format( + "The connection to '%s' failed ssl/tls hostname verification.", _hostAndPort.getHost()); + throw new JedisConnectionException(message); + } + + return new SSLSocketWrapper(sslSocket, plainSocket); } public void updateHostAndPort(HostAndPort hostAndPort) { diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index abe1f35237..4b65669124 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -72,6 +72,15 @@ default SSLParameters getSslParameters() { return null; } + /** + * {@link JedisClientConfig#isSsl()} and {@link JedisClientConfig#getSslSocketFactory()} will be ignored if + * {@link JedisClientConfig#getSslOptions() this} is set. + * @return ssl options + */ + default SslOptions getSslOptions() { + return null; + } + default HostnameVerifier getHostnameVerifier() { return null; } From 363d493f2b94fd08c13c239d4d5768693b3c8193 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 30 Sep 2024 16:58:13 +0600 Subject: [PATCH 05/46] Add SslHostnameVerifyMode inspired by SslVerifyMode in Lettuce --- .../jedis/DefaultJedisClientConfig.java | 16 ++++++- .../jedis/DefaultJedisSocketFactory.java | 42 +++++++++++++++---- .../clients/jedis/JedisClientConfig.java | 4 ++ .../clients/jedis/SslHostnameVerifyMode.java | 8 ++++ .../java/redis/clients/jedis/SslOptions.java | 2 + 5 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index 95c127f1b4..1da368cbee 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -21,6 +21,7 @@ public final class DefaultJedisClientConfig implements JedisClientConfig { private final SSLSocketFactory sslSocketFactory; private final SSLParameters sslParameters; private final SslOptions sslOptions; + private final SslHostnameVerifyMode sslHostnameVerifyMode; private final HostnameVerifier hostnameVerifier; private final HostAndPortMapper hostAndPortMapper; @@ -45,7 +46,8 @@ private DefaultJedisClientConfig(RedisProtocol protocol, int connectionTimeoutMi this.ssl = ssl; this.sslSocketFactory = sslSocketFactory; this.sslParameters = sslParameters; - this.sslOptions = null; + this.sslOptions = null; // TODO: + this.sslHostnameVerifyMode = null; // TODO: this.hostnameVerifier = hostnameVerifier; this.hostAndPortMapper = hostAndPortMapper; this.clientSetInfoConfig = clientSetInfoConfig; @@ -64,6 +66,7 @@ private DefaultJedisClientConfig(DefaultJedisClientConfig.Builder builder) { this.sslSocketFactory = builder.sslSocketFactory; this.sslParameters = builder.sslParameters; this.sslOptions = builder.sslOptions; + this.sslHostnameVerifyMode = builder.sslHostnameVerifyMode; this.hostnameVerifier = builder.hostnameVerifier; this.hostAndPortMapper = builder.hostAndPortMapper; this.clientSetInfoConfig = builder.clientSetInfoConfig; @@ -136,6 +139,11 @@ public SslOptions getSslOptions() { return sslOptions; } + @Override + public SslHostnameVerifyMode getSslHostnameVerifyMode() { + return sslHostnameVerifyMode; + } + @Override public HostnameVerifier getHostnameVerifier() { return hostnameVerifier; @@ -178,6 +186,7 @@ public static class Builder { private SSLSocketFactory sslSocketFactory = null; private SSLParameters sslParameters = null; private SslOptions sslOptions = null; + private SslHostnameVerifyMode sslHostnameVerifyMode = null; private HostnameVerifier hostnameVerifier = null; private HostAndPortMapper hostAndPortMapper = null; @@ -282,6 +291,11 @@ public Builder sslOptions(SslOptions sslOptions) { return this; } + public Builder sslHostnameVerifyMode(SslHostnameVerifyMode sslHostnameVerifyMode) { + this.sslHostnameVerifyMode = sslHostnameVerifyMode; + return this; + } + public Builder hostnameVerifier(HostnameVerifier hostnameVerifier) { this.hostnameVerifier = hostnameVerifier; return this; diff --git a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java index 7ff1d0a6dd..c9ebad1756 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java @@ -9,6 +9,7 @@ import java.util.Collections; import java.util.List; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLContext; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; @@ -27,6 +28,7 @@ public class DefaultJedisSocketFactory implements JedisSocketFactory { private boolean ssl = false; private SSLSocketFactory sslSocketFactory = null; private SslOptions sslOptions = null; + private SslHostnameVerifyMode sslHostnameVerifyMode = null; private SSLParameters sslParameters = null; private HostnameVerifier hostnameVerifier = null; private HostAndPortMapper hostAndPortMapper = null; @@ -53,6 +55,7 @@ public DefaultJedisSocketFactory(HostAndPort hostAndPort, JedisClientConfig conf this.sslSocketFactory = config.getSslSocketFactory(); this.sslParameters = config.getSslParameters(); this.sslOptions = config.getSslOptions(); + this.sslHostnameVerifyMode = config.getSslHostnameVerifyMode(); this.hostnameVerifier = config.getHostnameVerifier(); this.hostAndPortMapper = config.getHostAndPortMapper(); } @@ -109,30 +112,51 @@ public Socket createSocket() throws JedisConnectionException { } } + /** + * ssl enable check is done before this. + */ private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws IOException, GeneralSecurityException { Socket plainSocket = socket; SSLSocketFactory _sslSocketFactory; + SSLParameters _sslParameters; + if (sslOptions != null) { - _sslSocketFactory = sslOptions.createSslContext().getSocketFactory(); + + SSLContext _sslContext = sslOptions.createSslContext(); + + _sslParameters = sslParameters != null ? sslParameters : new SSLParameters(); // TODO: + + if (sslHostnameVerifyMode == SslHostnameVerifyMode.HTTPS) { + _sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + } else if (sslHostnameVerifyMode == SslHostnameVerifyMode.DISABLE) { + _sslParameters.setEndpointIdentificationAlgorithm(""); + } + + _sslSocketFactory = _sslContext.getSocketFactory(); + } else { + _sslSocketFactory = this.sslSocketFactory; + _sslParameters = this.sslParameters; + } - if (null == _sslSocketFactory) { + + if (_sslSocketFactory == null) { _sslSocketFactory = (SSLSocketFactory) SSLSocketFactory.getDefault(); } - SSLSocket sslSocket = (SSLSocket) _sslSocketFactory.createSocket(socket, _hostAndPort.getHost(), _hostAndPort.getPort(), true); + SSLSocket sslSocket = (SSLSocket) _sslSocketFactory.createSocket(socket, + _hostAndPort.getHost(), _hostAndPort.getPort(), true); - if (null != sslParameters) { - sslSocket.setSSLParameters(sslParameters); + if (_sslParameters != null) { + sslSocket.setSSLParameters(_sslParameters); } - if (null != hostnameVerifier - && !hostnameVerifier.verify(_hostAndPort.getHost(), sslSocket.getSession())) { - String message = String.format( - "The connection to '%s' failed ssl/tls hostname verification.", _hostAndPort.getHost()); + if (hostnameVerifier != null && !hostnameVerifier.verify(_hostAndPort.getHost(), sslSocket.getSession())) { + String message = String.format("The connection to '%s' failed ssl/tls hostname verification.", + _hostAndPort.getHost()); throw new JedisConnectionException(message); } diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index 4b65669124..1c7c615905 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -81,6 +81,10 @@ default SslOptions getSslOptions() { return null; } + default SslHostnameVerifyMode getSslHostnameVerifyMode() { + return null; // TODO: SslVerifyMode.HTTPS + } + default HostnameVerifier getHostnameVerifier() { return null; } diff --git a/src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java b/src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java new file mode 100644 index 0000000000..284e42b350 --- /dev/null +++ b/src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java @@ -0,0 +1,8 @@ +package redis.clients.jedis; + +public enum SslHostnameVerifyMode { + + HTTPS, + + DISABLE; +} diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index b1f3ef97b4..2b90c01eef 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -327,6 +327,8 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio //SSLContext sslContext = SSLContext.getInstance("TLS"); // examples sslContext.init(keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), null); + // TODO: set null for key managers and/or trust managers + // TODO: null doesn't completely disables these... return sslContext; } From 261bf3067122b437ef3a8b024cd9ab16203f7bef Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 30 Sep 2024 17:18:23 +0600 Subject: [PATCH 06/46] Allow system default key and trust managers and insecure trust manager --- .../java/redis/clients/jedis/SslOptions.java | 70 +++++++++++++++---- 1 file changed, 56 insertions(+), 14 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index 2b90c01eef..c59cb8b348 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -31,6 +31,7 @@ import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.TrustManagerFactory; +import redis.clients.jedis.util.InsecureTrustManagerFactory; /** * Options to configure SSL options for the connections kept to Redis servers. @@ -55,6 +56,8 @@ public class SslOptions { private final char[] truststorePassword; + private final boolean insecureTrust; + private SslOptions(Builder builder) { this.keyManagerAlgorithm = builder.keyManagerAlgorithm; this.trustManagerAlgorithm = builder.trustManagerAlgorithm; @@ -64,6 +67,7 @@ private SslOptions(Builder builder) { this.keystorePassword = builder.keystorePassword; this.truststoreResource = builder.truststoreResource; this.truststorePassword = builder.truststorePassword; + this.insecureTrust = builder.insecureTrust; } /** @@ -97,6 +101,8 @@ public static class Builder { private char[] truststorePassword = new char[0]; + private boolean insecureTrust = false; + private Builder() { } @@ -105,11 +111,21 @@ public Builder keyManagerAlgorithm(String keyManagerAlgorithm) { return this; } + public Builder systemDefaultKeyManager() { + this.keyManagerAlgorithm = null; + return this; + } + public Builder trustManagerAlgorithm(String trustManagerAlgorithm) { this.trustManagerAlgorithm = Objects.requireNonNull(trustManagerAlgorithm, "TrustManagerAlgorithm must not be null"); return this; } + public Builder systemDefaultTrustManager() { + this.trustManagerAlgorithm = null; + return this; + } + /** * Sets the KeyStore type. Defaults to {@link KeyStore#getDefaultType()} if not set. * @@ -287,6 +303,11 @@ private Builder truststore(Resource resource, char[] truststorePassword) { return this; } + public Builder insecureTrust(boolean insecureTrust) { + this.insecureTrust = insecureTrust; + return this; + } + /** * Create a new instance of {@link SslOptions} * @@ -307,28 +328,49 @@ public SslOptions build() { */ public SSLContext createSslContext() throws IOException, GeneralSecurityException { - KeyStore keyStore = KeyStore.getInstance(keyStoreType); - try (InputStream keystoreStream = keystoreResource.get()) { - keyStore.load(keystoreStream, keystorePassword); - } + final KeyManagerFactory keyManagerFactory; + if (keyManagerAlgorithm == null) { + keyManagerFactory = null; + } else { + + KeyStore keyStore = KeyStore.getInstance(keyStoreType); + try (InputStream keystoreStream = keystoreResource.get()) { + keyStore.load(keystoreStream, keystorePassword); + } + + keyManagerFactory = KeyManagerFactory.getInstance(keyManagerAlgorithm); + keyManagerFactory.init(keyStore, keystorePassword); - KeyStore trustStore = KeyStore.getInstance(trustStoreType); - try (InputStream truststoreStream = truststoreResource.get()) { - trustStore.load(truststoreStream, truststorePassword); } - TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(trustManagerAlgorithm); - trustManagerFactory.init(trustStore); + final TrustManagerFactory trustManagerFactory; + if (insecureTrust) { + + trustManagerFactory = InsecureTrustManagerFactory.INSTANCE; + + } else if (trustManagerAlgorithm == null) { - KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(keyManagerAlgorithm); - keyManagerFactory.init(keyStore, keystorePassword); + trustManagerFactory = null; + + } else { + + KeyStore trustStore = KeyStore.getInstance(trustStoreType); + try (InputStream truststoreStream = truststoreResource.get()) { + trustStore.load(truststoreStream, truststorePassword); + } + + trustManagerFactory = TrustManagerFactory.getInstance(trustManagerAlgorithm); + trustManagerFactory.init(trustStore); + + } SSLContext sslContext = SSLContext.getDefault(); //SSLContext sslContext = SSLContext.getInstance("TLS"); // examples - sslContext.init(keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), null); - // TODO: set null for key managers and/or trust managers - // TODO: null doesn't completely disables these... + sslContext.init( + keyManagerFactory == null ? null : keyManagerFactory.getKeyManagers(), + trustManagerFactory == null ? null : trustManagerFactory.getTrustManagers(), + null); return sslContext; } From 2ca615ee1637642c2ead9d93fb673aa194a11725 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 30 Sep 2024 17:26:50 +0600 Subject: [PATCH 07/46] fix jdoc --- src/main/java/redis/clients/jedis/SslOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index c59cb8b348..ee4135aacb 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -198,7 +198,7 @@ public Builder keystore(URL keystore) { * * @param keystore the keystore file, must not be {@code null}. * @param keystorePassword - * @return {@code this + * @return {@code this} */ public Builder keystore(URL keystore, char[] keystorePassword) { From 8da45c331de7f6620c4c425d1e30a49ba281e2c2 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 1 Oct 2024 14:59:59 +0600 Subject: [PATCH 08/46] Address review comments --- .../java/redis/clients/jedis/DefaultJedisClientConfig.java | 6 +++--- .../java/redis/clients/jedis/DefaultJedisSocketFactory.java | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index 1da368cbee..bed8bc3966 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -30,7 +30,7 @@ public final class DefaultJedisClientConfig implements JedisClientConfig { private final boolean readOnlyForRedisClusterReplicas; - // TODO: + // TODO: how many constructors should we have? 1) all params, 2) builder 3) another config private DefaultJedisClientConfig(RedisProtocol protocol, int connectionTimeoutMillis, int soTimeoutMillis, int blockingSocketTimeoutMillis, Supplier credentialsProvider, int database, String clientName, boolean ssl, SSLSocketFactory sslSocketFactory, SSLParameters sslParameters, @@ -317,7 +317,7 @@ public Builder readOnlyForRedisClusterReplicas() { } } - // TOOD: + // TOOD: depends on which constructors we're going to have/keep public static DefaultJedisClientConfig create(int connectionTimeoutMillis, int soTimeoutMillis, int blockingSocketTimeoutMillis, String user, String password, int database, String clientName, boolean ssl, SSLSocketFactory sslSocketFactory, SSLParameters sslParameters, @@ -329,7 +329,7 @@ public static DefaultJedisClientConfig create(int connectionTimeoutMillis, int s false); } - // TODO: + // TOOD: depends on which constructors we're going to have/keep public static DefaultJedisClientConfig copyConfig(JedisClientConfig copy) { return new DefaultJedisClientConfig(copy.getRedisProtocol(), copy.getConnectionTimeoutMillis(), copy.getSocketTimeoutMillis(), diff --git a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java index c9ebad1756..15e40b4ff2 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java @@ -124,8 +124,6 @@ private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws I if (sslOptions != null) { - SSLContext _sslContext = sslOptions.createSslContext(); - _sslParameters = sslParameters != null ? sslParameters : new SSLParameters(); // TODO: if (sslHostnameVerifyMode == SslHostnameVerifyMode.HTTPS) { @@ -134,6 +132,8 @@ private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws I _sslParameters.setEndpointIdentificationAlgorithm(""); } + SSLContext _sslContext = sslOptions.createSslContext(); + _sslSocketFactory = _sslContext.getSocketFactory(); } else { From e9444d91948edf8374195f2eaf36d14e4d6c69af Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 10 Oct 2024 15:57:48 +0600 Subject: [PATCH 09/46] Revert back to verification mode names in Lettuce, to avoid confusion at this moment. --- .../redis/clients/jedis/DefaultJedisSocketFactory.java | 4 ++-- .../java/redis/clients/jedis/SslHostnameVerifyMode.java | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java index 15e40b4ff2..3cd49eef7d 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java @@ -126,9 +126,9 @@ private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws I _sslParameters = sslParameters != null ? sslParameters : new SSLParameters(); // TODO: - if (sslHostnameVerifyMode == SslHostnameVerifyMode.HTTPS) { + if (sslHostnameVerifyMode == SslHostnameVerifyMode.FULL) { _sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); - } else if (sslHostnameVerifyMode == SslHostnameVerifyMode.DISABLE) { + } else if (sslHostnameVerifyMode == SslHostnameVerifyMode.CA) { _sslParameters.setEndpointIdentificationAlgorithm(""); } diff --git a/src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java b/src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java index 284e42b350..4b39b947ae 100644 --- a/src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java +++ b/src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java @@ -1,8 +1,11 @@ package redis.clients.jedis; +/** + * Enumeration of SSL/TLS hostname verification modes. + */ public enum SslHostnameVerifyMode { - HTTPS, + FULL, - DISABLE; + CA; } From ae97c006d1edee1deacf6d41755696d77afc6a78 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 10 Oct 2024 16:05:31 +0600 Subject: [PATCH 10/46] Rename insecureTrust to noTruststoreVerification; does this reduce confusion? --- src/main/java/redis/clients/jedis/SslOptions.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index ee4135aacb..84879ccd68 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -56,7 +56,7 @@ public class SslOptions { private final char[] truststorePassword; - private final boolean insecureTrust; + private final boolean noTruststoreVerification; private SslOptions(Builder builder) { this.keyManagerAlgorithm = builder.keyManagerAlgorithm; @@ -67,7 +67,7 @@ private SslOptions(Builder builder) { this.keystorePassword = builder.keystorePassword; this.truststoreResource = builder.truststoreResource; this.truststorePassword = builder.truststorePassword; - this.insecureTrust = builder.insecureTrust; + this.noTruststoreVerification = builder.noTruststoreVerification; } /** @@ -101,7 +101,7 @@ public static class Builder { private char[] truststorePassword = new char[0]; - private boolean insecureTrust = false; + private boolean noTruststoreVerification = false; private Builder() { } @@ -303,8 +303,8 @@ private Builder truststore(Resource resource, char[] truststorePassword) { return this; } - public Builder insecureTrust(boolean insecureTrust) { - this.insecureTrust = insecureTrust; + public Builder noTruststoreVerification(boolean noTruststoreVerification) { + this.noTruststoreVerification = noTruststoreVerification; return this; } @@ -344,7 +344,7 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio } final TrustManagerFactory trustManagerFactory; - if (insecureTrust) { + if (noTruststoreVerification) { trustManagerFactory = InsecureTrustManagerFactory.INSTANCE; From 09fee5e8e2a25617be97b09dc173d1f47e6d6b32 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 10 Nov 2024 22:30:05 +0600 Subject: [PATCH 11/46] SslVerifyMode is renamed back and INSECURE option (renamed from NONE) is added back in SslVerifyMode. --- .../jedis/DefaultJedisClientConfig.java | 14 ---- .../jedis/DefaultJedisSocketFactory.java | 14 +--- .../clients/jedis/JedisClientConfig.java | 4 -- .../java/redis/clients/jedis/SslOptions.java | 67 ++++++++++++------- ...nameVerifyMode.java => SslVerifyMode.java} | 8 ++- 5 files changed, 51 insertions(+), 56 deletions(-) rename src/main/java/redis/clients/jedis/{SslHostnameVerifyMode.java => SslVerifyMode.java} (61%) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index f6d3156d74..b4ab76064b 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -21,7 +21,6 @@ public final class DefaultJedisClientConfig implements JedisClientConfig { private final SSLSocketFactory sslSocketFactory; private final SSLParameters sslParameters; private final SslOptions sslOptions; - private final SslHostnameVerifyMode sslHostnameVerifyMode; private final HostnameVerifier hostnameVerifier; private final HostAndPortMapper hostAndPortMapper; @@ -47,7 +46,6 @@ private DefaultJedisClientConfig(RedisProtocol protocol, int connectionTimeoutMi this.sslSocketFactory = sslSocketFactory; this.sslParameters = sslParameters; this.sslOptions = null; // TODO: - this.sslHostnameVerifyMode = null; // TODO: this.hostnameVerifier = hostnameVerifier; this.hostAndPortMapper = hostAndPortMapper; this.clientSetInfoConfig = clientSetInfoConfig; @@ -66,7 +64,6 @@ private DefaultJedisClientConfig(DefaultJedisClientConfig.Builder builder) { this.sslSocketFactory = builder.sslSocketFactory; this.sslParameters = builder.sslParameters; this.sslOptions = builder.sslOptions; - this.sslHostnameVerifyMode = builder.sslHostnameVerifyMode; this.hostnameVerifier = builder.hostnameVerifier; this.hostAndPortMapper = builder.hostAndPortMapper; this.clientSetInfoConfig = builder.clientSetInfoConfig; @@ -139,11 +136,6 @@ public SslOptions getSslOptions() { return sslOptions; } - @Override - public SslHostnameVerifyMode getSslHostnameVerifyMode() { - return sslHostnameVerifyMode; - } - @Override public HostnameVerifier getHostnameVerifier() { return hostnameVerifier; @@ -186,7 +178,6 @@ public static class Builder { private SSLSocketFactory sslSocketFactory = null; private SSLParameters sslParameters = null; private SslOptions sslOptions = null; - private SslHostnameVerifyMode sslHostnameVerifyMode = null; private HostnameVerifier hostnameVerifier = null; private HostAndPortMapper hostAndPortMapper = null; @@ -292,11 +283,6 @@ public Builder sslOptions(SslOptions sslOptions) { return this; } - public Builder sslHostnameVerifyMode(SslHostnameVerifyMode sslHostnameVerifyMode) { - this.sslHostnameVerifyMode = sslHostnameVerifyMode; - return this; - } - public Builder hostnameVerifier(HostnameVerifier hostnameVerifier) { this.hostnameVerifier = hostnameVerifier; return this; diff --git a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java index 3cd49eef7d..564e409b98 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java @@ -28,7 +28,6 @@ public class DefaultJedisSocketFactory implements JedisSocketFactory { private boolean ssl = false; private SSLSocketFactory sslSocketFactory = null; private SslOptions sslOptions = null; - private SslHostnameVerifyMode sslHostnameVerifyMode = null; private SSLParameters sslParameters = null; private HostnameVerifier hostnameVerifier = null; private HostAndPortMapper hostAndPortMapper = null; @@ -55,7 +54,6 @@ public DefaultJedisSocketFactory(HostAndPort hostAndPort, JedisClientConfig conf this.sslSocketFactory = config.getSslSocketFactory(); this.sslParameters = config.getSslParameters(); this.sslOptions = config.getSslOptions(); - this.sslHostnameVerifyMode = config.getSslHostnameVerifyMode(); this.hostnameVerifier = config.getHostnameVerifier(); this.hostAndPortMapper = config.getHostAndPortMapper(); } @@ -124,23 +122,15 @@ private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws I if (sslOptions != null) { - _sslParameters = sslParameters != null ? sslParameters : new SSLParameters(); // TODO: - - if (sslHostnameVerifyMode == SslHostnameVerifyMode.FULL) { - _sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); - } else if (sslHostnameVerifyMode == SslHostnameVerifyMode.CA) { - _sslParameters.setEndpointIdentificationAlgorithm(""); - } - SSLContext _sslContext = sslOptions.createSslContext(); - _sslSocketFactory = _sslContext.getSocketFactory(); + _sslParameters = sslOptions.getSslParameters(); + } else { _sslSocketFactory = this.sslSocketFactory; _sslParameters = this.sslParameters; - } if (_sslSocketFactory == null) { diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index 1c7c615905..4b65669124 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -81,10 +81,6 @@ default SslOptions getSslOptions() { return null; } - default SslHostnameVerifyMode getSslHostnameVerifyMode() { - return null; // TODO: SslVerifyMode.HTTPS - } - default HostnameVerifier getHostnameVerifier() { return null; } diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index 84879ccd68..1e26e55678 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -30,6 +30,7 @@ import java.util.Objects; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLParameters; import javax.net.ssl.TrustManagerFactory; import redis.clients.jedis.util.InsecureTrustManagerFactory; @@ -56,7 +57,9 @@ public class SslOptions { private final char[] truststorePassword; - private final boolean noTruststoreVerification; + private final SSLParameters sslParameters; + + private final SslVerifyMode sslVerifyMode; private SslOptions(Builder builder) { this.keyManagerAlgorithm = builder.keyManagerAlgorithm; @@ -67,7 +70,8 @@ private SslOptions(Builder builder) { this.keystorePassword = builder.keystorePassword; this.truststoreResource = builder.truststoreResource; this.truststorePassword = builder.truststorePassword; - this.noTruststoreVerification = builder.noTruststoreVerification; + this.sslParameters = builder.sslParameters; + this.sslVerifyMode = builder.sslVerifyMode; } /** @@ -101,7 +105,9 @@ public static class Builder { private char[] truststorePassword = new char[0]; - private boolean noTruststoreVerification = false; + private SSLParameters sslParameters; + + private SslVerifyMode sslVerifyMode = SslVerifyMode.FULL; private Builder() { } @@ -303,8 +309,13 @@ private Builder truststore(Resource resource, char[] truststorePassword) { return this; } - public Builder noTruststoreVerification(boolean noTruststoreVerification) { - this.noTruststoreVerification = noTruststoreVerification; + public Builder sslParameters(SSLParameters sslParameters) { + this.sslParameters = sslParameters; + return this; + } + + public Builder sslVerifyMode(SslVerifyMode sslVerifyMode) { + this.sslVerifyMode = sslVerifyMode; return this; } @@ -314,6 +325,9 @@ public Builder noTruststoreVerification(boolean noTruststoreVerification) { * @return new instance of {@link SslOptions} */ public SslOptions build() { + if (this.sslParameters == null) { + this.sslParameters = new SSLParameters(); + } return new SslOptions(this); } @@ -322,16 +336,24 @@ public SslOptions build() { /** * A {@link SSLContext} object that is configured with values from this {@link SslOptions} object. * - * @return a {@link SSLContext}. + * @return {@link SSLContext} * @throws IOException thrown when loading the keystore or the truststore fails. * @throws GeneralSecurityException thrown when loading the keystore or the truststore fails. */ public SSLContext createSslContext() throws IOException, GeneralSecurityException { - final KeyManagerFactory keyManagerFactory; - if (keyManagerAlgorithm == null) { - keyManagerFactory = null; - } else { + TrustManagerFactory trustManagerFactory = null; + + if (sslVerifyMode == SslVerifyMode.FULL) { + this.sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + } else if (sslVerifyMode == SslVerifyMode.CA) { + this.sslParameters.setEndpointIdentificationAlgorithm(""); + } else if (sslVerifyMode == SslVerifyMode.INSECURE) { + trustManagerFactory = InsecureTrustManagerFactory.INSTANCE; + } + + KeyManagerFactory keyManagerFactory = null; + if (keyManagerAlgorithm != null) { KeyStore keyStore = KeyStore.getInstance(keyStoreType); try (InputStream keystoreStream = keystoreResource.get()) { @@ -340,20 +362,12 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio keyManagerFactory = KeyManagerFactory.getInstance(keyManagerAlgorithm); keyManagerFactory.init(keyStore, keystorePassword); - } - final TrustManagerFactory trustManagerFactory; - if (noTruststoreVerification) { - - trustManagerFactory = InsecureTrustManagerFactory.INSTANCE; - + if (trustManagerFactory != null) { + // skip } else if (trustManagerAlgorithm == null) { - trustManagerFactory = null; - - } else { - KeyStore trustStore = KeyStore.getInstance(trustStoreType); try (InputStream truststoreStream = truststoreResource.get()) { trustStore.load(truststoreStream, truststorePassword); @@ -361,7 +375,6 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio trustManagerFactory = TrustManagerFactory.getInstance(trustManagerAlgorithm); trustManagerFactory.init(trustStore); - } SSLContext sslContext = SSLContext.getDefault(); @@ -375,8 +388,16 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio return sslContext; } - private static boolean isEmpty(String cs) { - return cs == null || cs.isEmpty(); + /** + * {@link #createSslContext()} must be called before this. + * @return {@link SSLParameters} + */ + public SSLParameters getSslParameters() { + return sslParameters; + } + + private static boolean isEmpty(String str) { + return str == null || str.isEmpty(); } private static char[] getPassword(String truststorePassword) { diff --git a/src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java b/src/main/java/redis/clients/jedis/SslVerifyMode.java similarity index 61% rename from src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java rename to src/main/java/redis/clients/jedis/SslVerifyMode.java index 4b39b947ae..3cd0146161 100644 --- a/src/main/java/redis/clients/jedis/SslHostnameVerifyMode.java +++ b/src/main/java/redis/clients/jedis/SslVerifyMode.java @@ -3,9 +3,11 @@ /** * Enumeration of SSL/TLS hostname verification modes. */ -public enum SslHostnameVerifyMode { +public enum SslVerifyMode { - FULL, + INSECURE, - CA; + CA, + + FULL; } From 499dfed8ff2bbeb3815641a14b4c1aa4cc11ac03 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 10 Nov 2024 22:49:22 +0600 Subject: [PATCH 12/46] Remove InsecureTrustManagerFactory --- .../java/redis/clients/jedis/SslOptions.java | 86 ++++++- .../util/InsecureTrustManagerFactory.java | 225 ------------------ 2 files changed, 73 insertions(+), 238 deletions(-) delete mode 100644 src/main/java/redis/clients/jedis/util/InsecureTrustManagerFactory.java diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index 1e26e55678..e5d3c5f059 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -23,16 +23,27 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.Socket; import java.net.URL; -import java.security.GeneralSecurityException; -import java.security.KeyStore; import java.util.Arrays; import java.util.Objects; + +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import javax.net.ssl.KeyManager; + import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; +import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; -import redis.clients.jedis.util.InsecureTrustManagerFactory; +import javax.net.ssl.X509ExtendedTrustManager; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Options to configure SSL options for the connections kept to Redis servers. @@ -41,6 +52,8 @@ */ public class SslOptions { + private static final Logger logger = LoggerFactory.getLogger(SslOptions.class); + private final String keyManagerAlgorithm; private final String trustManagerAlgorithm; @@ -342,17 +355,17 @@ public SslOptions build() { */ public SSLContext createSslContext() throws IOException, GeneralSecurityException { - TrustManagerFactory trustManagerFactory = null; + TrustManager[] trustManagers = null; if (sslVerifyMode == SslVerifyMode.FULL) { this.sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); } else if (sslVerifyMode == SslVerifyMode.CA) { this.sslParameters.setEndpointIdentificationAlgorithm(""); } else if (sslVerifyMode == SslVerifyMode.INSECURE) { - trustManagerFactory = InsecureTrustManagerFactory.INSTANCE; + trustManagers = new TrustManager[] { INSECURE_TRUST_MANAGER }; } - KeyManagerFactory keyManagerFactory = null; + KeyManager[] keyManagers = null; if (keyManagerAlgorithm != null) { KeyStore keyStore = KeyStore.getInstance(keyStoreType); @@ -360,11 +373,12 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio keyStore.load(keystoreStream, keystorePassword); } - keyManagerFactory = KeyManagerFactory.getInstance(keyManagerAlgorithm); + KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(keyManagerAlgorithm); keyManagerFactory.init(keyStore, keystorePassword); + keyManagers = keyManagerFactory.getKeyManagers(); } - if (trustManagerFactory != null) { + if (trustManagers != null) { // skip } else if (trustManagerAlgorithm == null) { @@ -373,17 +387,15 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio trustStore.load(truststoreStream, truststorePassword); } - trustManagerFactory = TrustManagerFactory.getInstance(trustManagerAlgorithm); + TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(trustManagerAlgorithm); trustManagerFactory.init(trustStore); + trustManagers = trustManagerFactory.getTrustManagers(); } SSLContext sslContext = SSLContext.getDefault(); //SSLContext sslContext = SSLContext.getInstance("TLS"); // examples - sslContext.init( - keyManagerFactory == null ? null : keyManagerFactory.getKeyManagers(), - trustManagerFactory == null ? null : trustManagerFactory.getTrustManagers(), - null); + sslContext.init(keyManagers, trustManagers, null); return sslContext; } @@ -451,4 +463,52 @@ static Resource from(File file) { } + private static final X509Certificate[] EMPTY_X509_CERTIFICATES = {}; + + private static final TrustManager INSECURE_TRUST_MANAGER = new X509ExtendedTrustManager() { + + @Override + public void checkClientTrusted(X509Certificate[] chain, String s) { + if (logger.isDebugEnabled()) { + logger.debug("Accepting a client certificate: " + chain[0].getSubjectDN()); + } + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String s) { + if (logger.isDebugEnabled()) { + logger.debug("Accepting a server certificate: " + chain[0].getSubjectDN()); + } + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String s, Socket socket) + throws CertificateException { + checkClientTrusted(chain, s); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String s, SSLEngine sslEngine) + throws CertificateException { + checkClientTrusted(chain, s); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String s, Socket socket) + throws CertificateException { + checkServerTrusted(chain, s); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String s, SSLEngine sslEngine) + throws CertificateException { + checkServerTrusted(chain, s); + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return EMPTY_X509_CERTIFICATES; + } + }; + } diff --git a/src/main/java/redis/clients/jedis/util/InsecureTrustManagerFactory.java b/src/main/java/redis/clients/jedis/util/InsecureTrustManagerFactory.java deleted file mode 100644 index c5e4411340..0000000000 --- a/src/main/java/redis/clients/jedis/util/InsecureTrustManagerFactory.java +++ /dev/null @@ -1,225 +0,0 @@ -/* - * Copyright 2014 The Netty Project - * - * The Netty Project 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: - * - * https://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 redis.clients.jedis.util; - -import java.net.Socket; -import javax.net.ssl.ManagerFactoryParameters; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.TrustManager; -import javax.net.ssl.TrustManagerFactory; -import javax.net.ssl.TrustManagerFactorySpi; -import javax.net.ssl.X509ExtendedTrustManager; -import javax.net.ssl.X509TrustManager; -import java.security.InvalidAlgorithmParameterException; -import java.security.KeyStore; -import java.security.KeyStoreException; -import java.security.Provider; -import java.security.cert.CertificateException; -import java.security.cert.X509Certificate; - -import java.util.Objects; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * An insecure {@link TrustManagerFactory} that trusts all X.509 certificates without any verification. - *

- * NOTE: - * Never use this {@link TrustManagerFactory} in production. - * It is purely for testing purposes, and thus it is very insecure. - *

- */ -public final class InsecureTrustManagerFactory extends TrustManagerFactory { - - private static final Logger logger = LoggerFactory.getLogger(InsecureTrustManagerFactory.class); - - public static final TrustManagerFactory INSTANCE = new InsecureTrustManagerFactory(); - - private static final X509Certificate[] EMPTY_X509_CERTIFICATES = {}; - - private static final Provider PROVIDER = new Provider("", 0.0, "") { - private static final long serialVersionUID = -2680540247105807895L; - }; - - /** - * {@link InsecureTrustManagerFactorySpi} must have a reference to {@link InsecureTrustManagerFactory} - * to delegate its callbacks back to {@link InsecureTrustManagerFactory}. However, it is impossible to do so, - * because {@link TrustManagerFactory} requires {@link TrustManagerFactorySpi} at construction time and - * does not provide a way to access it later. - * - * To work around this issue, we use an ugly hack which uses a {@link ThreadLocal}. - */ - private static final ThreadLocal CURRENT_SPI = - new ThreadLocal() { - @Override - protected InsecureTrustManagerFactorySpi initialValue() { - return new InsecureTrustManagerFactorySpi(); - } - }; - - private static final TrustManager tm = new X509TrustManager() { - @Override - public void checkClientTrusted(X509Certificate[] chain, String s) { - if (logger.isDebugEnabled()) { - logger.debug("Accepting a client certificate: " + chain[0].getSubjectDN()); - } - } - - @Override - public void checkServerTrusted(X509Certificate[] chain, String s) { - if (logger.isDebugEnabled()) { - logger.debug("Accepting a server certificate: " + chain[0].getSubjectDN()); - } - } - - @Override - public X509Certificate[] getAcceptedIssuers() { - return EMPTY_X509_CERTIFICATES; - } - }; - - private InsecureTrustManagerFactory() { - super(CURRENT_SPI.get(), PROVIDER, ""); - CURRENT_SPI.get().init(this); - CURRENT_SPI.remove(); - } - - /** - * Initializes this factory with a source of certificate authorities and related trust material. - * - * @see TrustManagerFactorySpi#engineInit(KeyStore) - */ - protected void engineInit(KeyStore keyStore) throws Exception { } - - /** - * Initializes this factory with a source of provider-specific key material. - * - * @see TrustManagerFactorySpi#engineInit(ManagerFactoryParameters) - */ - protected void engineInit(ManagerFactoryParameters managerFactoryParameters) throws Exception { } - - /** - * Returns one trust manager for each type of trust material. - * - * @see TrustManagerFactorySpi#engineGetTrustManagers() - */ - protected TrustManager[] engineGetTrustManagers() { - return new TrustManager[] { tm }; - } - - static final class InsecureTrustManagerFactorySpi extends TrustManagerFactorySpi { - - private InsecureTrustManagerFactory parent; - private volatile TrustManager[] trustManagers; - - void init(InsecureTrustManagerFactory parent) { - this.parent = parent; - } - - @Override - protected void engineInit(KeyStore keyStore) throws KeyStoreException { - try { - parent.engineInit(keyStore); - } catch (KeyStoreException e) { - throw e; - } catch (Exception e) { - throw new KeyStoreException(e); - } - } - - @Override - protected void engineInit(ManagerFactoryParameters managerFactoryParameters) - throws InvalidAlgorithmParameterException { - try { - parent.engineInit(managerFactoryParameters); - } catch (InvalidAlgorithmParameterException e) { - throw e; - } catch (Exception e) { - throw new InvalidAlgorithmParameterException(e); - } - } - - @Override - protected TrustManager[] engineGetTrustManagers() { - TrustManager[] trustManagers = this.trustManagers; - if (trustManagers == null) { - trustManagers = parent.engineGetTrustManagers(); - wrapForJava7Plus(trustManagers); - this.trustManagers = trustManagers; - } - return trustManagers.clone(); - } - - private static void wrapForJava7Plus(TrustManager[] trustManagers) { - for (int i = 0; i < trustManagers.length; i++) { - final TrustManager tm = trustManagers[i]; - if (tm instanceof X509TrustManager && !(tm instanceof X509ExtendedTrustManager)) { - trustManagers[i] = new X509TrustManagerWrapper((X509TrustManager) tm); - } - } - } - } - - static final class X509TrustManagerWrapper extends X509ExtendedTrustManager { - - private final X509TrustManager delegate; - - X509TrustManagerWrapper(X509TrustManager delegate) { - this.delegate = Objects.requireNonNull(delegate, "delegate"); - } - - @Override - public void checkClientTrusted(X509Certificate[] chain, String s) throws CertificateException { - delegate.checkClientTrusted(chain, s); - } - - @Override - public void checkClientTrusted(X509Certificate[] chain, String s, Socket socket) - throws CertificateException { - delegate.checkClientTrusted(chain, s); - } - - @Override - public void checkClientTrusted(X509Certificate[] chain, String s, SSLEngine sslEngine) - throws CertificateException { - delegate.checkClientTrusted(chain, s); - } - - @Override - public void checkServerTrusted(X509Certificate[] chain, String s) throws CertificateException { - delegate.checkServerTrusted(chain, s); - } - - @Override - public void checkServerTrusted(X509Certificate[] chain, String s, Socket socket) - throws CertificateException { - delegate.checkServerTrusted(chain, s); - } - - @Override - public void checkServerTrusted(X509Certificate[] chain, String s, SSLEngine sslEngine) - throws CertificateException { - delegate.checkServerTrusted(chain, s); - } - - @Override - public X509Certificate[] getAcceptedIssuers() { - return delegate.getAcceptedIssuers(); - } - } - -} From b196739b9f6a465c8d2202137b7865b0442d2c97 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 18 Nov 2024 17:06:27 +0600 Subject: [PATCH 13/46] Disable ALL existing SSL tests --- .../clients/jedis/SSLACLJedisClusterTest.java | 1 + .../java/redis/clients/jedis/SSLACLJedisTest.java | 1 + .../redis/clients/jedis/SSLJedisClusterTest.java | 1 + .../clients/jedis/SSLJedisSentinelPoolTest.java | 1 + .../java/redis/clients/jedis/SSLJedisTest.java | 15 ++++++++------- .../csc/SSLJedisPooledClientSideCacheTest.java | 1 + 6 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java index 16eb63c0e0..2540473e17 100644 --- a/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java @@ -18,6 +18,7 @@ import redis.clients.jedis.SSLJedisTest.BasicHostnameVerifier; import redis.clients.jedis.util.RedisVersionUtil; +@org.junit.Ignore public class SSLACLJedisClusterTest extends JedisClusterTestBase { private static final int DEFAULT_REDIRECTIONS = 5; diff --git a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java index 8151729e6e..df0f89acbd 100644 --- a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java @@ -20,6 +20,7 @@ *

* This test is only executed when the server/cluster is Redis 6. or more. */ +@org.junit.Ignore public class SSLACLJedisTest { protected static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-acl-tls"); diff --git a/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java index f4763fe875..802b99620b 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java @@ -17,6 +17,7 @@ import redis.clients.jedis.exceptions.JedisClusterOperationException; import redis.clients.jedis.SSLJedisTest.BasicHostnameVerifier; +@org.junit.Ignore public class SSLJedisClusterTest extends JedisClusterTestBase { private static final int DEFAULT_REDIRECTIONS = 5; diff --git a/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java index 7468c9abfa..7fa8d88ec3 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java @@ -6,6 +6,7 @@ import org.junit.BeforeClass; import org.junit.Test; +@org.junit.Ignore public class SSLJedisSentinelPoolTest { private static final String MASTER_NAME = "aclmaster"; diff --git a/src/test/java/redis/clients/jedis/SSLJedisTest.java b/src/test/java/redis/clients/jedis/SSLJedisTest.java index 4ef4f969bb..8aae443fe6 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisTest.java @@ -23,6 +23,7 @@ import org.junit.BeforeClass; import org.junit.Test; +@org.junit.Ignore public class SSLJedisTest { protected static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-tls"); @@ -33,15 +34,15 @@ public static void prepare() { } public static void setupTrustStore() { - setJvmTrustStore("src/test/resources/truststore.jceks", "jceks"); +// setJvmTrustStore("src/test/resources/truststore.jceks", "jceks"); } - private static void setJvmTrustStore(String trustStoreFilePath, String trustStoreType) { - assertTrue(String.format("Could not find trust store at '%s'.", trustStoreFilePath), - new File(trustStoreFilePath).exists()); - System.setProperty("javax.net.ssl.trustStore", trustStoreFilePath); - System.setProperty("javax.net.ssl.trustStoreType", trustStoreType); - } +// private static void setJvmTrustStore(String trustStoreFilePath, String trustStoreType) { +// assertTrue(String.format("Could not find trust store at '%s'.", trustStoreFilePath), +// new File(trustStoreFilePath).exists()); +// System.setProperty("javax.net.ssl.trustStore", trustStoreFilePath); +// System.setProperty("javax.net.ssl.trustStoreType", trustStoreType); +// } @Test public void connectWithSsl() { diff --git a/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java b/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java index b8df3910cd..42c8f29f63 100644 --- a/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java +++ b/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java @@ -4,6 +4,7 @@ import redis.clients.jedis.HostAndPorts; import redis.clients.jedis.SSLJedisTest; +@org.junit.Ignore public class SSLJedisPooledClientSideCacheTest extends JedisPooledClientSideCacheTestBase { @BeforeClass From 1566a2a73cd59764ddaa1784b5d7105caa1e9ea3 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 18 Nov 2024 17:57:30 +0600 Subject: [PATCH 14/46] JedisTest with SslOptions --- .../java/redis/clients/jedis/SslOptions.java | 9 +- .../clients/jedis/SSLOptionsJedisTest.java | 139 ++++++++++++++++++ 2 files changed, 144 insertions(+), 4 deletions(-) create mode 100644 src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index e5d3c5f059..278ddc3ba5 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -366,7 +366,7 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio } KeyManager[] keyManagers = null; - if (keyManagerAlgorithm != null) { + if (keystoreResource != null) { KeyStore keyStore = KeyStore.getInstance(keyStoreType); try (InputStream keystoreStream = keystoreResource.get()) { @@ -380,7 +380,7 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio if (trustManagers != null) { // skip - } else if (trustManagerAlgorithm == null) { + } else if (truststoreResource != null) { KeyStore trustStore = KeyStore.getInstance(trustStoreType); try (InputStream truststoreStream = truststoreResource.get()) { @@ -392,8 +392,9 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio trustManagers = trustManagerFactory.getTrustManagers(); } - SSLContext sslContext = SSLContext.getDefault(); - //SSLContext sslContext = SSLContext.getInstance("TLS"); // examples + //SSLContext sslContext = SSLContext.getDefault(); // throws exception + //SSLContext sslContext = SSLContext.getInstance("TLS"); // in redis.io examples, works + SSLContext sslContext = SSLContext.getInstance("SSL"); // also works sslContext.init(keyManagers, trustManagers, null); diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java new file mode 100644 index 0000000000..15a6010326 --- /dev/null +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java @@ -0,0 +1,139 @@ +package redis.clients.jedis; + +import static org.junit.Assert.assertEquals; + +import java.io.File; +import java.io.FileInputStream; +import java.io.InputStream; +import java.security.InvalidAlgorithmParameterException; +import java.security.KeyStore; +import java.security.SecureRandom; +import java.security.cert.X509Certificate; + +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; +import javax.net.ssl.X509TrustManager; + +import org.junit.Test; + +public class SSLOptionsJedisTest { + + protected static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-tls"); + + @Test + public void connectWithSsl() { + try (Jedis jedis = new Jedis(endpoint.getHostAndPort(), + DefaultJedisClientConfig.builder() + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .build()).build())) { + jedis.auth(endpoint.getPassword()); + assertEquals("PONG", jedis.ping()); + } + } + + @Test + public void connectWithSslConfig() { + try (Jedis jedis = new Jedis(endpoint.getHostAndPort(), + endpoint.getClientConfigBuilder() + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .build()).build())) { + jedis.auth(endpoint.getPassword()); + assertEquals("PONG", jedis.ping()); + } + } + + @Test + public void connectWithSslInsecure() { + try (Jedis jedis = new Jedis(endpoint.getHostAndPort(), + endpoint.getClientConfigBuilder() + .sslOptions(SslOptions.builder() + .sslVerifyMode(SslVerifyMode.INSECURE) + .build()).build())) { + jedis.auth(endpoint.getPassword()); + assertEquals("PONG", jedis.ping()); + } + } + + /** + * Creates an SSLSocketFactory that trusts all certificates in truststore.jceks. + */ + static SSLSocketFactory createTrustStoreSslSocketFactory() throws Exception { + + KeyStore trustStore = KeyStore.getInstance("jceks"); + + try (InputStream inputStream = new FileInputStream("src/test/resources/truststore.jceks")) { + trustStore.load(inputStream, null); + } + + TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance("PKIX"); + trustManagerFactory.init(trustStore); + TrustManager[] trustManagers = trustManagerFactory.getTrustManagers(); + + SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init(null, trustManagers, new SecureRandom()); + return sslContext.getSocketFactory(); + } + + /** + * Creates an SSLSocketFactory with a trust manager that does not trust any certificates. + */ + static SSLSocketFactory createTrustNoOneSslSocketFactory() throws Exception { + TrustManager[] unTrustManagers = new TrustManager[] { new X509TrustManager() { + public X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; + } + + public void checkClientTrusted(X509Certificate[] chain, String authType) { + throw new RuntimeException(new InvalidAlgorithmParameterException()); + } + + public void checkServerTrusted(X509Certificate[] chain, String authType) { + throw new RuntimeException(new InvalidAlgorithmParameterException()); + } + } }; + SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init(null, unTrustManagers, new SecureRandom()); + return sslContext.getSocketFactory(); + } + + /** + * Very basic hostname verifier implementation for testing. NOT recommended for production. + */ + static class BasicHostnameVerifier implements HostnameVerifier { + + private static final String COMMON_NAME_RDN_PREFIX = "CN="; + + @Override + public boolean verify(String hostname, SSLSession session) { + X509Certificate peerCertificate; + try { + peerCertificate = (X509Certificate) session.getPeerCertificates()[0]; + } catch (SSLPeerUnverifiedException e) { + throw new IllegalStateException("The session does not contain a peer X.509 certificate.", e); + } + String peerCertificateCN = getCommonName(peerCertificate); + return hostname.equals(peerCertificateCN); + } + + private String getCommonName(X509Certificate peerCertificate) { + String subjectDN = peerCertificate.getSubjectDN().getName(); + String[] dnComponents = subjectDN.split(","); + for (String dnComponent : dnComponents) { + dnComponent = dnComponent.trim(); + if (dnComponent.startsWith(COMMON_NAME_RDN_PREFIX)) { + return dnComponent.substring(COMMON_NAME_RDN_PREFIX.length()); + } + } + throw new IllegalArgumentException("The certificate has no common name."); + } + } +} From 06e258724ba19225760431dca665d9c8c2358ac8 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 18 Nov 2024 18:16:35 +0600 Subject: [PATCH 15/46] SSLACLJedisTest with SslOptions --- .../redis/clients/jedis/SSLACLJedisTest.java | 81 ------------------- .../clients/jedis/SSLOptionsJedisTest.java | 18 ++++- 2 files changed, 15 insertions(+), 84 deletions(-) diff --git a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java index df0f89acbd..027c0055d4 100644 --- a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java @@ -2,14 +2,6 @@ import static org.junit.Assert.*; -import java.io.FileInputStream; -import java.io.InputStream; -import java.security.InvalidAlgorithmParameterException; -import java.security.KeyStore; -import java.security.SecureRandom; -import java.security.cert.X509Certificate; -import javax.net.ssl.*; - import org.junit.BeforeClass; import org.junit.Test; @@ -77,77 +69,4 @@ public void connectWithUri() { assertEquals("PONG", jedis.ping()); } } - - /** - * Creates an SSLSocketFactory that trusts all certificates in truststore.jceks. - */ - static SSLSocketFactory createTrustStoreSslSocketFactory() throws Exception { - - KeyStore trustStore = KeyStore.getInstance("jceks"); - try (InputStream inputStream = new FileInputStream("src/test/resources/truststore.jceks")) { - trustStore.load(inputStream, null); - } - - TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance("PKIX"); - trustManagerFactory.init(trustStore); - TrustManager[] trustManagers = trustManagerFactory.getTrustManagers(); - - SSLContext sslContext = SSLContext.getInstance("TLS"); - sslContext.init(null, trustManagers, new SecureRandom()); - return sslContext.getSocketFactory(); - } - - /** - * Creates an SSLSocketFactory with a trust manager that does not trust any certificates. - */ - static SSLSocketFactory createTrustNoOneSslSocketFactory() throws Exception { - TrustManager[] unTrustManagers = new TrustManager[] { new X509TrustManager() { - public X509Certificate[] getAcceptedIssuers() { - return new X509Certificate[0]; - } - - public void checkClientTrusted(X509Certificate[] chain, String authType) { - throw new RuntimeException(new InvalidAlgorithmParameterException()); - } - - public void checkServerTrusted(X509Certificate[] chain, String authType) { - throw new RuntimeException(new InvalidAlgorithmParameterException()); - } - } }; - SSLContext sslContext = SSLContext.getInstance("TLS"); - sslContext.init(null, unTrustManagers, new SecureRandom()); - return sslContext.getSocketFactory(); - } - - /** - * Very basic hostname verifier implementation for testing. NOT recommended for production. - */ - static class BasicHostnameVerifier implements HostnameVerifier { - - private static final String COMMON_NAME_RDN_PREFIX = "CN="; - - @Override - public boolean verify(String hostname, SSLSession session) { - X509Certificate peerCertificate; - try { - peerCertificate = (X509Certificate) session.getPeerCertificates()[0]; - } catch (SSLPeerUnverifiedException e) { - throw new IllegalStateException("The session does not contain a peer X.509 certificate.", e); - } - String peerCertificateCN = getCommonName(peerCertificate); - return hostname.equals(peerCertificateCN); - } - - private String getCommonName(X509Certificate peerCertificate) { - String subjectDN = peerCertificate.getSubjectDN().getName(); - String[] dnComponents = subjectDN.split(","); - for (String dnComponent : dnComponents) { - dnComponent = dnComponent.trim(); - if (dnComponent.startsWith(COMMON_NAME_RDN_PREFIX)) { - return dnComponent.substring(COMMON_NAME_RDN_PREFIX.length()); - } - } - throw new IllegalArgumentException("The certificate has no common name."); - } - } } diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java index 15a6010326..719b51b6ba 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java @@ -25,6 +25,8 @@ public class SSLOptionsJedisTest { protected static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-tls"); + protected static final EndpointConfig aclEndpoint = HostAndPorts.getRedisEndpoint("standalone0-acl-tls"); + @Test public void connectWithSsl() { try (Jedis jedis = new Jedis(endpoint.getHostAndPort(), @@ -39,14 +41,13 @@ public void connectWithSsl() { } @Test - public void connectWithSslConfig() { + public void connectWithConfig() { try (Jedis jedis = new Jedis(endpoint.getHostAndPort(), endpoint.getClientConfigBuilder() .sslOptions(SslOptions.builder() .truststore(new File("src/test/resources/truststore.jceks")) .trustStoreType("jceks") .build()).build())) { - jedis.auth(endpoint.getPassword()); assertEquals("PONG", jedis.ping()); } } @@ -58,7 +59,18 @@ public void connectWithSslInsecure() { .sslOptions(SslOptions.builder() .sslVerifyMode(SslVerifyMode.INSECURE) .build()).build())) { - jedis.auth(endpoint.getPassword()); + assertEquals("PONG", jedis.ping()); + } + } + + @Test + public void connectWithAcl() { + try (Jedis jedis = new Jedis(aclEndpoint.getHostAndPort(), + aclEndpoint.getClientConfigBuilder() + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .build()).build())) { assertEquals("PONG", jedis.ping()); } } From 4b27241e466022d457233081f6f5ba3ef0ab5662 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 18 Nov 2024 19:34:54 +0600 Subject: [PATCH 16/46] SSLOptionsJedisSentinelPoolTest with SslOptions --- .../SSLOptionsJedisSentinelPoolTest.java | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 src/test/java/redis/clients/jedis/SSLOptionsJedisSentinelPoolTest.java diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisSentinelPoolTest.java new file mode 100644 index 0000000000..062ef49052 --- /dev/null +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisSentinelPoolTest.java @@ -0,0 +1,100 @@ +package redis.clients.jedis; + +import java.io.File; +import java.util.HashSet; +import java.util.Set; +import org.apache.commons.pool2.impl.GenericObjectPoolConfig; +import org.junit.BeforeClass; +import org.junit.Test; + +public class SSLOptionsJedisSentinelPoolTest { + + private static final String MASTER_NAME = "aclmaster"; + + private static Set sentinels = new HashSet<>(); + + private static final HostAndPortMapper SSL_PORT_MAPPER = (HostAndPort hap) + -> new HostAndPort(hap.getHost(), hap.getPort() + 10000); + + private static final GenericObjectPoolConfig POOL_CONFIG = new GenericObjectPoolConfig<>(); + + @BeforeClass + public static void prepare() { + sentinels.add(HostAndPorts.getSentinelServers().get(4)); + } + + @Test + public void sentinelWithoutSslConnectsToRedisWithSsl() { + + DefaultJedisClientConfig masterConfig = DefaultJedisClientConfig.builder() + .user("acljedis").password("fizzbuzz").clientName("master-client") + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .sslVerifyMode(SslVerifyMode.CA).build()) + .hostAndPortMapper(SSL_PORT_MAPPER).build(); + + DefaultJedisClientConfig sentinelConfig = DefaultJedisClientConfig.builder() + .user("sentinel").password("foobared").clientName("sentinel-client").build(); + + try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, masterConfig, sentinelConfig)) { + pool.getResource().close(); + } + + try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, POOL_CONFIG, + masterConfig, sentinelConfig)) { + pool.getResource().close(); + } + } + + @Test + public void sentinelWithSslConnectsToRedisWithoutSsl() { + + DefaultJedisClientConfig masterConfig = DefaultJedisClientConfig.builder() + .user("acljedis").password("fizzbuzz").clientName("master-client").build(); + + DefaultJedisClientConfig sentinelConfig = DefaultJedisClientConfig.builder() + .user("sentinel").password("foobared").clientName("sentinel-client") + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .sslVerifyMode(SslVerifyMode.CA).build()) + .hostAndPortMapper(SSL_PORT_MAPPER).build(); + + try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, masterConfig, sentinelConfig)) { + pool.getResource().close(); + } + + try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, POOL_CONFIG, + masterConfig, sentinelConfig)) { + pool.getResource().close(); + } + } + + @Test + public void sentinelWithSslConnectsToRedisWithSsl() { + + SslOptions sslOptions = SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .sslVerifyMode(SslVerifyMode.CA).build(); + + DefaultJedisClientConfig masterConfig = DefaultJedisClientConfig.builder() + .user("acljedis").password("fizzbuzz").clientName("master-client").sslOptions(sslOptions) + .hostAndPortMapper(SSL_PORT_MAPPER).build(); + + DefaultJedisClientConfig sentinelConfig = DefaultJedisClientConfig.builder() + .user("sentinel").password("foobared").clientName("sentinel-client").sslOptions(sslOptions) + .hostAndPortMapper(SSL_PORT_MAPPER).build(); + + try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, masterConfig, sentinelConfig)) { + pool.getResource().close(); + } + + try (JedisSentinelPool pool = new JedisSentinelPool(MASTER_NAME, sentinels, POOL_CONFIG, + masterConfig, sentinelConfig)) { + pool.getResource().close(); + } + } + +} From caa0b34cd937d5d87e8824fd2f845a90fc9d47f7 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 18 Nov 2024 21:19:13 +0600 Subject: [PATCH 17/46] SSLJedisClusterTest with SslOptions --- .../jedis/SSLOptionsJedisClusterTest.java | 216 ++++++++++++++++++ 1 file changed, 216 insertions(+) create mode 100644 src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java new file mode 100644 index 0000000000..0f6974e070 --- /dev/null +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java @@ -0,0 +1,216 @@ +package redis.clients.jedis; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.util.Collections; +import java.util.Map; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocketFactory; + +import org.junit.Assert; +import org.junit.Test; + +import redis.clients.jedis.exceptions.JedisClusterOperationException; +import redis.clients.jedis.SSLJedisTest.BasicHostnameVerifier; + +public class SSLOptionsJedisClusterTest extends JedisClusterTestBase { + + private static final int DEFAULT_REDIRECTIONS = 5; + private static final ConnectionPoolConfig DEFAULT_POOL_CONFIG = new ConnectionPoolConfig(); + + private final HostAndPortMapper hostAndPortMap = (HostAndPort hostAndPort) -> { + String host = hostAndPort.getHost(); + int port = hostAndPort.getPort(); + if (host.equals("127.0.0.1")) { + host = "localhost"; + port = port + 1000; + } + return new HostAndPort(host, port); + }; + + // don't map IP addresses so that we try to connect with host 127.0.0.1 + private final HostAndPortMapper portMap = (HostAndPort hostAndPort) -> { + if ("localhost".equals(hostAndPort.getHost())) { + return hostAndPort; + } + return new HostAndPort(hostAndPort.getHost(), hostAndPort.getPort() + 1000); + }; + + @Test + public void testSSLDiscoverNodesAutomatically() { + try (JedisCluster jc2 = new JedisCluster(new HostAndPort("localhost", 8379), + DefaultJedisClientConfig.builder().password("cluster") + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build()) + .hostAndPortMapper(hostAndPortMap).build(), + DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + Map clusterNodes = jc2.getClusterNodes(); + assertEquals(3, clusterNodes.size()); + assertTrue(clusterNodes.containsKey("127.0.0.1:7379")); + assertTrue(clusterNodes.containsKey("127.0.0.1:7380")); + assertTrue(clusterNodes.containsKey("127.0.0.1:7381")); + jc2.get("foo"); + } + } + + @Test + public void testSSLWithoutPortMap() { + try (JedisCluster jc = new JedisCluster(Collections.singleton(new HostAndPort("localhost", 8379)), + DefaultJedisClientConfig.builder().password("cluster") + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build()) + .build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + Map clusterNodes = jc.getClusterNodes(); + assertEquals(3, clusterNodes.size()); + assertTrue(clusterNodes.containsKey("127.0.0.1:7379")); + assertTrue(clusterNodes.containsKey("127.0.0.1:7380")); + assertTrue(clusterNodes.containsKey("127.0.0.1:7381")); + } + } + + @Test + public void connectByIpAddress() { + try (JedisCluster jc = new JedisCluster(new HostAndPort("127.0.0.1", 7379), + DefaultJedisClientConfig.builder().password("cluster") + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build()) + .hostAndPortMapper(hostAndPortMap).build(), + DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + jc.get("foo"); + } + } + + @Test + public void connectToNodesFailsWithSSLParametersAndNoHostMapping() { + final SSLParameters sslParameters = new SSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + + try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), + DefaultJedisClientConfig.builder().password("cluster") + .sslOptions(SslOptions.builder() + .sslParameters(sslParameters) + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build()) + .hostAndPortMapper(portMap).build(), + DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + jc.get("foo"); + Assert.fail("It should fail after all cluster attempts."); + } catch (JedisClusterOperationException e) { + // initial connection to localhost works, but subsequent connections to nodes use 127.0.0.1 + // and fail hostname verification + assertEquals("No more cluster attempts left.", e.getMessage()); + } + } + + @Test + public void connectToNodesSucceedsWithSSLParametersAndHostMapping() { + final SSLParameters sslParameters = new SSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + + try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), + DefaultJedisClientConfig.builder().password("cluster") + .sslOptions(SslOptions.builder() + .sslParameters(sslParameters) + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build()) + .hostAndPortMapper(hostAndPortMap).build(), + DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + jc.get("foo"); + } + } + + @Test + public void connectByIpAddressFailsWithSSLParameters() { + final SSLParameters sslParameters = new SSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + + try (JedisCluster jc = new JedisCluster(new HostAndPort("127.0.0.1", 8379), + DefaultJedisClientConfig.builder().password("cluster") + .sslOptions(SslOptions.builder() + .sslParameters(sslParameters) + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build()) + .hostAndPortMapper(hostAndPortMap).build(), + DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + } catch (JedisClusterOperationException e) { + assertEquals("Could not initialize cluster slots cache.", e.getMessage()); + } + } + + @Test + @org.junit.Ignore // TODO: drop support for custom hostname verifier (with SslOptions) ?? + public void connectWithCustomHostNameVerifier() { + HostnameVerifier hostnameVerifier = new BasicHostnameVerifier(); + HostnameVerifier localhostVerifier = new LocalhostVerifier(); + + try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), + DefaultJedisClientConfig.builder().password("cluster").ssl(true) + .hostnameVerifier(hostnameVerifier).hostAndPortMapper(portMap).build(), + DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + jc.get("foo"); + Assert.fail("It should fail after all cluster attempts."); + } catch (JedisClusterOperationException e) { + // initial connection made with 'localhost' but subsequent connections to nodes use 127.0.0.1 + // which causes custom hostname verification to fail + assertEquals("No more cluster attempts left.", e.getMessage()); + } + + try (JedisCluster jc2 = new JedisCluster(new HostAndPort("127.0.0.1", 8379), + DefaultJedisClientConfig.builder().password("cluster").ssl(true) + .hostnameVerifier(hostnameVerifier).hostAndPortMapper(portMap).build(), + DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + } catch (JedisClusterOperationException e) { + assertEquals("Could not initialize cluster slots cache.", e.getMessage()); + } + + try (JedisCluster jc3 = new JedisCluster(new HostAndPort("localhost", 8379), + DefaultJedisClientConfig.builder().password("cluster").ssl(true) + .hostnameVerifier(localhostVerifier).hostAndPortMapper(portMap).build(), + DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + jc3.get("foo"); + } + } + + @Test + @org.junit.Ignore // TODO: how do we do this with SslOptions + public void connectWithCustomSocketFactory() throws Exception { + final SSLSocketFactory sslSocketFactory = SSLJedisTest.createTrustStoreSslSocketFactory(); + + try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), + DefaultJedisClientConfig.builder().password("cluster").ssl(true) + .sslSocketFactory(sslSocketFactory).hostAndPortMapper(portMap).build(), + DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + assertEquals(3, jc.getClusterNodes().size()); + } + } + + @Test + @org.junit.Ignore // TODO: how do we do this with SslOptions + public void connectWithEmptyTrustStore() throws Exception { + final SSLSocketFactory sslSocketFactory = SSLJedisTest.createTrustNoOneSslSocketFactory(); + + try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), + DefaultJedisClientConfig.builder().password("cluster").ssl(true) + .sslSocketFactory(sslSocketFactory).build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { + } catch (JedisClusterOperationException e) { + assertEquals("Could not initialize cluster slots cache.", e.getMessage()); + } + } + + public class LocalhostVerifier extends BasicHostnameVerifier { + @Override + public boolean verify(String hostname, SSLSession session) { + if (hostname.equals("127.0.0.1")) { + hostname = "localhost"; + } + return super.verify(hostname, session); + } + } +} From 11bb4465686b38823c0e13a51848f36e89e60c35 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 18 Nov 2024 22:22:59 +0600 Subject: [PATCH 18/46] TODO comment to enable existing SSL tests --- src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java | 2 +- src/test/java/redis/clients/jedis/SSLACLJedisTest.java | 2 +- src/test/java/redis/clients/jedis/SSLJedisClusterTest.java | 2 +- src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java | 2 +- src/test/java/redis/clients/jedis/SSLJedisTest.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java index 2540473e17..2dfaf9e43d 100644 --- a/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java @@ -18,7 +18,7 @@ import redis.clients.jedis.SSLJedisTest.BasicHostnameVerifier; import redis.clients.jedis.util.RedisVersionUtil; -@org.junit.Ignore +@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLACLJedisClusterTest extends JedisClusterTestBase { private static final int DEFAULT_REDIRECTIONS = 5; diff --git a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java index 027c0055d4..af7fd49e62 100644 --- a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java @@ -12,7 +12,7 @@ *

* This test is only executed when the server/cluster is Redis 6. or more. */ -@org.junit.Ignore +@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLACLJedisTest { protected static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-acl-tls"); diff --git a/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java index 802b99620b..f46cd45db7 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java @@ -17,7 +17,7 @@ import redis.clients.jedis.exceptions.JedisClusterOperationException; import redis.clients.jedis.SSLJedisTest.BasicHostnameVerifier; -@org.junit.Ignore +@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLJedisClusterTest extends JedisClusterTestBase { private static final int DEFAULT_REDIRECTIONS = 5; diff --git a/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java index 7fa8d88ec3..3f576956d8 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java @@ -6,7 +6,7 @@ import org.junit.BeforeClass; import org.junit.Test; -@org.junit.Ignore +@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLJedisSentinelPoolTest { private static final String MASTER_NAME = "aclmaster"; diff --git a/src/test/java/redis/clients/jedis/SSLJedisTest.java b/src/test/java/redis/clients/jedis/SSLJedisTest.java index 8aae443fe6..0d3c1e01d6 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisTest.java @@ -23,7 +23,7 @@ import org.junit.BeforeClass; import org.junit.Test; -@org.junit.Ignore +@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLJedisTest { protected static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-tls"); From ff88451400df82e7e38a9a984f6523660df12152 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 18 Nov 2024 22:24:58 +0600 Subject: [PATCH 19/46] TODO command to enable existing SSL tests in csc package --- .../clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java b/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java index 42c8f29f63..c62a4460ee 100644 --- a/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java +++ b/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java @@ -4,7 +4,7 @@ import redis.clients.jedis.HostAndPorts; import redis.clients.jedis.SSLJedisTest; -@org.junit.Ignore +@org.junit.Ignore // TODO: enable public class SSLJedisPooledClientSideCacheTest extends JedisPooledClientSideCacheTestBase { @BeforeClass From 315d73d5f8382a8c606a38858537a5485587fa28 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 18 Nov 2024 22:59:20 +0600 Subject: [PATCH 20/46] Enable existing SSL tests without impacting new ones --- .../clients/jedis/SSLACLJedisClusterTest.java | 13 ++++--- .../redis/clients/jedis/SSLACLJedisTest.java | 6 ++++ .../clients/jedis/SSLJedisClusterTest.java | 7 +++- .../jedis/SSLJedisSentinelPoolTest.java | 7 +++- .../redis/clients/jedis/SSLJedisTest.java | 34 +++++++++++++------ .../SSLJedisPooledClientSideCacheTest.java | 7 +++- 6 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java index 2dfaf9e43d..fbe57f66f7 100644 --- a/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLACLJedisClusterTest.java @@ -10,6 +10,7 @@ import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocketFactory; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; @@ -18,7 +19,6 @@ import redis.clients.jedis.SSLJedisTest.BasicHostnameVerifier; import redis.clients.jedis.util.RedisVersionUtil; -@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLACLJedisClusterTest extends JedisClusterTestBase { private static final int DEFAULT_REDIRECTIONS = 5; @@ -44,15 +44,20 @@ public class SSLACLJedisClusterTest extends JedisClusterTestBase { @BeforeClass public static void prepare() { - // We need to set up certificates first before connecting to the endpoint with enabled TLS - SSLJedisTest.setupTrustStore(); - // TODO(imalinovskyi): Remove hardcoded connection settings // once this test is refactored to support RE org.junit.Assume.assumeTrue("Not running ACL test on this version of Redis", RedisVersionUtil.checkRedisMajorVersionNumber(6, new EndpointConfig(new HostAndPort("localhost", 8379), "default", "cluster", true))); + + // We need to set up certificates first before connecting to the endpoint with enabled TLS + SSLJedisTest.setupTrustStore(); + } + + @AfterClass + public static void unprepare() { + SSLJedisTest.cleanupTrustStore(); } @Test diff --git a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java index af7fd49e62..8e39e26b6d 100644 --- a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.*; +import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -28,6 +29,11 @@ public static void prepare() { RedisVersionUtil.checkRedisMajorVersionNumber(6, endpoint)); } + @AfterClass + public static void unprepare() { + SSLJedisTest.cleanupTrustStore(); + } + @Test public void connectWithSsl() { try (Jedis jedis = new Jedis(endpoint.getHost(), endpoint.getPort(), true)) { diff --git a/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java index f46cd45db7..805783601c 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java @@ -10,6 +10,7 @@ import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocketFactory; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; @@ -17,7 +18,6 @@ import redis.clients.jedis.exceptions.JedisClusterOperationException; import redis.clients.jedis.SSLJedisTest.BasicHostnameVerifier; -@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLJedisClusterTest extends JedisClusterTestBase { private static final int DEFAULT_REDIRECTIONS = 5; @@ -46,6 +46,11 @@ public static void prepare() { SSLJedisTest.setupTrustStore(); // set up trust store for SSL tests } + @AfterClass + public static void unprepare() { + SSLJedisTest.cleanupTrustStore(); + } + @Test public void testSSLDiscoverNodesAutomatically() { try (JedisCluster jc = new JedisCluster(Collections.singleton(new HostAndPort("localhost", 8379)), diff --git a/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java b/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java index 3f576956d8..4fda9646c3 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisSentinelPoolTest.java @@ -3,10 +3,10 @@ import java.util.HashSet; import java.util.Set; import org.apache.commons.pool2.impl.GenericObjectPoolConfig; +import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; -@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLJedisSentinelPoolTest { private static final String MASTER_NAME = "aclmaster"; @@ -25,6 +25,11 @@ public static void prepare() { sentinels.add(HostAndPorts.getSentinelServers().get(4)); } + @AfterClass + public static void unprepare() { + SSLJedisTest.cleanupTrustStore(); + } + @Test public void sentinelWithoutSslConnectsToRedisWithSsl() { diff --git a/src/test/java/redis/clients/jedis/SSLJedisTest.java b/src/test/java/redis/clients/jedis/SSLJedisTest.java index 0d3c1e01d6..a26fc4a9ae 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisTest.java @@ -20,30 +20,44 @@ import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509TrustManager; +import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; -@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLJedisTest { protected static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-tls"); + public static void setupTrustStore() { + setJvmTrustStore("src/test/resources/truststore.jceks", "jceks"); + } + + private static void setJvmTrustStore(String trustStoreFilePath, String trustStoreType) { + assertTrue(String.format("Could not find trust store at '%s'.", trustStoreFilePath), + new File(trustStoreFilePath).exists()); + System.setProperty("javax.net.ssl.trustStore", trustStoreFilePath); + System.setProperty("javax.net.ssl.trustStoreType", trustStoreType); + } + + public static void cleanupTrustStore() { + clearJvmTrustStore(); + } + + private static void clearJvmTrustStore() { + System.clearProperty("javax.net.ssl.trustStore"); + System.clearProperty("javax.net.ssl.trustStoreType"); + } + @BeforeClass public static void prepare() { setupTrustStore(); } - public static void setupTrustStore() { -// setJvmTrustStore("src/test/resources/truststore.jceks", "jceks"); + @AfterClass + public static void unprepare() { + cleanupTrustStore(); } -// private static void setJvmTrustStore(String trustStoreFilePath, String trustStoreType) { -// assertTrue(String.format("Could not find trust store at '%s'.", trustStoreFilePath), -// new File(trustStoreFilePath).exists()); -// System.setProperty("javax.net.ssl.trustStore", trustStoreFilePath); -// System.setProperty("javax.net.ssl.trustStoreType", trustStoreType); -// } - @Test public void connectWithSsl() { try (Jedis jedis = new Jedis(endpoint.getHost(), endpoint.getPort(), true)) { diff --git a/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java b/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java index c62a4460ee..f59f248b7a 100644 --- a/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java +++ b/src/test/java/redis/clients/jedis/csc/SSLJedisPooledClientSideCacheTest.java @@ -1,10 +1,10 @@ package redis.clients.jedis.csc; +import org.junit.AfterClass; import org.junit.BeforeClass; import redis.clients.jedis.HostAndPorts; import redis.clients.jedis.SSLJedisTest; -@org.junit.Ignore // TODO: enable public class SSLJedisPooledClientSideCacheTest extends JedisPooledClientSideCacheTestBase { @BeforeClass @@ -14,4 +14,9 @@ public static void prepare() { endpoint = HostAndPorts.getRedisEndpoint("standalone0-tls"); } + @AfterClass + public static void unprepare() { + SSLJedisTest.cleanupTrustStore(); + } + } From 00d6dcece33851bf4f71a3e8183b9a202fedc8e6 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 18 Nov 2024 23:03:41 +0600 Subject: [PATCH 21/46] Missing enable existing SSL tests without impacting new ones --- src/test/java/redis/clients/jedis/SSLACLJedisTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java index 8e39e26b6d..a6b2d4c059 100644 --- a/src/test/java/redis/clients/jedis/SSLACLJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLACLJedisTest.java @@ -13,7 +13,6 @@ *

* This test is only executed when the server/cluster is Redis 6. or more. */ -@org.junit.Ignore // TODO: enable -- (in a different way?) public class SSLACLJedisTest { protected static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-acl-tls"); From f56aec33bd4cf451f249fd66d71b02a092f2e938 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 24 Nov 2024 17:47:31 +0600 Subject: [PATCH 22/46] Keep only Builder pattern constructor for DefaultJedisClientConfig --- .../jedis/DefaultJedisClientConfig.java | 75 ++++++++++--------- .../clients/jedis/JedisClientConfig.java | 1 + 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index b4ab76064b..7ae0746b82 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -29,29 +29,6 @@ public final class DefaultJedisClientConfig implements JedisClientConfig { private final boolean readOnlyForRedisClusterReplicas; - // TODO: how many constructors should we have? 1) all params, 2) builder 3) another config - private DefaultJedisClientConfig(RedisProtocol protocol, int connectionTimeoutMillis, int soTimeoutMillis, - int blockingSocketTimeoutMillis, Supplier credentialsProvider, int database, - String clientName, boolean ssl, SSLSocketFactory sslSocketFactory, SSLParameters sslParameters, - HostnameVerifier hostnameVerifier, HostAndPortMapper hostAndPortMapper, - ClientSetInfoConfig clientSetInfoConfig, boolean readOnlyForRedisClusterReplicas) { - this.redisProtocol = protocol; - this.connectionTimeoutMillis = connectionTimeoutMillis; - this.socketTimeoutMillis = soTimeoutMillis; - this.blockingSocketTimeoutMillis = blockingSocketTimeoutMillis; - this.credentialsProvider = credentialsProvider; - this.database = database; - this.clientName = clientName; - this.ssl = ssl; - this.sslSocketFactory = sslSocketFactory; - this.sslParameters = sslParameters; - this.sslOptions = null; // TODO: - this.hostnameVerifier = hostnameVerifier; - this.hostAndPortMapper = hostAndPortMapper; - this.clientSetInfoConfig = clientSetInfoConfig; - this.readOnlyForRedisClusterReplicas = readOnlyForRedisClusterReplicas; - } - private DefaultJedisClientConfig(DefaultJedisClientConfig.Builder builder) { this.redisProtocol = builder.redisProtocol; this.connectionTimeoutMillis = builder.connectionTimeoutMillis; @@ -304,25 +281,51 @@ public Builder readOnlyForRedisClusterReplicas() { } } - // TOOD: depends on which constructors we're going to have/keep public static DefaultJedisClientConfig create(int connectionTimeoutMillis, int soTimeoutMillis, int blockingSocketTimeoutMillis, String user, String password, int database, String clientName, boolean ssl, SSLSocketFactory sslSocketFactory, SSLParameters sslParameters, HostnameVerifier hostnameVerifier, HostAndPortMapper hostAndPortMapper) { - return new DefaultJedisClientConfig(null, - connectionTimeoutMillis, soTimeoutMillis, blockingSocketTimeoutMillis, - new DefaultRedisCredentialsProvider(new DefaultRedisCredentials(user, password)), database, - clientName, ssl, sslSocketFactory, sslParameters, hostnameVerifier, hostAndPortMapper, null, - false); + Builder builder = builder(); + builder.connectionTimeoutMillis(connectionTimeoutMillis).socketTimeoutMillis(soTimeoutMillis) + .blockingSocketTimeoutMillis(blockingSocketTimeoutMillis); + if (user != null || password != null) { + // deliberately not handling 'user != null && password == null' here + builder.credentials(new DefaultRedisCredentials(user, password)); + } + builder.database(database).clientName(clientName); + builder.ssl(ssl).sslSocketFactory(sslSocketFactory).sslParameters(sslParameters).hostnameVerifier(hostnameVerifier); + builder.hostAndPortMapper(hostAndPortMapper); + return builder.build(); } - // TOOD: depends on which constructors we're going to have/keep public static DefaultJedisClientConfig copyConfig(JedisClientConfig copy) { - return new DefaultJedisClientConfig(copy.getRedisProtocol(), - copy.getConnectionTimeoutMillis(), copy.getSocketTimeoutMillis(), - copy.getBlockingSocketTimeoutMillis(), copy.getCredentialsProvider(), - copy.getDatabase(), copy.getClientName(), copy.isSsl(), copy.getSslSocketFactory(), - copy.getSslParameters(), copy.getHostnameVerifier(), copy.getHostAndPortMapper(), - copy.getClientSetInfoConfig(), copy.isReadOnlyForRedisClusterReplicas()); + Builder builder = builder(); + builder.protocol(copy.getRedisProtocol()); + builder.connectionTimeoutMillis(copy.getConnectionTimeoutMillis()); + builder.socketTimeoutMillis(copy.getSocketTimeoutMillis()); + builder.blockingSocketTimeoutMillis(copy.getBlockingSocketTimeoutMillis()); + + Supplier credentialsProvider = copy.getCredentialsProvider(); + if (credentialsProvider != null) { + builder.credentialsProvider(credentialsProvider); + } else { + String user = copy.getUser(); + String password = copy.getPassword(); + if (user != null || password != null) { + // deliberately not handling 'user != null && password == null' here + builder.credentials(new DefaultRedisCredentials(user, password)); + } + } + + builder.database(copy.getDatabase()); + builder.clientName(copy.getClientName()); + + builder.ssl(copy.isSsl()); + builder.sslSocketFactory(copy.getSslSocketFactory()); + builder.sslParameters(copy.getSslParameters()); + builder.hostnameVerifier(copy.getHostnameVerifier()); + builder.sslOptions(copy.getSslOptions()); + builder.hostAndPortMapper(copy.getHostAndPortMapper()); + return builder.build(); } } diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index 4b65669124..1c99195308 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -44,6 +44,7 @@ default String getPassword() { return null; } + // TODO: return null default Supplier getCredentialsProvider() { return new DefaultRedisCredentialsProvider( new DefaultRedisCredentials(getUser(), getPassword())); From 06371b5af55527106a12be9c866fc55440b902d3 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 24 Nov 2024 17:54:53 +0600 Subject: [PATCH 23/46] Limit HostnameVerifier only for legacy ssl config and document as JavaDoc in JedisClientConfig --- .../clients/jedis/DefaultJedisSocketFactory.java | 11 +++++++---- .../java/redis/clients/jedis/JedisClientConfig.java | 11 ++++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java index 564e409b98..04b4e673eb 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java @@ -144,10 +144,13 @@ private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws I sslSocket.setSSLParameters(_sslParameters); } - if (hostnameVerifier != null && !hostnameVerifier.verify(_hostAndPort.getHost(), sslSocket.getSession())) { - String message = String.format("The connection to '%s' failed ssl/tls hostname verification.", - _hostAndPort.getHost()); - throw new JedisConnectionException(message); + if (sslOptions == null) { + // limiting HostnameVerifier only for legacy ssl config + if (hostnameVerifier != null && !hostnameVerifier.verify(_hostAndPort.getHost(), sslSocket.getSession())) { + String message = String.format("The connection to '%s' failed ssl/tls hostname verification.", + _hostAndPort.getHost()); + throw new JedisConnectionException(message); + } } return new SSLSocketWrapper(sslSocket, plainSocket); diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index 1c99195308..4bbdba1f6f 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -73,8 +73,13 @@ default SSLParameters getSslParameters() { return null; } + default HostnameVerifier getHostnameVerifier() { + return null; + } + /** - * {@link JedisClientConfig#isSsl()} and {@link JedisClientConfig#getSslSocketFactory()} will be ignored if + * {@link JedisClientConfig#isSsl()}, {@link JedisClientConfig#getSslSocketFactory()} and + * {@link JedisClientConfig#getHostnameVerifier()} will be ignored if * {@link JedisClientConfig#getSslOptions() this} is set. * @return ssl options */ @@ -82,10 +87,6 @@ default SslOptions getSslOptions() { return null; } - default HostnameVerifier getHostnameVerifier() { - return null; - } - default HostAndPortMapper getHostAndPortMapper() { return null; } From bcce93b942883a44444c66ae044ff76b8e5f6265 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 24 Nov 2024 18:05:08 +0600 Subject: [PATCH 24/46] Remove unused codes from SSLOptionsJedisTest --- .../clients/jedis/SSLOptionsJedisTest.java | 89 ------------------- 1 file changed, 89 deletions(-) diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java index 719b51b6ba..931b01d156 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java @@ -3,21 +3,6 @@ import static org.junit.Assert.assertEquals; import java.io.File; -import java.io.FileInputStream; -import java.io.InputStream; -import java.security.InvalidAlgorithmParameterException; -import java.security.KeyStore; -import java.security.SecureRandom; -import java.security.cert.X509Certificate; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLPeerUnverifiedException; -import javax.net.ssl.SSLSession; -import javax.net.ssl.SSLSocketFactory; -import javax.net.ssl.TrustManager; -import javax.net.ssl.TrustManagerFactory; -import javax.net.ssl.X509TrustManager; import org.junit.Test; @@ -74,78 +59,4 @@ public void connectWithAcl() { assertEquals("PONG", jedis.ping()); } } - - /** - * Creates an SSLSocketFactory that trusts all certificates in truststore.jceks. - */ - static SSLSocketFactory createTrustStoreSslSocketFactory() throws Exception { - - KeyStore trustStore = KeyStore.getInstance("jceks"); - - try (InputStream inputStream = new FileInputStream("src/test/resources/truststore.jceks")) { - trustStore.load(inputStream, null); - } - - TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance("PKIX"); - trustManagerFactory.init(trustStore); - TrustManager[] trustManagers = trustManagerFactory.getTrustManagers(); - - SSLContext sslContext = SSLContext.getInstance("TLS"); - sslContext.init(null, trustManagers, new SecureRandom()); - return sslContext.getSocketFactory(); - } - - /** - * Creates an SSLSocketFactory with a trust manager that does not trust any certificates. - */ - static SSLSocketFactory createTrustNoOneSslSocketFactory() throws Exception { - TrustManager[] unTrustManagers = new TrustManager[] { new X509TrustManager() { - public X509Certificate[] getAcceptedIssuers() { - return new X509Certificate[0]; - } - - public void checkClientTrusted(X509Certificate[] chain, String authType) { - throw new RuntimeException(new InvalidAlgorithmParameterException()); - } - - public void checkServerTrusted(X509Certificate[] chain, String authType) { - throw new RuntimeException(new InvalidAlgorithmParameterException()); - } - } }; - SSLContext sslContext = SSLContext.getInstance("TLS"); - sslContext.init(null, unTrustManagers, new SecureRandom()); - return sslContext.getSocketFactory(); - } - - /** - * Very basic hostname verifier implementation for testing. NOT recommended for production. - */ - static class BasicHostnameVerifier implements HostnameVerifier { - - private static final String COMMON_NAME_RDN_PREFIX = "CN="; - - @Override - public boolean verify(String hostname, SSLSession session) { - X509Certificate peerCertificate; - try { - peerCertificate = (X509Certificate) session.getPeerCertificates()[0]; - } catch (SSLPeerUnverifiedException e) { - throw new IllegalStateException("The session does not contain a peer X.509 certificate.", e); - } - String peerCertificateCN = getCommonName(peerCertificate); - return hostname.equals(peerCertificateCN); - } - - private String getCommonName(X509Certificate peerCertificate) { - String subjectDN = peerCertificate.getSubjectDN().getName(); - String[] dnComponents = subjectDN.split(","); - for (String dnComponent : dnComponents) { - dnComponent = dnComponent.trim(); - if (dnComponent.startsWith(COMMON_NAME_RDN_PREFIX)) { - return dnComponent.substring(COMMON_NAME_RDN_PREFIX.length()); - } - } - throw new IllegalArgumentException("The certificate has no common name."); - } - } } From 1fa23f292fd57cbe606ac535cfed44f1d7a78582 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 24 Nov 2024 18:12:13 +0600 Subject: [PATCH 25/46] Increase code reuse for LocalhostVerifier --- .../redis/clients/jedis/SSLJedisClusterTest.java | 12 +----------- src/test/java/redis/clients/jedis/SSLJedisTest.java | 10 ++++++++++ .../clients/jedis/SSLOptionsJedisClusterTest.java | 12 +----------- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java index 805783601c..bc43bff7b6 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisClusterTest.java @@ -7,7 +7,6 @@ import java.util.Map; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLParameters; -import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocketFactory; import org.junit.AfterClass; @@ -17,6 +16,7 @@ import redis.clients.jedis.exceptions.JedisClusterOperationException; import redis.clients.jedis.SSLJedisTest.BasicHostnameVerifier; +import redis.clients.jedis.SSLJedisTest.LocalhostVerifier; public class SSLJedisClusterTest extends JedisClusterTestBase { @@ -237,14 +237,4 @@ public void defaultHostAndPortUsedIfMapReturnsNull() { assertTrue(clusterNodes.containsKey("127.0.0.1:7381")); } } - - public class LocalhostVerifier extends BasicHostnameVerifier { - @Override - public boolean verify(String hostname, SSLSession session) { - if (hostname.equals("127.0.0.1")) { - hostname = "localhost"; - } - return super.verify(hostname, session); - } - } } diff --git a/src/test/java/redis/clients/jedis/SSLJedisTest.java b/src/test/java/redis/clients/jedis/SSLJedisTest.java index a26fc4a9ae..b0c11a4aab 100644 --- a/src/test/java/redis/clients/jedis/SSLJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLJedisTest.java @@ -186,4 +186,14 @@ private String getCommonName(X509Certificate peerCertificate) { throw new IllegalArgumentException("The certificate has no common name."); } } + + static class LocalhostVerifier extends BasicHostnameVerifier { + @Override + public boolean verify(String hostname, SSLSession session) { + if (hostname.equals("127.0.0.1")) { + hostname = "localhost"; + } + return super.verify(hostname, session); + } + } } diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java index 0f6974e070..18b334bc7a 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java @@ -8,7 +8,6 @@ import java.util.Map; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLParameters; -import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocketFactory; import org.junit.Assert; @@ -16,6 +15,7 @@ import redis.clients.jedis.exceptions.JedisClusterOperationException; import redis.clients.jedis.SSLJedisTest.BasicHostnameVerifier; +import redis.clients.jedis.SSLJedisTest.LocalhostVerifier; public class SSLOptionsJedisClusterTest extends JedisClusterTestBase { @@ -203,14 +203,4 @@ public void connectWithEmptyTrustStore() throws Exception { assertEquals("Could not initialize cluster slots cache.", e.getMessage()); } } - - public class LocalhostVerifier extends BasicHostnameVerifier { - @Override - public boolean verify(String hostname, SSLSession session) { - if (hostname.equals("127.0.0.1")) { - hostname = "localhost"; - } - return super.verify(hostname, session); - } - } } From a64a5106fec6e6f28f541e0f7b3d672bce37afe0 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 24 Nov 2024 19:14:46 +0600 Subject: [PATCH 26/46] Individual JavaDoc for each SslVerifyMode --- src/main/java/redis/clients/jedis/SslVerifyMode.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main/java/redis/clients/jedis/SslVerifyMode.java b/src/main/java/redis/clients/jedis/SslVerifyMode.java index 3cd0146161..e737240350 100644 --- a/src/main/java/redis/clients/jedis/SslVerifyMode.java +++ b/src/main/java/redis/clients/jedis/SslVerifyMode.java @@ -5,9 +5,18 @@ */ public enum SslVerifyMode { + /** + * No verification at all. + */ INSECURE, + /** + * Verify the CA and certificate without verifying that the hostname matches. + */ CA, + /** + * Full certificate verification. + */ FULL; } From 63aa76bdab5236e6d99383a81ce42d73e521b836 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 25 Nov 2024 20:33:44 +0600 Subject: [PATCH 27/46] Custom SocketFactory won't be supported with SslOptions --- .../jedis/SSLOptionsJedisClusterTest.java | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java index 18b334bc7a..3d9260a401 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java @@ -178,29 +178,4 @@ public void connectWithCustomHostNameVerifier() { } } - @Test - @org.junit.Ignore // TODO: how do we do this with SslOptions - public void connectWithCustomSocketFactory() throws Exception { - final SSLSocketFactory sslSocketFactory = SSLJedisTest.createTrustStoreSslSocketFactory(); - - try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), - DefaultJedisClientConfig.builder().password("cluster").ssl(true) - .sslSocketFactory(sslSocketFactory).hostAndPortMapper(portMap).build(), - DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { - assertEquals(3, jc.getClusterNodes().size()); - } - } - - @Test - @org.junit.Ignore // TODO: how do we do this with SslOptions - public void connectWithEmptyTrustStore() throws Exception { - final SSLSocketFactory sslSocketFactory = SSLJedisTest.createTrustNoOneSslSocketFactory(); - - try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), - DefaultJedisClientConfig.builder().password("cluster").ssl(true) - .sslSocketFactory(sslSocketFactory).build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { - } catch (JedisClusterOperationException e) { - assertEquals("Could not initialize cluster slots cache.", e.getMessage()); - } - } } From 10bc2dc012afe748e9f4042ed9991a42467fb9cd Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 25 Nov 2024 20:38:04 +0600 Subject: [PATCH 28/46] Deprecate DefaultJedisClientConfig.copyConfig() --- .../java/redis/clients/jedis/DefaultJedisClientConfig.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index 7ae0746b82..f845b230b9 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -298,6 +298,7 @@ public static DefaultJedisClientConfig create(int connectionTimeoutMillis, int s return builder.build(); } + @Deprecated public static DefaultJedisClientConfig copyConfig(JedisClientConfig copy) { Builder builder = builder(); builder.protocol(copy.getRedisProtocol()); @@ -326,6 +327,11 @@ public static DefaultJedisClientConfig copyConfig(JedisClientConfig copy) { builder.hostnameVerifier(copy.getHostnameVerifier()); builder.sslOptions(copy.getSslOptions()); builder.hostAndPortMapper(copy.getHostAndPortMapper()); + + builder.clientSetInfoConfig(copy.getClientSetInfoConfig()); + if (copy.isReadOnlyForRedisClusterReplicas()) { + builder.readOnlyForRedisClusterReplicas(); + } return builder.build(); } } From a0591c8ce289dad7f1b292788f1bfe950d8d141d Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 25 Nov 2024 21:02:40 +0600 Subject: [PATCH 29/46] Add option to set SSLContext protocol --- .../java/redis/clients/jedis/SslOptions.java | 17 +++++++++++------ .../clients/jedis/SSLOptionsJedisTest.java | 13 +++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index 278ddc3ba5..10774b8886 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -47,8 +47,6 @@ /** * Options to configure SSL options for the connections kept to Redis servers. - * - * @author Mark Paluch */ public class SslOptions { @@ -74,6 +72,8 @@ public class SslOptions { private final SslVerifyMode sslVerifyMode; + private final String sslContextProtocol; + private SslOptions(Builder builder) { this.keyManagerAlgorithm = builder.keyManagerAlgorithm; this.trustManagerAlgorithm = builder.trustManagerAlgorithm; @@ -85,6 +85,7 @@ private SslOptions(Builder builder) { this.truststorePassword = builder.truststorePassword; this.sslParameters = builder.sslParameters; this.sslVerifyMode = builder.sslVerifyMode; + this.sslContextProtocol = builder.sslContextProtocol; } /** @@ -122,6 +123,8 @@ public static class Builder { private SslVerifyMode sslVerifyMode = SslVerifyMode.FULL; + private String sslContextProtocol = "TLS"; + private Builder() { } @@ -332,6 +335,11 @@ public Builder sslVerifyMode(SslVerifyMode sslVerifyMode) { return this; } + public Builder sslContextProtocol(String protocol) { + this.sslContextProtocol = protocol; + return this; + } + /** * Create a new instance of {@link SslOptions} * @@ -392,10 +400,7 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio trustManagers = trustManagerFactory.getTrustManagers(); } - //SSLContext sslContext = SSLContext.getDefault(); // throws exception - //SSLContext sslContext = SSLContext.getInstance("TLS"); // in redis.io examples, works - SSLContext sslContext = SSLContext.getInstance("SSL"); // also works - + SSLContext sslContext = SSLContext.getInstance(sslContextProtocol); sslContext.init(keyManagers, trustManagers, null); return sslContext; diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java index 931b01d156..2ea1d233da 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java @@ -48,6 +48,19 @@ public void connectWithSslInsecure() { } } + @Test + public void connectWithSslContextProtocol() { + try (Jedis jedis = new Jedis(endpoint.getHostAndPort(), + endpoint.getClientConfigBuilder() + .sslOptions(SslOptions.builder() + .sslContextProtocol("SSL") + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .build()).build())) { + assertEquals("PONG", jedis.ping()); + } + } + @Test public void connectWithAcl() { try (Jedis jedis = new Jedis(aclEndpoint.getHostAndPort(), From 309d47766fc81f24064d0070244bc1232e16e412 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 25 Nov 2024 21:03:36 +0600 Subject: [PATCH 30/46] Remove options to set KeyManager and TrustManager algorithms --- .../java/redis/clients/jedis/SslOptions.java | 32 ++----------------- 1 file changed, 3 insertions(+), 29 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index 10774b8886..a636c9a502 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -52,9 +52,10 @@ public class SslOptions { private static final Logger logger = LoggerFactory.getLogger(SslOptions.class); - private final String keyManagerAlgorithm; + private final String keyManagerAlgorithm = KeyManagerFactory.getDefaultAlgorithm(); + //private final String keyManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); // Lettuce - private final String trustManagerAlgorithm; + private final String trustManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); private final String keyStoreType; @@ -75,8 +76,6 @@ public class SslOptions { private final String sslContextProtocol; private SslOptions(Builder builder) { - this.keyManagerAlgorithm = builder.keyManagerAlgorithm; - this.trustManagerAlgorithm = builder.trustManagerAlgorithm; this.keyStoreType = builder.keyStoreType; this.trustStoreType = builder.trustStoreType; this.keystoreResource = builder.keystoreResource; @@ -102,11 +101,6 @@ public static SslOptions.Builder builder() { */ public static class Builder { - private String keyManagerAlgorithm = KeyManagerFactory.getDefaultAlgorithm(); - //private String keyManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); // Lettuce - - private String trustManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); - private String keyStoreType; private String trustStoreType; @@ -128,26 +122,6 @@ public static class Builder { private Builder() { } - public Builder keyManagerAlgorithm(String keyManagerAlgorithm) { - this.keyManagerAlgorithm = Objects.requireNonNull(keyManagerAlgorithm, "KeyManagerAlgorithm must not be null"); - return this; - } - - public Builder systemDefaultKeyManager() { - this.keyManagerAlgorithm = null; - return this; - } - - public Builder trustManagerAlgorithm(String trustManagerAlgorithm) { - this.trustManagerAlgorithm = Objects.requireNonNull(trustManagerAlgorithm, "TrustManagerAlgorithm must not be null"); - return this; - } - - public Builder systemDefaultTrustManager() { - this.trustManagerAlgorithm = null; - return this; - } - /** * Sets the KeyStore type. Defaults to {@link KeyStore#getDefaultType()} if not set. * From 42596bd8639d3c33931bb62698c94ba518729276 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 25 Nov 2024 22:15:22 +0600 Subject: [PATCH 31/46] Add File checkers --- .../java/redis/clients/jedis/SslOptions.java | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index a636c9a502..bd803a3e00 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -32,8 +32,8 @@ import java.security.KeyStore; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; -import javax.net.ssl.KeyManager; +import javax.net.ssl.KeyManager; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; @@ -169,8 +169,7 @@ public Builder keystore(File keystore) { public Builder keystore(File keystore, char[] keystorePassword) { Objects.requireNonNull(keystore, "Keystore must not be null"); -// LettuceAssert.isTrue(keystore.exists(), () -> String.format("Keystore file %s does not exist", truststore)); -// LettuceAssert.isTrue(keystore.isFile(), () -> String.format("Keystore %s is not a file", truststore)); + assertFile("Keystore", keystore); return keystore(Resource.from(keystore), keystorePassword); } @@ -214,10 +213,9 @@ public Builder keystore(URL keystore, char[] keystorePassword) { */ public Builder keystore(Resource resource, char[] keystorePassword) { - Objects.requireNonNull(resource, "Keystore InputStreamProvider must not be null"); + this.keystoreResource = Objects.requireNonNull(resource, "Keystore InputStreamProvider must not be null"); this.keystorePassword = getPassword(keystorePassword); - this.keystoreResource = resource; return this; } @@ -246,8 +244,7 @@ public Builder truststore(File truststore) { public Builder truststore(File truststore, String truststorePassword) { Objects.requireNonNull(truststore, "Truststore must not be null"); -// LettuceAssert.isTrue(truststore.exists(), () -> String.format("Truststore file %s does not exist", truststore)); -// LettuceAssert.isTrue(truststore.isFile(), () -> String.format("Truststore file %s is not a file", truststore)); + assertFile("Truststore", truststore); return truststore(Resource.from(truststore), getPassword(truststorePassword)); } @@ -291,10 +288,9 @@ public Builder truststore(URL truststore, String truststorePassword) { */ private Builder truststore(Resource resource, char[] truststorePassword) { - Objects.requireNonNull(resource, "Truststore InputStreamProvider must not be null"); + this.truststoreResource = Objects.requireNonNull(resource, "Truststore InputStreamProvider must not be null"); this.truststorePassword = getPassword(truststorePassword); - this.truststoreResource = resource; return this; } @@ -361,7 +357,7 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio } if (trustManagers != null) { - // skip + // already processed } else if (truststoreResource != null) { KeyStore trustStore = KeyStore.getInstance(trustStoreType); @@ -388,18 +384,30 @@ public SSLParameters getSslParameters() { return sslParameters; } - private static boolean isEmpty(String str) { - return str == null || str.isEmpty(); - } - - private static char[] getPassword(String truststorePassword) { - return !isEmpty(truststorePassword) ? truststorePassword.toCharArray() : null; + private static char[] getPassword(String password) { + return password != null ? password.toCharArray() : null; } private static char[] getPassword(char[] chars) { return chars != null ? Arrays.copyOf(chars, chars.length) : null; } + /** + * Assert that {@code file} {@link File#exists() exists}. + * + * @param keyword file recognizer + * @param file + * @throws IllegalArgumentException if the file doesn't exist + */ + public static void assertFile(String keyword, File file) { + if (!file.exists()) { + throw new IllegalArgumentException(String.format("%s file %s does not exist", keyword, file)); + } + if (!file.isFile()) { + throw new IllegalArgumentException(String.format("%s file %s is not a file", keyword, file)); + } + } + /** * Supplier for a {@link InputStream} representing a resource. The resulting {@link InputStream} must be closed by * the calling code. From 1a44be1ab6d0c104600d68ab18700b52f9a0c822 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 27 Nov 2024 15:11:59 +0600 Subject: [PATCH 32/46] minor user/password change --- .../redis/clients/jedis/DefaultJedisClientConfig.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index f845b230b9..d5468f3a46 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -310,12 +310,8 @@ public static DefaultJedisClientConfig copyConfig(JedisClientConfig copy) { if (credentialsProvider != null) { builder.credentialsProvider(credentialsProvider); } else { - String user = copy.getUser(); - String password = copy.getPassword(); - if (user != null || password != null) { - // deliberately not handling 'user != null && password == null' here - builder.credentials(new DefaultRedisCredentials(user, password)); - } + builder.user(copy.getUser()); + builder.password(copy.getPassword()); } builder.database(copy.getDatabase()); From 46c031efb3bbc5942739c67db443c05f1ced7ffd Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 28 Nov 2024 17:02:13 +0600 Subject: [PATCH 33/46] minor update javadoc --- src/main/java/redis/clients/jedis/JedisClientConfig.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index 4bbdba1f6f..ef5478e30e 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -78,8 +78,8 @@ default HostnameVerifier getHostnameVerifier() { } /** - * {@link JedisClientConfig#isSsl()}, {@link JedisClientConfig#getSslSocketFactory()} and - * {@link JedisClientConfig#getHostnameVerifier()} will be ignored if + * {@link JedisClientConfig#isSsl()}, {@link JedisClientConfig#getSslSocketFactory()}, + * {@link JedisClientConfig#getSslParameters()} and {@link JedisClientConfig#getHostnameVerifier()} will be ignored if * {@link JedisClientConfig#getSslOptions() this} is set. * @return ssl options */ From 294cf258aa3039e1bc0939ea26713c429c561734 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 28 Nov 2024 17:14:38 +0600 Subject: [PATCH 34/46] Allow manual HostnameVerifier with SslOptions --- .../jedis/DefaultJedisSocketFactory.java | 12 +++++------ .../clients/jedis/JedisClientConfig.java | 12 +++++------ .../jedis/SSLOptionsJedisClusterTest.java | 20 +++++++++++++++---- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java index 04b4e673eb..1e88f21f32 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java @@ -144,13 +144,11 @@ private Socket createSslSocket(HostAndPort _hostAndPort, Socket socket) throws I sslSocket.setSSLParameters(_sslParameters); } - if (sslOptions == null) { - // limiting HostnameVerifier only for legacy ssl config - if (hostnameVerifier != null && !hostnameVerifier.verify(_hostAndPort.getHost(), sslSocket.getSession())) { - String message = String.format("The connection to '%s' failed ssl/tls hostname verification.", - _hostAndPort.getHost()); - throw new JedisConnectionException(message); - } + // allowing HostnameVerifier for both SslOptions and legacy ssl config + if (hostnameVerifier != null && !hostnameVerifier.verify(_hostAndPort.getHost(), sslSocket.getSession())) { + String message = String.format("The connection to '%s' failed ssl/tls hostname verification.", + _hostAndPort.getHost()); + throw new JedisConnectionException(message); } return new SSLSocketWrapper(sslSocket, plainSocket); diff --git a/src/main/java/redis/clients/jedis/JedisClientConfig.java b/src/main/java/redis/clients/jedis/JedisClientConfig.java index ef5478e30e..8bd18b5aaa 100644 --- a/src/main/java/redis/clients/jedis/JedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/JedisClientConfig.java @@ -73,13 +73,9 @@ default SSLParameters getSslParameters() { return null; } - default HostnameVerifier getHostnameVerifier() { - return null; - } - /** - * {@link JedisClientConfig#isSsl()}, {@link JedisClientConfig#getSslSocketFactory()}, - * {@link JedisClientConfig#getSslParameters()} and {@link JedisClientConfig#getHostnameVerifier()} will be ignored if + * {@link JedisClientConfig#isSsl()}, {@link JedisClientConfig#getSslSocketFactory()} and + * {@link JedisClientConfig#getSslParameters()} will be ignored if * {@link JedisClientConfig#getSslOptions() this} is set. * @return ssl options */ @@ -87,6 +83,10 @@ default SslOptions getSslOptions() { return null; } + default HostnameVerifier getHostnameVerifier() { + return null; + } + default HostAndPortMapper getHostAndPortMapper() { return null; } diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java index 3d9260a401..5cde75dd56 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java @@ -145,13 +145,19 @@ public void connectByIpAddressFailsWithSSLParameters() { } @Test - @org.junit.Ignore // TODO: drop support for custom hostname verifier (with SslOptions) ?? public void connectWithCustomHostNameVerifier() { HostnameVerifier hostnameVerifier = new BasicHostnameVerifier(); HostnameVerifier localhostVerifier = new LocalhostVerifier(); + SslOptions sslOptions = SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build(); + try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), - DefaultJedisClientConfig.builder().password("cluster").ssl(true) + DefaultJedisClientConfig.builder().password("cluster") + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build()) .hostnameVerifier(hostnameVerifier).hostAndPortMapper(portMap).build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { jc.get("foo"); @@ -163,7 +169,10 @@ public void connectWithCustomHostNameVerifier() { } try (JedisCluster jc2 = new JedisCluster(new HostAndPort("127.0.0.1", 8379), - DefaultJedisClientConfig.builder().password("cluster").ssl(true) + DefaultJedisClientConfig.builder().password("cluster") + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build()) .hostnameVerifier(hostnameVerifier).hostAndPortMapper(portMap).build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { } catch (JedisClusterOperationException e) { @@ -171,7 +180,10 @@ public void connectWithCustomHostNameVerifier() { } try (JedisCluster jc3 = new JedisCluster(new HostAndPort("localhost", 8379), - DefaultJedisClientConfig.builder().password("cluster").ssl(true) + DefaultJedisClientConfig.builder().password("cluster") + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks").build()) .hostnameVerifier(localhostVerifier).hostAndPortMapper(portMap).build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { jc3.get("foo"); From ac1de9cd21a242a9175f5deeb0058d0cddaaec09 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 28 Nov 2024 19:00:41 +0600 Subject: [PATCH 35/46] Make test connectWithCustomHostNameVerifier() pass --- .../redis/clients/jedis/SSLOptionsJedisClusterTest.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java index 5cde75dd56..a4ca35cbab 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java @@ -8,7 +8,6 @@ import java.util.Map; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLParameters; -import javax.net.ssl.SSLSocketFactory; import org.junit.Assert; import org.junit.Test; @@ -149,10 +148,6 @@ public void connectWithCustomHostNameVerifier() { HostnameVerifier hostnameVerifier = new BasicHostnameVerifier(); HostnameVerifier localhostVerifier = new LocalhostVerifier(); - SslOptions sslOptions = SslOptions.builder() - .truststore(new File("src/test/resources/truststore.jceks")) - .trustStoreType("jceks").build(); - try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), DefaultJedisClientConfig.builder().password("cluster") .sslOptions(SslOptions.builder() @@ -183,7 +178,8 @@ public void connectWithCustomHostNameVerifier() { DefaultJedisClientConfig.builder().password("cluster") .sslOptions(SslOptions.builder() .truststore(new File("src/test/resources/truststore.jceks")) - .trustStoreType("jceks").build()) + .trustStoreType("jceks") + .sslVerifyMode(SslVerifyMode.CA).build()) .hostnameVerifier(localhostVerifier).hostAndPortMapper(portMap).build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { jc3.get("foo"); From 9e60a8e734b85350df69eab5bf63a6dd8729c959 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 28 Nov 2024 20:01:44 +0600 Subject: [PATCH 36/46] Better SslOptions with custom HostnameVerifier in connectWithCustomHostNameVerifier() test --- .../jedis/SSLOptionsJedisClusterTest.java | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java index a4ca35cbab..bc568791c1 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisClusterTest.java @@ -148,11 +148,13 @@ public void connectWithCustomHostNameVerifier() { HostnameVerifier hostnameVerifier = new BasicHostnameVerifier(); HostnameVerifier localhostVerifier = new LocalhostVerifier(); - try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), - DefaultJedisClientConfig.builder().password("cluster") - .sslOptions(SslOptions.builder() + SslOptions sslOptions = SslOptions.builder() .truststore(new File("src/test/resources/truststore.jceks")) - .trustStoreType("jceks").build()) + .trustStoreType("jceks") + .sslVerifyMode(SslVerifyMode.CA).build(); + + try (JedisCluster jc = new JedisCluster(new HostAndPort("localhost", 8379), + DefaultJedisClientConfig.builder().password("cluster").sslOptions(sslOptions) .hostnameVerifier(hostnameVerifier).hostAndPortMapper(portMap).build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { jc.get("foo"); @@ -164,10 +166,7 @@ public void connectWithCustomHostNameVerifier() { } try (JedisCluster jc2 = new JedisCluster(new HostAndPort("127.0.0.1", 8379), - DefaultJedisClientConfig.builder().password("cluster") - .sslOptions(SslOptions.builder() - .truststore(new File("src/test/resources/truststore.jceks")) - .trustStoreType("jceks").build()) + DefaultJedisClientConfig.builder().password("cluster").sslOptions(sslOptions) .hostnameVerifier(hostnameVerifier).hostAndPortMapper(portMap).build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { } catch (JedisClusterOperationException e) { @@ -175,11 +174,7 @@ public void connectWithCustomHostNameVerifier() { } try (JedisCluster jc3 = new JedisCluster(new HostAndPort("localhost", 8379), - DefaultJedisClientConfig.builder().password("cluster") - .sslOptions(SslOptions.builder() - .truststore(new File("src/test/resources/truststore.jceks")) - .trustStoreType("jceks") - .sslVerifyMode(SslVerifyMode.CA).build()) + DefaultJedisClientConfig.builder().password("cluster").sslOptions(sslOptions) .hostnameVerifier(localhostVerifier).hostAndPortMapper(portMap).build(), DEFAULT_REDIRECTIONS, DEFAULT_POOL_CONFIG)) { jc3.get("foo"); From 0129990921cafdda198e39a5ecbe429fc4db644a Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 3 Dec 2024 20:02:00 +0600 Subject: [PATCH 37/46] Shorten sslContextProtocol to sslProtocol --- .../java/redis/clients/jedis/SslOptions.java | 18 +++++++++++------- .../clients/jedis/SSLOptionsJedisTest.java | 2 +- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index bd803a3e00..5221d77736 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -73,7 +73,7 @@ public class SslOptions { private final SslVerifyMode sslVerifyMode; - private final String sslContextProtocol; + private final String sslProtocol; // protocol for SSLContext private SslOptions(Builder builder) { this.keyStoreType = builder.keyStoreType; @@ -84,7 +84,7 @@ private SslOptions(Builder builder) { this.truststorePassword = builder.truststorePassword; this.sslParameters = builder.sslParameters; this.sslVerifyMode = builder.sslVerifyMode; - this.sslContextProtocol = builder.sslContextProtocol; + this.sslProtocol = builder.sslProtocol; } /** @@ -117,7 +117,7 @@ public static class Builder { private SslVerifyMode sslVerifyMode = SslVerifyMode.FULL; - private String sslContextProtocol = "TLS"; + private String sslProtocol = "TLS"; // protocol for SSLContext private Builder() { } @@ -305,8 +305,13 @@ public Builder sslVerifyMode(SslVerifyMode sslVerifyMode) { return this; } - public Builder sslContextProtocol(String protocol) { - this.sslContextProtocol = protocol; + /** + * The SSL/TLS protocol to be used to initialize {@link SSLContext}. + * @param protocol the ssl/tls protocol + * @return {@code this} + */ + public Builder sslProtocol(String protocol) { + this.sslProtocol = protocol; return this; } @@ -321,7 +326,6 @@ public SslOptions build() { } return new SslOptions(this); } - } /** @@ -370,7 +374,7 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio trustManagers = trustManagerFactory.getTrustManagers(); } - SSLContext sslContext = SSLContext.getInstance(sslContextProtocol); + SSLContext sslContext = SSLContext.getInstance(sslProtocol); sslContext.init(keyManagers, trustManagers, null); return sslContext; diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java index 2ea1d233da..1baa7c016b 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java @@ -53,7 +53,7 @@ public void connectWithSslContextProtocol() { try (Jedis jedis = new Jedis(endpoint.getHostAndPort(), endpoint.getClientConfigBuilder() .sslOptions(SslOptions.builder() - .sslContextProtocol("SSL") + .sslProtocol("SSL") .truststore(new File("src/test/resources/truststore.jceks")) .trustStoreType("jceks") .build()).build())) { From 7e298bb4c330a6a9c5a838eff61f3709278e09cb Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 3 Dec 2024 23:56:30 +0600 Subject: [PATCH 38/46] Use null as default password, unlike Lettuce where it uses empty char array. --- src/main/java/redis/clients/jedis/SslOptions.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index 5221d77736..b79dad3c27 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -52,8 +52,8 @@ public class SslOptions { private static final Logger logger = LoggerFactory.getLogger(SslOptions.class); - private final String keyManagerAlgorithm = KeyManagerFactory.getDefaultAlgorithm(); //private final String keyManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); // Lettuce + private final String keyManagerAlgorithm = KeyManagerFactory.getDefaultAlgorithm(); private final String trustManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); @@ -107,11 +107,13 @@ public static class Builder { private Resource keystoreResource; - private char[] keystorePassword = new char[0]; + //private char[] keystorePassword = new char[0]; // Lettuce + private char[] keystorePassword = null; private Resource truststoreResource; - private char[] truststorePassword = new char[0]; + //private char[] truststorePassword = new char[0]; // Lettuce + private char[] truststorePassword = null; private SSLParameters sslParameters; From fae11c0f8821206e1a8450401e9e34a578f3306c Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 5 Dec 2024 22:59:47 +0600 Subject: [PATCH 39/46] Make an accidental private truststore builder option public --- src/main/java/redis/clients/jedis/SslOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index b79dad3c27..c47b5f3e1b 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -288,7 +288,7 @@ public Builder truststore(URL truststore, String truststorePassword) { * @param truststorePassword the truststore password. May be empty to omit password and the truststore integrity check. * @return {@code this} */ - private Builder truststore(Resource resource, char[] truststorePassword) { + public Builder truststore(Resource resource, char[] truststorePassword) { this.truststoreResource = Objects.requireNonNull(resource, "Truststore InputStreamProvider must not be null"); From 04ea192d61a7b70a78e723991a54cbfe57e20528 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 17 Dec 2024 20:54:10 +0600 Subject: [PATCH 40/46] Remove Lettuce comments --- src/main/java/redis/clients/jedis/SslOptions.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index c47b5f3e1b..9c98029af7 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -52,7 +52,6 @@ public class SslOptions { private static final Logger logger = LoggerFactory.getLogger(SslOptions.class); - //private final String keyManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); // Lettuce private final String keyManagerAlgorithm = KeyManagerFactory.getDefaultAlgorithm(); private final String trustManagerAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); @@ -107,12 +106,10 @@ public static class Builder { private Resource keystoreResource; - //private char[] keystorePassword = new char[0]; // Lettuce private char[] keystorePassword = null; private Resource truststoreResource; - //private char[] truststorePassword = new char[0]; // Lettuce private char[] truststorePassword = null; private SSLParameters sslParameters; @@ -156,7 +153,6 @@ public Builder trustStoreType(String trustStoreType) { */ public Builder keystore(File keystore) { return keystore(keystore, null); - //return keystore(keystore, new char[0]); // Lettuce } /** From 9709018f399c9199c8e59044691cde7acc096664 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 17 Dec 2024 21:00:38 +0600 Subject: [PATCH 41/46] Add JedisPooled tests --- .../jedis/SSLOptionsJedisPooledTest.java | 62 +++++++++++++++++++ .../clients/jedis/SSLOptionsJedisTest.java | 2 +- 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 src/test/java/redis/clients/jedis/SSLOptionsJedisPooledTest.java diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisPooledTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisPooledTest.java new file mode 100644 index 0000000000..9aea9b945b --- /dev/null +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisPooledTest.java @@ -0,0 +1,62 @@ +package redis.clients.jedis; + +import static org.junit.Assert.assertEquals; + +import java.io.File; + +import org.junit.Test; + +public class SSLOptionsJedisPooledTest { + + protected static final EndpointConfig endpoint = HostAndPorts.getRedisEndpoint("standalone0-tls"); + + protected static final EndpointConfig aclEndpoint = HostAndPorts.getRedisEndpoint("standalone0-acl-tls"); + + @Test + public void connectWithClientConfig() { + try (JedisPooled jedis = new JedisPooled(endpoint.getHostAndPort(), + endpoint.getClientConfigBuilder() + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .build()).build())) { + assertEquals("PONG", jedis.ping()); + } + } + + @Test + public void connectWithSslInsecure() { + try (JedisPooled jedis = new JedisPooled(endpoint.getHostAndPort(), + endpoint.getClientConfigBuilder() + .sslOptions(SslOptions.builder() + .sslVerifyMode(SslVerifyMode.INSECURE) + .build()).build())) { + assertEquals("PONG", jedis.ping()); + } + } + + @Test + public void connectWithSslContextProtocol() { + try (JedisPooled jedis = new JedisPooled(endpoint.getHostAndPort(), + endpoint.getClientConfigBuilder() + .sslOptions(SslOptions.builder() + .sslProtocol("SSL") + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .build()).build())) { + assertEquals("PONG", jedis.ping()); + } + } + + @Test + public void connectWithAcl() { + try (JedisPooled jedis = new JedisPooled(aclEndpoint.getHostAndPort(), + aclEndpoint.getClientConfigBuilder() + .sslOptions(SslOptions.builder() + .truststore(new File("src/test/resources/truststore.jceks")) + .trustStoreType("jceks") + .build()).build())) { + assertEquals("PONG", jedis.ping()); + } + } +} diff --git a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java index 1baa7c016b..cc0f7f598e 100644 --- a/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java +++ b/src/test/java/redis/clients/jedis/SSLOptionsJedisTest.java @@ -26,7 +26,7 @@ public void connectWithSsl() { } @Test - public void connectWithConfig() { + public void connectWithClientConfig() { try (Jedis jedis = new Jedis(endpoint.getHostAndPort(), endpoint.getClientConfigBuilder() .sslOptions(SslOptions.builder() From 8e1152c04382a554defd540d3b766c41337cd652 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 17 Dec 2024 22:42:29 +0600 Subject: [PATCH 42/46] Use char array for password --- src/main/java/redis/clients/jedis/SslOptions.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index 9c98029af7..09b724da97 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -239,12 +239,12 @@ public Builder truststore(File truststore) { * @param truststorePassword the truststore password. May be empty to omit password and the truststore integrity check. * @return {@code this} */ - public Builder truststore(File truststore, String truststorePassword) { + public Builder truststore(File truststore, char[] truststorePassword) { Objects.requireNonNull(truststore, "Truststore must not be null"); assertFile("Truststore", truststore); - return truststore(Resource.from(truststore), getPassword(truststorePassword)); + return truststore(Resource.from(truststore), truststorePassword); } /** @@ -268,11 +268,11 @@ public Builder truststore(URL truststore) { * @param truststorePassword the truststore password. May be empty to omit password and the truststore integrity check. * @return {@code this} */ - public Builder truststore(URL truststore, String truststorePassword) { + public Builder truststore(URL truststore, char[] truststorePassword) { Objects.requireNonNull(truststore, "Truststore must not be null"); - return truststore(Resource.from(truststore), getPassword(truststorePassword)); + return truststore(Resource.from(truststore), truststorePassword); } /** @@ -386,10 +386,6 @@ public SSLParameters getSslParameters() { return sslParameters; } - private static char[] getPassword(String password) { - return password != null ? password.toCharArray() : null; - } - private static char[] getPassword(char[] chars) { return chars != null ? Arrays.copyOf(chars, chars.length) : null; } From 5357b68bc7bd44f7e57642af585b764dc525d1a0 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 17 Dec 2024 22:43:40 +0600 Subject: [PATCH 43/46] Remove file license --- .../java/redis/clients/jedis/SslOptions.java | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index 09b724da97..36e1334a51 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -1,22 +1,3 @@ -/* - * Copyright 2011-Present, Redis Ltd. and Contributors - * All rights reserved. - * - * Licensed under the MIT License. - * - * This file contains contributions from third-party contributors - * licensed 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 - * - * https://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 redis.clients.jedis; import java.io.File; From 9d13917843a456fba1c310585aa28ed77302ca57 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 18 Dec 2024 22:25:13 +0600 Subject: [PATCH 44/46] Address code review --- .../java/redis/clients/jedis/SslOptions.java | 20 +++++++++++++++---- .../redis/clients/jedis/SslVerifyMode.java | 2 ++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/main/java/redis/clients/jedis/SslOptions.java b/src/main/java/redis/clients/jedis/SslOptions.java index 36e1334a51..31b15c66fc 100644 --- a/src/main/java/redis/clients/jedis/SslOptions.java +++ b/src/main/java/redis/clients/jedis/SslOptions.java @@ -28,6 +28,8 @@ /** * Options to configure SSL options for the connections kept to Redis servers. + * + * @author Mark Paluch */ public class SslOptions { @@ -274,11 +276,23 @@ public Builder truststore(Resource resource, char[] truststorePassword) { return this; } + /** + * Sets a configured {@link SSLParameters}. + * + * @param sslParameters a {@link SSLParameters} object. + * @return {@code this} + */ public Builder sslParameters(SSLParameters sslParameters) { this.sslParameters = sslParameters; return this; } + /** + * Sets the {@link SslVerifyMode}. + * + * @param sslVerifyMode the {@link SslVerifyMode}. + * @return {@code this} + */ public Builder sslVerifyMode(SslVerifyMode sslVerifyMode) { this.sslVerifyMode = sslVerifyMode; return this; @@ -316,6 +330,7 @@ public SslOptions build() { */ public SSLContext createSslContext() throws IOException, GeneralSecurityException { + KeyManager[] keyManagers = null; TrustManager[] trustManagers = null; if (sslVerifyMode == SslVerifyMode.FULL) { @@ -326,7 +341,6 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio trustManagers = new TrustManager[] { INSECURE_TRUST_MANAGER }; } - KeyManager[] keyManagers = null; if (keystoreResource != null) { KeyStore keyStore = KeyStore.getInstance(keyStoreType); @@ -339,9 +353,7 @@ public SSLContext createSslContext() throws IOException, GeneralSecurityExceptio keyManagers = keyManagerFactory.getKeyManagers(); } - if (trustManagers != null) { - // already processed - } else if (truststoreResource != null) { + if (trustManagers == null && truststoreResource != null) { KeyStore trustStore = KeyStore.getInstance(trustStoreType); try (InputStream truststoreStream = truststoreResource.get()) { diff --git a/src/main/java/redis/clients/jedis/SslVerifyMode.java b/src/main/java/redis/clients/jedis/SslVerifyMode.java index e737240350..c0f999acca 100644 --- a/src/main/java/redis/clients/jedis/SslVerifyMode.java +++ b/src/main/java/redis/clients/jedis/SslVerifyMode.java @@ -6,6 +6,8 @@ public enum SslVerifyMode { /** + * DO NOT USE THIS IN PRODUCTION. + *

* No verification at all. */ INSECURE, From 8fbadc228f33c67db0223ef24f8ad5294cd7f9bd Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 24 Dec 2024 13:46:27 +0600 Subject: [PATCH 45/46] Merge fix --- src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index 7d41d9d28a..cf083e7407 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -308,6 +308,7 @@ public Builder from(JedisClientConfig instance) { this.ssl = instance.isSsl(); this.sslSocketFactory = instance.getSslSocketFactory(); this.sslParameters = instance.getSslParameters(); + this.sslOptions = instance.getSslOptions(); this.hostnameVerifier = instance.getHostnameVerifier(); this.hostAndPortMapper = instance.getHostAndPortMapper(); this.clientSetInfoConfig = instance.getClientSetInfoConfig(); From 25e65d36e78f51d7882b1cdbb9f165ac96529eb1 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Tue, 24 Dec 2024 13:47:02 +0600 Subject: [PATCH 46/46] Deprecate helper methods in DefaultJedisClientConfig --- .../jedis/DefaultJedisClientConfig.java | 8 ++++++++ .../java/redis/clients/jedis/JedisPooled.java | 19 ++++++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java index cf083e7407..25a4737ec0 100644 --- a/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java +++ b/src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java @@ -318,6 +318,10 @@ public Builder from(JedisClientConfig instance) { } } + /** + * @deprecated Use {@link redis.clients.jedis.DefaultJedisClientConfig.Builder}. + */ + @Deprecated public static DefaultJedisClientConfig create(int connectionTimeoutMillis, int soTimeoutMillis, int blockingSocketTimeoutMillis, String user, String password, int database, String clientName, boolean ssl, SSLSocketFactory sslSocketFactory, SSLParameters sslParameters, @@ -335,6 +339,10 @@ public static DefaultJedisClientConfig create(int connectionTimeoutMillis, int s return builder.build(); } + /** + * @deprecated Use + * {@link redis.clients.jedis.DefaultJedisClientConfig.Builder#from(redis.clients.jedis.JedisClientConfig)}. + */ @Deprecated public static DefaultJedisClientConfig copyConfig(JedisClientConfig copy) { Builder builder = builder(); diff --git a/src/main/java/redis/clients/jedis/JedisPooled.java b/src/main/java/redis/clients/jedis/JedisPooled.java index c3429319e7..dc7d231450 100644 --- a/src/main/java/redis/clients/jedis/JedisPooled.java +++ b/src/main/java/redis/clients/jedis/JedisPooled.java @@ -292,10 +292,12 @@ public JedisPooled(final GenericObjectPoolConfig poolConfig, final S } public JedisPooled(final GenericObjectPoolConfig poolConfig, final String host, int port, - final int connectionTimeout, final int soTimeout, final int infiniteSoTimeout, - final String user, final String password, final int database, final String clientName) { - this(new HostAndPort(host, port), DefaultJedisClientConfig.create(connectionTimeout, soTimeout, - infiniteSoTimeout, user, password, database, clientName, false, null, null, null, null), + final int connectionTimeout, final int soTimeout, final int infiniteSoTimeout, final String user, + final String password, final int database, final String clientName) { + this(new HostAndPort(host, port), + DefaultJedisClientConfig.builder().connectionTimeoutMillis(connectionTimeout).socketTimeoutMillis(soTimeout) + .blockingSocketTimeoutMillis(infiniteSoTimeout).user(user).password(password).database(database) + .clientName(clientName).build(), poolConfig); } @@ -304,9 +306,12 @@ public JedisPooled(final GenericObjectPoolConfig poolConfig, final S final String password, final int database, final String clientName, final boolean ssl, final SSLSocketFactory sslSocketFactory, final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) { - this(new HostAndPort(host, port), DefaultJedisClientConfig.create(connectionTimeout, soTimeout, - infiniteSoTimeout, user, password, database, clientName, ssl, sslSocketFactory, sslParameters, - hostnameVerifier, null), poolConfig); + this(new HostAndPort(host, port), + DefaultJedisClientConfig.builder().connectionTimeoutMillis(connectionTimeout).socketTimeoutMillis(soTimeout) + .blockingSocketTimeoutMillis(infiniteSoTimeout).user(user).password(password).database(database) + .clientName(clientName).ssl(ssl).sslSocketFactory(sslSocketFactory).sslParameters(sslParameters) + .hostnameVerifier(hostnameVerifier).build(), + poolConfig); } public JedisPooled(final URI uri) {