From 83e0339d27d9b60c797af53cc1bf7c03c954e16d Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sat, 23 Sep 2023 20:52:00 +0200 Subject: [PATCH] HTTPCLIENT-2293 - Implement RFC-compliant TRACE request interceptor (#486) - Add RequestTraceInterceptor class to handle HTTP TRACE requests in compliance with RFC 7231, Section 4.3.8. - Throw ProtocolException for sensitive headers like 'Authorization' and 'Cookie' in TRACE requests. - Throw ProtocolException if TRACE request contains a body. --- .../http/impl/classic/HttpClientBuilder.java | 3 + .../protocol/RequestTraceInterceptor.java | 161 ++++++++++++++++++ .../protocol/TestRequestTraceInterceptor.java | 95 +++++++++++ 3 files changed, 259 insertions(+) create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestTraceInterceptor.java create mode 100644 httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestTraceInterceptor.java diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java index b2f41dfeb..0999f9d28 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java @@ -77,6 +77,7 @@ import org.apache.hc.client5.http.io.HttpClientConnectionManager; import org.apache.hc.client5.http.protocol.RedirectStrategy; import org.apache.hc.client5.http.protocol.RequestAddCookies; +import org.apache.hc.client5.http.protocol.RequestTraceInterceptor; import org.apache.hc.client5.http.protocol.RequestClientConnControl; import org.apache.hc.client5.http.protocol.RequestDefaultHeaders; import org.apache.hc.client5.http.protocol.RequestExpectContinue; @@ -820,6 +821,8 @@ public CloseableHttpClient build() { new RequestClientConnControl(), new RequestUserAgent(userAgentCopy), new RequestExpectContinue(), + new RequestTraceInterceptor(), + new RequestExpectContinue(), new RequestIfRange()); if (!cookieManagementDisabled) { b.add(RequestAddCookies.INSTANCE); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestTraceInterceptor.java b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestTraceInterceptor.java new file mode 100644 index 000000000..27e3b089e --- /dev/null +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestTraceInterceptor.java @@ -0,0 +1,161 @@ +/* + * ==================================================================== + * 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 + * . + * + */ + +package org.apache.hc.client5.http.protocol; + +import java.io.IOException; + +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.ThreadingBehavior; +import org.apache.hc.core5.http.EntityDetails; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpRequestInterceptor; +import org.apache.hc.core5.http.Method; +import org.apache.hc.core5.http.ProtocolException; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.util.Args; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + *

RequestTraceInterceptor

+ * + *

This class serves as an interceptor for HTTP TRACE requests, ensuring they adhere to specific security and protocol guidelines.

+ * + *

Responsibilities:

+ * + * + *

Thread Safety: This class is stateless and therefore thread-safe, as indicated by its {@code ThreadingBehavior.STATELESS} annotation.

+ * + *

Interceptor Behavior:

+ * + * + * @version 5.4 + * @see HttpRequestInterceptor + * @see HttpException + * @see IOException + * @see ProtocolException + * @see Method#TRACE + * @see HttpHeaders#AUTHORIZATION + * @see HttpHeaders#COOKIE + *//** + *

RequestTraceInterceptor

+ * + *

This class serves as an interceptor for HTTP TRACE requests, ensuring they adhere to specific security and protocol guidelines.

+ * + *

Responsibilities:

+ * + * + *

Thread Safety: This class is stateless and therefore thread-safe, as indicated by its {@code ThreadingBehavior.STATELESS} annotation.

+ * + *

Interceptor Behavior:

+ * + * + * @version 5.4 + * @see HttpRequestInterceptor + * @see HttpException + * @see IOException + * @see ProtocolException + * @see Method#TRACE + * @see HttpHeaders#AUTHORIZATION + * @see HttpHeaders#COOKIE + */ +@Contract(threading = ThreadingBehavior.STATELESS) +public class RequestTraceInterceptor implements HttpRequestInterceptor { + + private static final Logger LOG = LoggerFactory.getLogger(RequestTraceInterceptor.class); + + /** + * Singleton instance of {@link RequestTraceInterceptor}. + */ + public static final HttpRequestInterceptor INSTANCE = new RequestTraceInterceptor(); + + /** + * Default constructor. + */ + public RequestTraceInterceptor() { + super(); + } + + /** + * Processes an incoming HTTP request. If the request is of type TRACE, it performs the following actions: + * + * + * @param request The incoming HTTP request. Cannot be {@code null}. + * @param entity Details of the request entity. Can be {@code null}. + * @param context The HTTP context. + * @throws HttpException If a protocol error occurs. + * @throws IOException If an I/O error occurs. + */ + @Override + public void process(final HttpRequest request, final EntityDetails entity, final HttpContext context) + throws HttpException, IOException { + + Args.notNull(request, "HTTP request"); + Args.notNull(context, "HTTP context"); + + // Check if the request method is TRACE + if (Method.TRACE.isSame(request.getMethod())) { + + // A client MUST NOT send content in a TRACE request. + if (entity != null) { + throw new ProtocolException("TRACE request MUST NOT contain a request body."); + } + + // Check for sensitive headers + final Header authHeader = request.getHeader(HttpHeaders.AUTHORIZATION); + if (authHeader != null) { + throw new ProtocolException("TRACE request MUST NOT contain an Authorization header."); + } + + // Check for cookies + final Header cookieHeader = request.getHeader(HttpHeaders.COOKIE); + if (cookieHeader != null) { + throw new ProtocolException("TRACE request MUST NOT contain a Cookie header."); + } + } + } +} diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestTraceInterceptor.java b/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestTraceInterceptor.java new file mode 100644 index 000000000..43705650d --- /dev/null +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestTraceInterceptor.java @@ -0,0 +1,95 @@ +/* + * ==================================================================== + * 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 + * . + * + */ + +package org.apache.hc.client5.http.protocol; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; + +import org.apache.hc.core5.http.EntityDetails; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.ProtocolException; +import org.apache.hc.core5.http.impl.BasicEntityDetails; +import org.apache.hc.core5.http.message.BasicHttpRequest; +import org.apache.hc.core5.http.protocol.BasicHttpContext; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class TestRequestTraceInterceptor { + + private RequestTraceInterceptor interceptor; + private HttpRequest request; + private HttpContext context; + + @BeforeEach + void setUp() { + interceptor = new RequestTraceInterceptor(); + context = new BasicHttpContext(); + } + + @Test + void testTraceRequestWithoutSensitiveHeaders() throws HttpException, IOException { + request = new BasicHttpRequest("TRACE", "/"); + interceptor.process(request, null, context); + assertNull(request.getHeader(HttpHeaders.AUTHORIZATION)); + } + + @Test + void testTraceRequestWithSensitiveHeaders() { + request = new BasicHttpRequest("TRACE", "/"); + request.setHeader(HttpHeaders.AUTHORIZATION, "Bearer token"); + assertThrows(ProtocolException.class, () -> interceptor.process(request, null, context)); + } + + @Test + void testTraceRequestWithBody() { + request = new BasicHttpRequest("TRACE", "/"); + final EntityDetails entity = new BasicEntityDetails(10, null); + assertThrows(ProtocolException.class, () -> interceptor.process(request, entity, context)); + } + + @Test + void testNonTraceRequest() throws HttpException, IOException { + request = new BasicHttpRequest("GET", "/"); + request.setHeader(HttpHeaders.AUTHORIZATION, "Bearer token"); + interceptor.process(request, null, context); + assertNotNull(request.getHeader(HttpHeaders.AUTHORIZATION)); + } + + @Test + void testTraceRequestWithCookieHeader() { + request = new BasicHttpRequest("TRACE", "/"); + request.setHeader(HttpHeaders.COOKIE, "someCookie=someValue"); + assertThrows(ProtocolException.class, () -> interceptor.process(request, null, context)); + } +}