66471 – JSessionId secure attribute missing with RemoteIpFilter and X-Forwarded-Proto set to https
Bug 66471 - JSessionId secure attribute missing with RemoteIpFilter and X-Forwarded-Proto set to https
Summary: JSessionId secure attribute missing with RemoteIpFilter and X-Forwarded-Proto...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 9.0.68
Hardware: PC All
: P2 critical (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-09 07:48 UTC by Reto Weiss
Modified: 2023-02-10 02:08 UTC (History)
0 users


Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Reto Weiss 2023-02-09 07:48:43 UTC 
I use the org.apache.catalina.filters.RemoteIpFilter Filter behind a NGINX reverse proxy. On the NGINX I set the http header X-Forwarded-Proto to https.

If I now make a request with a Browser to the reverse proxy the JSESSIONID cookie I get back is missing the secure attribute.

I have debugged the RemoteIpFilter, the isSecure flag of the wrapper request it creates, is correctly set to true. Unfortunately, the method getSession() or getSession(Boolean) is forwarded to the wrapped original request were the isSecure Flag is still not set. Therefore, the JSESSIONID cookie is missing the secure flag. See org.apache.catalina.connector.Request method doGetSession and org.apache.catalina.core.ApplicationSessionCookieConfig method createSessionCookie.

As workaround org.apache.catalina.valves.RemoteIpValve can be used, which seems to handle this correct. Also, the secure flag can be enforced by setting it in the web.xml.

However, I would like to use RemoteIpFilter because it has some advantages over the RemoteIpValve or statically setting it in the web.xml.
Comment 1 Han Li 2023-02-09 09:57:00 UTC 
I think I already know what bug is, but I haven't come up with a good solution yet. :|
Comment 2 Reto Weiss 2023-02-09 10:15:52 UTC 
The FilterChain could register the latest request in a ThreadLocal. Which is then read to use the isSecure flag from the most inner request when creating the session cookie.
Comment 3 Konstantin Kolinko 2023-02-09 12:57:45 UTC 
(In reply to Reto Weiss from comment #2)

Use of any ThreadLocal does not play well with asynchronous processing.

The information of "whether the request was submitted via a secure channel" belongs to the request, not to a specific thread.

I think that using Request.setAttibute() may be a way to go, if other more specific API is missing.

Alternatively, navigating up the wrapper chain via ServletRequestWrapper.getRequest().


Note the in org.apache.catalina.connector.Request:

1) Method Request.setSecure(boolean secure).

2) How Request.setAttribute(String name, Object value) is implemented, and use of SpecialAttributeAdapter there.
Comment 4 Han Li 2023-02-09 14:50:53 UTC 
(In reply to Konstantin Kolinko from comment #3)
> (In reply to Reto Weiss from comment #2)
> 
> Use of any ThreadLocal does not play well with asynchronous processing.
> 
> The information of "whether the request was submitted via a secure channel"
> belongs to the request, not to a specific thread.

+1
> 
> I think that using Request.setAttibute() may be a way to go, if other more
> specific API is missing.
> 
> Alternatively, navigating up the wrapper chain via
> ServletRequestWrapper.getRequest().
> 
> 
> Note the in org.apache.catalina.connector.Request:
> 
> 1) Method Request.setSecure(boolean secure).
> 
> 2) How Request.setAttribute(String name, Object value) is implemented, and
> use of SpecialAttributeAdapter there.

I haven't come up with a better solution than this, I've already implemented it according to this solution and commit, please review it for me, thanks!
Comment 5 Han Li 2023-02-10 02:08:50 UTC 
Fixed in:
- 11.0.x for 11.0.0-M3 onwards
- 10.1.x for 10.1.6 onwards
- 9.0.x for 9.0.72 onwards
- 8.5.x for 8.5.86 onwards