From 6ab43b08ff2e296ea4fa296d7db5b3186ca8d5b8 Mon Sep 17 00:00:00 2001
From: Stephane Landelle <slandelle@gatling.io>
Date: Tue, 28 Feb 2017 17:10:44 +0100
Subject: [PATCH] Add javadoc warning on SslContext#newHandler client-side
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Motivation:

SslContext#newHandler currently creates underlying SSLEngine without
enabling HTTPS endpointIdentificationAlgorithm. This behavior in
unsecured when used on the client side.

We can’t harden the behavior for now, as it would break existing
behavior, for example tests using self signed certificates.

Proper hardening will happen in a future major version when we can
break behavior.

Modifications:

Add javadoc warnings with code snippets.

Result:

Existing unsafe behavior and workaround documented.
---
 .../java/io/netty/handler/ssl/SslContext.java | 31 +++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/handler/src/main/java/io/netty/handler/ssl/SslContext.java b/handler/src/main/java/io/netty/handler/ssl/SslContext.java
index c707ed4029..b3b81012d8 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslContext.java
@@ -859,7 +859,21 @@ public abstract class SslContext {
      * Creates a new {@link SslHandler}.
      * <p>If {@link SslProvider#OPENSSL_REFCNT} is used then the returned {@link SslHandler} will release the engine
      * that is wrapped. If the returned {@link SslHandler} is not inserted into a pipeline then you may leak native
-     * memory!
+     * memory!</p>
+     * <p><b>Beware</b>: the underlying generated {@link SSLEngine} won't have
+     * <a href="https://wiki.openssl.org/index.php/Hostname_validation">hostname verification</a> enabled by default.
+     * If you create {@link SslHandler} for the client side and want proper security, we advice that you configure
+     * the {@link SSLEngine} (see {@link javax.net.ssl.SSLParameters#setEndpointIdentificationAlgorithm(String)}):</p>
+     * <pre>
+     * SSLEngine sslEngine = sslHandler.engine();
+     * SSLParameters sslParameters = sslEngine.getSSLParameters();
+     * // only available since Java 7
+     * sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
+     * sslEngine.setSSLParameters(sslParameters);
+     * </pre>
+     *
+     * @param alloc If supported by the SSLEngine then the SSLEngine will use this to allocate ByteBuf objects.
+     *
      * @return a new {@link SslHandler}
      */
     public final SslHandler newHandler(ByteBufAllocator alloc) {
@@ -870,7 +884,20 @@ public abstract class SslContext {
      * Creates a new {@link SslHandler} with advisory peer information.
      * <p>If {@link SslProvider#OPENSSL_REFCNT} is used then the returned {@link SslHandler} will release the engine
      * that is wrapped. If the returned {@link SslHandler} is not inserted into a pipeline then you may leak native
-     * memory!
+     * memory!</p>
+     * <p><b>Beware</b>: the underlying generated {@link SSLEngine} won't have
+     * <a href="https://wiki.openssl.org/index.php/Hostname_validation">hostname verification</a> enabled by default.
+     * If you create {@link SslHandler} for the client side and want proper security, we advice that you configure
+     * the {@link SSLEngine} (see {@link javax.net.ssl.SSLParameters#setEndpointIdentificationAlgorithm(String)}):</p>
+     * <pre>
+     * SSLEngine sslEngine = sslHandler.engine();
+     * SSLParameters sslParameters = sslEngine.getSSLParameters();
+     * // only available since Java 7
+     * sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
+     * sslEngine.setSSLParameters(sslParameters);
+     * </pre>
+     *
+     * @param alloc If supported by the SSLEngine then the SSLEngine will use this to allocate ByteBuf objects.
      * @param peerHost the non-authoritative name of the host
      * @param peerPort the non-authoritative port
      *