File tomcat-9.0.36-CVE-2023-28708.patch of Package tomcat.28365
From 3b51230764da595bb19e8d0962dd8c69ab40dfab Mon Sep 17 00:00:00 2001
From: lihan <lihan@apache.org>
Date: Fri, 10 Feb 2023 10:01:27 +0800
Subject: [PATCH] Fix BZ 66471 - JSessionId secure attribute missing with
RemoteIpFilter and X-Forwarded-Proto set to https
https://bz.apache.org/bugzilla/show_bug.cgi?id=66471
---
java/org/apache/catalina/Globals.java | 7 ++
.../apache/catalina/connector/Request.java | 14 +++
.../catalina/filters/RemoteIpFilter.java | 7 +-
.../catalina/filters/TestRemoteIpFilter.java | 96 ++++++++++++++-----
webapps/docs/changelog.xml | 5 +
5 files changed, 100 insertions(+), 29 deletions(-)
Index: apache-tomcat-9.0.36-src/java/org/apache/catalina/Globals.java
===================================================================
--- apache-tomcat-9.0.36-src.orig/java/org/apache/catalina/Globals.java
+++ apache-tomcat-9.0.36-src/java/org/apache/catalina/Globals.java
@@ -113,6 +113,13 @@ public final class Globals {
/**
+ * The request attribute that is set to the value of {@code Boolean.TRUE}
+ * if {@link org.apache.catalina.filters.RemoteIpFilter} determines
+ * that this request was submitted via a secure channel.
+ */
+ public static final String REMOTE_IP_FILTER_SECURE = "org.apache.catalina.filters.RemoteIpFilter.secure";
+
+ /**
* The request attribute that can be used by a servlet to pass
* to the connector the name of the file that is to be served
* by sendfile. The value should be {@code java.lang.String}
Index: apache-tomcat-9.0.36-src/java/org/apache/catalina/connector/Request.java
===================================================================
--- apache-tomcat-9.0.36-src.orig/java/org/apache/catalina/connector/Request.java
+++ apache-tomcat-9.0.36-src/java/org/apache/catalina/connector/Request.java
@@ -3548,6 +3548,20 @@ public class Request implements HttpServ
// NO-OP
}
});
+ specialAttributes.put(Globals.REMOTE_IP_FILTER_SECURE,
+ new SpecialAttributeAdapter() {
+ @Override
+ public Object get(Request request, String name) {
+ return Boolean.valueOf(request.isSecure());
+ }
+
+ @Override
+ public void set(Request request, String name, Object value) {
+ if (value instanceof Boolean) {
+ request.setSecure(((Boolean) value).booleanValue());
+ }
+ }
+ });
specialAttributes.put(Globals.STREAM_ID,
new SpecialAttributeAdapter() {
@Override
Index: apache-tomcat-9.0.36-src/java/org/apache/catalina/filters/RemoteIpFilter.java
===================================================================
--- apache-tomcat-9.0.36-src.orig/java/org/apache/catalina/filters/RemoteIpFilter.java
+++ apache-tomcat-9.0.36-src/java/org/apache/catalina/filters/RemoteIpFilter.java
@@ -572,11 +572,6 @@ public class RemoteIpFilter extends Gene
return serverPort;
}
- @Override
- public boolean isSecure() {
- return secure;
- }
-
public void removeHeader(String name) {
Map.Entry<String, List<String>> header = getHeaderEntry(name);
if (header != null) {
@@ -616,7 +611,7 @@ public class RemoteIpFilter extends Gene
}
public void setSecure(boolean secure) {
- this.secure = secure;
+ super.getRequest().setAttribute(Globals.REMOTE_IP_FILTER_SECURE, Boolean.valueOf(secure));
}
public void setServerName(String serverName) {
Index: apache-tomcat-9.0.36-src/test/org/apache/catalina/filters/TestRemoteIpFilter.java
===================================================================
--- apache-tomcat-9.0.36-src.orig/test/org/apache/catalina/filters/TestRemoteIpFilter.java
+++ apache-tomcat-9.0.36-src/test/org/apache/catalina/filters/TestRemoteIpFilter.java
@@ -82,15 +82,21 @@ public class TestRemoteIpFilter extends
private static final long serialVersionUID = 1L;
- private transient HttpServletRequest request;
-
- public HttpServletRequest getRequest() {
- return request;
- }
+ public String remoteAddr;
+ public String remoteHost;
+ public String scheme;
+ public String serverName;
+ public int serverPort;
+ public boolean isSecure;
@Override
public void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
- this.request = request;
+ this.isSecure = request.isSecure();
+ this.remoteAddr = request.getRemoteAddr();
+ this.remoteHost = request.getRemoteHost();
+ this.scheme = request.getScheme();
+ this.serverName = request.getServerName();
+ this.serverPort = request.getServerPort();
PrintWriter writer = response.getWriter();
writer.println("request.remoteAddr=" + request.getRemoteAddr());
@@ -130,16 +136,6 @@ public class TestRemoteIpFilter extends
}
@Override
- public void setAttribute(String name, Object value) {
- getCoyoteRequest().getAttributes().put(name, value);
- }
-
- @Override
- public Object getAttribute(String name) {
- return getCoyoteRequest().getAttributes().get(name);
- }
-
- @Override
public String getServerName() {
return "localhost";
}
@@ -770,16 +766,70 @@ public class TestRemoteIpFilter extends
// VALIDATE
Assert.assertEquals(HttpURLConnection.HTTP_OK, httpURLConnection.getResponseCode());
- HttpServletRequest request = mockServlet.getRequest();
- Assert.assertNotNull(request);
// VALIDATE X-FORWARDED-FOR
- Assert.assertEquals(expectedRemoteAddr, request.getRemoteAddr());
- Assert.assertEquals(expectedRemoteAddr, request.getRemoteHost());
+ Assert.assertEquals(expectedRemoteAddr, mockServlet.remoteAddr);
+ Assert.assertEquals(expectedRemoteAddr, mockServlet.remoteHost);
// VALIDATE X-FORWARDED-PROTO
- Assert.assertTrue(request.isSecure());
- Assert.assertEquals("https", request.getScheme());
- Assert.assertEquals(443, request.getServerPort());
+ Assert.assertTrue(mockServlet.isSecure);
+ Assert.assertEquals("https", mockServlet.scheme);
+ Assert.assertEquals(443, mockServlet.serverPort);
+ }
+
+ @Test
+ public void testJSessionIdSecureAttributeMissing() throws Exception {
+
+ // mostly default configuration : enable "x-forwarded-proto"
+ Map<String, String> remoteIpFilterParameter = new HashMap<>();
+ remoteIpFilterParameter.put("protocolHeader", "x-forwarded-proto");
+
+ // SETUP
+ Tomcat tomcat = getTomcatInstance();
+ Context root = tomcat.addContext("", TEMP_DIR);
+
+ FilterDef filterDef = new FilterDef();
+ filterDef.getParameterMap().putAll(remoteIpFilterParameter);
+ filterDef.setFilterClass(RemoteIpFilter.class.getName());
+ filterDef.setFilterName(RemoteIpFilter.class.getName());
+
+ root.addFilterDef(filterDef);
+
+ FilterMap filterMap = new FilterMap();
+ filterMap.setFilterName(RemoteIpFilter.class.getName());
+ filterMap.addURLPatternDecoded("*");
+ root.addFilterMap(filterMap);
+
+ Bug66471Servlet bug66471Servlet = new Bug66471Servlet();
+
+ Tomcat.addServlet(root, bug66471Servlet.getClass().getName(), bug66471Servlet);
+ root.addServletMappingDecoded("/test", bug66471Servlet.getClass().getName());
+
+ getTomcatInstance().start();
+
+ Map<String, List<String>> resHeaders = new HashMap<>();
+ Map<String, List<String>> reqHeaders = new HashMap<>();
+ String expectedRemoteAddr = "my-remote-addr";
+ List<String> forwardedFor = new ArrayList<>(1);
+ forwardedFor.add(expectedRemoteAddr);
+ List<String> forwardedProto = new ArrayList<>(1);
+ forwardedProto.add("https");
+ reqHeaders.put("x-forwarded-for", forwardedFor);
+ reqHeaders.put("x-forwarded-proto", forwardedProto);
+
+ getUrl("http://localhost:" + tomcat.getConnector().getLocalPort() +
+ "/test", null, reqHeaders, resHeaders);
+ String setCookie = resHeaders.get("Set-Cookie").get(0);
+ Assert.assertTrue(setCookie.contains("Secure"));
+ Assert.assertTrue(bug66471Servlet.isSecure.booleanValue());
+ }
+ public static class Bug66471Servlet extends HttpServlet {
+ private static final long serialVersionUID = 1L;
+ public Boolean isSecure;
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+ req.getSession();
+ isSecure = (Boolean) req.getAttribute(Globals.REMOTE_IP_FILTER_SECURE);
+ }
}
}
Index: apache-tomcat-9.0.36-src/webapps/docs/changelog.xml
===================================================================
--- apache-tomcat-9.0.36-src.orig/webapps/docs/changelog.xml
+++ apache-tomcat-9.0.36-src/webapps/docs/changelog.xml
@@ -103,6 +103,11 @@
Make the calculation of the session storage location more robust when
using file based persistent storage. (markt)
</fix>
+ <fix>
+ <bug>66471</bug>: Fix JSessionId secure attribute missing When
+ <code>RemoteIpFilter</code> determines that this request was submitted
+ via a secure channel. (lihan)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
