You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/08/01 16:36:29 UTC
[tomcat] branch main updated: Do not include sensitive headers in responses to HTTP TRACE requests
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 7c8a605bf8 Do not include sensitive headers in responses to HTTP TRACE requests
7c8a605bf8 is described below
commit 7c8a605bf86585ae7687a99ef473227007cd8f5e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Aug 1 17:36:18 2022 +0100
Do not include sensitive headers in responses to HTTP TRACE requests
This is a requirement of RFC 7231, 4.3.8
---
java/jakarta/servlet/http/HttpServlet.java | 22 ++++++++++++++++++----
test/jakarta/servlet/http/TestHttpServlet.java | 8 ++++++++
webapps/docs/changelog.xml | 4 ++++
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/java/jakarta/servlet/http/HttpServlet.java b/java/jakarta/servlet/http/HttpServlet.java
index ce85988633..16007586ed 100644
--- a/java/jakarta/servlet/http/HttpServlet.java
+++ b/java/jakarta/servlet/http/HttpServlet.java
@@ -25,7 +25,10 @@ import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.text.MessageFormat;
import java.util.Enumeration;
+import java.util.HashSet;
+import java.util.Locale;
import java.util.ResourceBundle;
+import java.util.Set;
import jakarta.servlet.AsyncEvent;
import jakarta.servlet.AsyncListener;
@@ -95,6 +98,8 @@ public abstract class HttpServlet extends GenericServlet {
private static final String LSTRING_FILE = "jakarta.servlet.http.LocalStrings";
private static final ResourceBundle lStrings = ResourceBundle.getBundle(LSTRING_FILE);
+ private static final Set<String> SENSITIVE_HTTP_HEADERS = new HashSet<>();
+
/**
* @deprecated May be removed in a future release
*
@@ -116,6 +121,12 @@ public abstract class HttpServlet extends GenericServlet {
*/
private volatile boolean cachedUseLegacyDoHead;
+ static {
+ SENSITIVE_HTTP_HEADERS.add("cookie");
+ SENSITIVE_HTTP_HEADERS.add("www-authenticate");
+ }
+
+
/**
* Does nothing, because this is an abstract class.
*/
@@ -634,10 +645,13 @@ public abstract class HttpServlet extends GenericServlet {
while (reqHeaderNames.hasMoreElements()) {
String headerName = reqHeaderNames.nextElement();
- Enumeration<String> headerValues = req.getHeaders(headerName);
- while (headerValues.hasMoreElements()) {
- String headerValue = headerValues.nextElement();
- buffer.append(CRLF).append(headerName).append(": ").append(headerValue);
+ // RFC 7231, 4.3.8 - skip 'sensitive' headers
+ if (!SENSITIVE_HTTP_HEADERS.contains(headerName.toLowerCase(Locale.ENGLISH))) {
+ Enumeration<String> headerValues = req.getHeaders(headerName);
+ while (headerValues.hasMoreElements()) {
+ String headerValue = headerValues.nextElement();
+ buffer.append(CRLF).append(headerName).append(": ").append(headerValue);
+ }
}
}
diff --git a/test/jakarta/servlet/http/TestHttpServlet.java b/test/jakarta/servlet/http/TestHttpServlet.java
index 3697960245..44174e009a 100644
--- a/test/jakarta/servlet/http/TestHttpServlet.java
+++ b/test/jakarta/servlet/http/TestHttpServlet.java
@@ -20,6 +20,7 @@ import java.io.IOException;
import java.io.PrintWriter;
import java.util.HashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import jakarta.servlet.AsyncContext;
@@ -315,6 +316,8 @@ public class TestHttpServlet extends TomcatBaseTest {
"Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
"X-aaa: a1, a2" + SimpleHttpClient.CRLF +
"X-aaa: a3" + SimpleHttpClient.CRLF +
+ "Cookie: c1-v1" + SimpleHttpClient.CRLF +
+ "WWW-Authenticate: not-a-real-credential" + SimpleHttpClient.CRLF +
SimpleHttpClient.CRLF});
client.setUseContentLength(true);
@@ -328,9 +331,14 @@ public class TestHttpServlet extends TomcatBaseTest {
Assert.assertTrue(client.getResponseLine(), client.isResponse200());
// Far from perfect but good enough
+ body = body.toLowerCase(Locale.ENGLISH);
Assert.assertTrue(body.contains("a1"));
Assert.assertTrue(body.contains("a2"));
Assert.assertTrue(body.contains("a3"));
+ // Sensitive headers (cookies, WWW-Authenticate) must not be reflected
+ // (since RFC 7231)
+ Assert.assertFalse(body.contains("cookie"));
+ Assert.assertFalse(body.contains("www-authenticate"));
client.disconnect();
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2c856cc4d3..d414f93bad 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -111,6 +111,10 @@
Correct handling of HTTP TRACE requests where there are multiple
instances of an HTTP header with the same name. (markt)
</fix>
+ <fix>
+ Implement the requirements of RFC 7231 and do not include sensitive
+ headers in responses to HTTP TRACE requests. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Do not include sensitive headers in responses to HTTP TRACE requests
Posted by Mark Thomas <ma...@apache.org>.
On 01/08/2022 18:03, Christopher Schultz wrote:
<snip/>
>> private volatile boolean cachedUseLegacyDoHead;
>> + static {
>> + SENSITIVE_HTTP_HEADERS.add("cookie");
>> + SENSITIVE_HTTP_HEADERS.add("www-authenticate");
>
> How about "Authorization"?
That makes more sense than WWW-Authenticate which is the challenge
rather than the response. I'll get that fixed.
> Is there a standard way for HTTP TRACE to reply to the client saying "oh
> and btw I removed the Cookie and Authentication headers you sent, so
> they aren't there but you did send them"?
Unfortunately not.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Do not include sensitive headers in responses to HTTP TRACE requests
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 8/1/22 12:36, markt@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/main by this push:
> new 7c8a605bf8 Do not include sensitive headers in responses to HTTP TRACE requests
> 7c8a605bf8 is described below
>
> commit 7c8a605bf86585ae7687a99ef473227007cd8f5e
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Mon Aug 1 17:36:18 2022 +0100
>
> Do not include sensitive headers in responses to HTTP TRACE requests
>
> This is a requirement of RFC 7231, 4.3.8
> ---
> java/jakarta/servlet/http/HttpServlet.java | 22 ++++++++++++++++++----
> test/jakarta/servlet/http/TestHttpServlet.java | 8 ++++++++
> webapps/docs/changelog.xml | 4 ++++
> 3 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/java/jakarta/servlet/http/HttpServlet.java b/java/jakarta/servlet/http/HttpServlet.java
> index ce85988633..16007586ed 100644
> --- a/java/jakarta/servlet/http/HttpServlet.java
> +++ b/java/jakarta/servlet/http/HttpServlet.java
> @@ -25,7 +25,10 @@ import java.lang.reflect.InvocationTargetException;
> import java.lang.reflect.Method;
> import java.text.MessageFormat;
> import java.util.Enumeration;
> +import java.util.HashSet;
> +import java.util.Locale;
> import java.util.ResourceBundle;
> +import java.util.Set;
>
> import jakarta.servlet.AsyncEvent;
> import jakarta.servlet.AsyncListener;
> @@ -95,6 +98,8 @@ public abstract class HttpServlet extends GenericServlet {
> private static final String LSTRING_FILE = "jakarta.servlet.http.LocalStrings";
> private static final ResourceBundle lStrings = ResourceBundle.getBundle(LSTRING_FILE);
>
> + private static final Set<String> SENSITIVE_HTTP_HEADERS = new HashSet<>();
> +
> /**
> * @deprecated May be removed in a future release
> *
> @@ -116,6 +121,12 @@ public abstract class HttpServlet extends GenericServlet {
> */
> private volatile boolean cachedUseLegacyDoHead;
>
> + static {
> + SENSITIVE_HTTP_HEADERS.add("cookie");
> + SENSITIVE_HTTP_HEADERS.add("www-authenticate");
How about "Authorization"?
Is there a standard way for HTTP TRACE to reply to the client saying "oh
and btw I removed the Cookie and Authentication headers you sent, so
they aren't there but you did send them"?
-chris
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org