HTTP/2: validate trailer header fields (#623)

Reject pseudo-headers in inbound and outbound trailers

Author: arturobernalg

httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java

@@ -1555,6 +1555,7 @@ public void endStream(final List<? extends Header> trailers) throws IOException
                 ensureNotClosed();
                 localClosed = true;
                 if (trailers != null && !trailers.isEmpty()) {
+                    TrailersValidationSupport.verify(trailers);
                     commitHeaders(id, trailers, true);
                 } else {
                     final RawFrame frame = frameFactory.createData(id, null, true);

httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamHandler.java

@@ -216,6 +216,7 @@ public void consumeHeader(final List<Header> headers, final boolean endStream) t
                 responseState.set(endStream ? MessageState.COMPLETE : MessageState.BODY);
                 break;
             case BODY:
+                TrailersValidationSupport.verify(headers);
                 responseState.set(MessageState.COMPLETE);
                 exchangeHandler.streamEnd(headers);
                 break;

httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/TrailersValidationSupport.java

@@ -0,0 +1,56 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.core5.http2.impl.nio;
+
+import java.util.List;
+
+import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http2.H2ConnectionException;
+import org.apache.hc.core5.http2.H2Error;
+
+
+@Internal
+final class TrailersValidationSupport {
+
+    static void verify(final List<? extends Header> trailers) throws H2ConnectionException {
+        if (trailers == null || trailers.isEmpty()) {
+            return;
+        }
+        for (int i = 0; i < trailers.size(); i++) {
+            final String name = trailers.get(i).getName();
+            if (name != null && !name.isEmpty() && name.charAt(0) == ':') {
+                throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Pseudo-header '" + name + "' is not allowed in trailers");
+            }
+        }
+    }
+
+
+    private TrailersValidationSupport() {
+    }
+
+}

httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestAbstractH2StreamMultiplexer.java

@@ -1078,6 +1078,27 @@ void testStreamIdleTimeoutTriggersH2StreamTimeoutException() throws Exception {
         Assertions.assertEquals(1, timeoutEx.getStreamId());
 
     }
+
+    @Test
+    void testOutboundTrailersWithPseudoHeaderRejected() throws Exception {
+        final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl(
+                protocolIOSession,
+                FRAME_FACTORY,
+                StreamIdGenerator.EVEN,
+                httpProcessor,
+                CharCodingConfig.DEFAULT,
+                H2Config.custom().build(),
+                h2StreamListener,
+                () -> streamHandler);
+
+        final H2StreamChannel channel = mux.createChannel(1);
+        mux.createStream(channel, streamHandler);
+
+        final List<Header> trailers = Arrays.asList(
+                new BasicHeader(":status", "200"));
+
+        Assertions.assertThrows(H2ConnectionException.class, () -> channel.endStream(trailers));
+    }
     private static byte[] encodeFrame(final RawFrame frame) throws IOException {
         final WritableByteChannelMock writableChannel = new WritableByteChannelMock(256);
         final FrameOutputBuffer outBuffer = new FrameOutputBuffer(16 * 1024);

httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientH2StreamHandler.java

@@ -27,15 +27,20 @@
 package org.apache.hc.core5.http2.impl.nio;
 
 import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
 
+import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.ProtocolException;
 import org.apache.hc.core5.http.impl.BasicHttpConnectionMetrics;
 import org.apache.hc.core5.http.impl.BasicHttpTransportMetrics;
+import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.http.nio.AsyncClientExchangeHandler;
 import org.apache.hc.core5.http.nio.AsyncPushConsumer;
 import org.apache.hc.core5.http.nio.HandlerFactory;
 import org.apache.hc.core5.http.protocol.HttpCoreContext;
 import org.apache.hc.core5.http.protocol.HttpProcessor;
+import org.apache.hc.core5.http2.H2ConnectionException;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
@@ -101,4 +106,27 @@ void toStringIncludesStates() {
         Assertions.assertTrue(text.contains("responseState"));
     }
 
+    @Test
+    void consumeTrailersWithPseudoHeaderRejected() throws Exception {
+        final H2StreamChannel channel = Mockito.mock(H2StreamChannel.class);
+        final HttpProcessor httpProcessor = Mockito.mock(HttpProcessor.class);
+        final BasicHttpConnectionMetrics metrics = new BasicHttpConnectionMetrics(
+                new BasicHttpTransportMetrics(), new BasicHttpTransportMetrics());
+        final AsyncClientExchangeHandler exchangeHandler = Mockito.mock(AsyncClientExchangeHandler.class);
+        @SuppressWarnings("unchecked") final HandlerFactory<AsyncPushConsumer> pushHandlerFactory =
+                (HandlerFactory<AsyncPushConsumer>) Mockito.mock(HandlerFactory.class);
+        final ClientH2StreamHandler handler = new ClientH2StreamHandler(
+                channel, httpProcessor, metrics, exchangeHandler, pushHandlerFactory, HttpCoreContext.create());
+
+        final List<Header> responseHeaders = Collections.singletonList(
+                new BasicHeader(":status", "200"));
+        handler.consumeHeader(responseHeaders, false);
+
+        final List<Header> trailers = Collections.singletonList(
+                new BasicHeader(":status", "200"));
+
+        Assertions.assertThrows(H2ConnectionException.class, () -> handler.consumeHeader(trailers, true));
+        Mockito.verify(exchangeHandler, Mockito.never()).streamEnd(Mockito.anyList());
+    }
+
 }

httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TrailersValidationSupportTest.java

@@ -0,0 +1,52 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.core5.http2.impl.nio;
+
+import java.util.Arrays;
+import java.util.Collections;
+
+import org.apache.hc.core5.http.message.BasicHeader;
+import org.apache.hc.core5.http2.H2ConnectionException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class TestTrailersValidationSupport {
+
+    @Test
+    void testVerifyInboundAcceptsRegularTrailers() throws Exception {
+        TrailersValidationSupport.verify(Arrays.asList(
+                new BasicHeader("checksum", "abc"),
+                new BasicHeader("x-foo", "bar")));
+    }
+
+    @Test
+    void testVerifyInboundRejectsPseudoHeaders() {
+        Assertions.assertThrows(H2ConnectionException.class, () ->
+                TrailersValidationSupport.verify(Collections.singletonList(
+                        new BasicHeader(":status", "200"))));
+    }
+}