From 3e41a7f2311312462b11c18518d2ef8816dd42f2 Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Fri, 30 Oct 2020 11:23:42 +0100
Subject: [PATCH] Enable header valication in HttpServerUpgradeHandler (#10643)

Motivation:

HttpServerUpgradeHandler takes a list of protocols from an incoming
request and uses them for building a response.
Although the class does some validation while parsing the list,
it then disables HTTP header validation when it builds a responst.
The disabled validation may potentially allow
HTTP response splitting attacks.

Modifications:

- Enabled HTTP header validation in HttpServerUpgradeHandler
  as a defense-in-depth measure to prevent possible
  HTTP response splitting attacks.
- Added a new constructor that allows disabling the validation.

Result:

HttpServerUpgradeHandler validates incoming protocols
before including them into a response.
That should prevent possible HTTP response splitting attacks.
---
 .../codec/http/HttpServerUpgradeHandler.java  | 22 ++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java
index 38e45e2595..9266a8ef77 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java
@@ -169,6 +169,7 @@ public class HttpServerUpgradeHandler extends HttpObjectAggregator {
 
     private final SourceCodec sourceCodec;
     private final UpgradeCodecFactory upgradeCodecFactory;
+    private final boolean validateHeaders;
     private boolean handlingUpgrade;
 
     /**
@@ -199,10 +200,25 @@ public class HttpServerUpgradeHandler extends HttpObjectAggregator {
      */
     public HttpServerUpgradeHandler(
             SourceCodec sourceCodec, UpgradeCodecFactory upgradeCodecFactory, int maxContentLength) {
+        this(sourceCodec, upgradeCodecFactory, maxContentLength, true);
+    }
+
+    /**
+     * Constructs the upgrader with the supported codecs.
+     *
+     * @param sourceCodec the codec that is being used initially
+     * @param upgradeCodecFactory the factory that creates a new upgrade codec
+     *                            for one of the requested upgrade protocols
+     * @param maxContentLength the maximum length of the content of an upgrade request
+     * @param validateHeaders validate the header names and values of the upgrade response.
+     */
+    public HttpServerUpgradeHandler(SourceCodec sourceCodec, UpgradeCodecFactory upgradeCodecFactory,
+                                    int maxContentLength, boolean validateHeaders) {
         super(maxContentLength);
 
         this.sourceCodec = requireNonNull(sourceCodec, "sourceCodec");
         this.upgradeCodecFactory = requireNonNull(upgradeCodecFactory, "upgradeCodecFactory");
+        this.validateHeaders = validateHeaders;
     }
 
     @Override
@@ -349,9 +365,9 @@ public class HttpServerUpgradeHandler extends HttpObjectAggregator {
     /**
      * Creates the 101 Switching Protocols response message.
      */
-    private static FullHttpResponse createUpgradeResponse(CharSequence upgradeProtocol) {
-        DefaultFullHttpResponse res = new DefaultFullHttpResponse(HTTP_1_1, SWITCHING_PROTOCOLS,
-                Unpooled.EMPTY_BUFFER, false);
+    private FullHttpResponse createUpgradeResponse(CharSequence upgradeProtocol) {
+        DefaultFullHttpResponse res = new DefaultFullHttpResponse(
+                HTTP_1_1, SWITCHING_PROTOCOLS, Unpooled.EMPTY_BUFFER, validateHeaders);
         res.headers().add(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE);
         res.headers().add(HttpHeaderNames.UPGRADE, upgradeProtocol);
         return res;