SSL connections should support hostname verification out of the box

[maxant]

Expected behavior

When creating a connection using any constructor that does not take a hostname verifier, a default one should be used which actually verifies the hostname, to help avoid man in the middle attacks.

Actual behavior

Currently, no hostname verification is done, leading to the impression that actually everything is fine, when in reality there is a security weakness.

Steps to reproduce:

set up a redis node with TLS (normal as opposed to mutual TLS is sufficient), as shown below, and deploy it to the domain and then connect to it:

        JedisPooled jedis = new JedisPooled(new GenericObjectPoolConfig<>(), "something.else.at.same.ip", exposedPort, 10_000, 10_000, user, pass,
                0, clientName, true);
    System.out.println(jedis.keys("*");

The above code throws no errors. Using a hostname verifier like the one from okhttp3 however ensures that the domain name in the certificate actually matches the domain name that is requested.

Redis / Jedis Configuration

docker run -it --rm \
    --name redis \
    --network mynetwork \
    -v "/etc/letsencrypt/live/somedomain.ch/privkey.pem:/certs/server.key:ro" \
    -v "/etc/letsencrypt/live/somedomain.ch/fullchain.pem:/certs/server.crt:ro" \
    -v "/z/data:/data:rw" \
    --memory 300m --memory-swap 400m --cpus="2.0" \
    -e REDIS_ARGS="--loglevel notice --maxclients 200 --maxmemory 200m --maxmemory-policy noeviction --appendonly yes --appendfsync always --tls-port 6380 --tls-auth-clients no --tls-cert-file /certs/server.crt --tls-key-file /certs/server.key --requirepass asdfasdfasdfasdf" \
    redis/redis-stack:7.0.6-RC3

Jedis version:

4.3.1

Redis version:

7.0.6-RC3

Java version:

graalvm 17

[maxant]

example of a hostname verifier that works:

https://github.com/square/okhttp/blob/58ee1ce170c2aea75a113012956260873e808dad/okhttp/src/jvmMain/kotlin/okhttp3/internal/tls/OkHostnameVerifier.kt

[sazzad16]

@maxant Apologies for the late reply.

We may not address such change as this would break many existing applications. Also, not even in our next major release considering:

1. This would add another dependency.

2. Simpler constructors help novice users to kick-start their learning.

   Advanced and concerned users have the option to use constructors with more parameters to secure and improve their application.

[maxant]

Unfortunately I don't agree. Hostname verification is an industry standard and if you use TLS, you simply expect this to work out of the box. If you don't verify the hostname, you leave the client open to man in the middle attacks. Such a relaxed attitude to security is damaging.

At the very least I would ask you to provide a warning log or some good clear documentation that tells your library users that they are not properly protected.

As for the dependency, that is not true, if you implement the relatively simple code as part of your library. It wouldn't make sense to include okhttp as a dependency.

[github-actions[bot]]

This issue is marked stale. It will be closed in 30 days if it is not updated.