Issue with cookie parsing in Trac 1.6 and Python 3

[Chris Shelton]

After working through a plan to upgrade an old Trac installation from version 0.12 to 1.6, due to the need to shift to python 3, I believe I have discovered an issue with how Python 3 is parsing cookies when they are "invalid" cookies present for a session.  

First of all, I have made a small change to the code of trac/web/auth.py that seems to work for my environment where the entire Trac installation requires authentication to access.  Trac receives the REMOTE_USER information from wsgi, but the trac_auth cookie may not be set on a user's initial login.  My code change is in the authenticate function:

--- auth.py.orig        2025-03-04 17:46:22.942392186 -0500

+++ auth.py     2025-03-05 16:59:47.074857014 -0500
@@ -91,13 +91,28 @@

     def authenticate(self, req):
         authname = None
+        # following added by cshelton - 1/22/2025
+        self.env.log.debug("in authenticate function")
         if req.remote_user:
             authname = req.remote_user
+            self.env.log.debug("REMOTE_USER is copied to authname as %s", authname)
+            self.env.log.debug("request cookies are %s", req.incookie.output)
+            if not 'trac_auth' in req.incookie:
+                self.env.log.debug("no trac_auth cookie found, so empty authname and return None")
+                return None
+
         elif 'trac_auth' in req.incookie:
             authname = self._get_name_for_cookie(req,
                                                  req.incookie['trac_auth'])
+            self.env.log.debug("found trac_auth cookie")

         if not authname:
+            self.env.log.debug("authname not set, returning None")
             return None

         if self.ignore_case:

This change causes the authenticate function to return None if the remote_user is set, but there is not a trac_auth cookie present.  This triggers the noanonymousplugin to redirect the request to /trac/login, to set the trac_auth cookie, then redirects back to the page the user was trying to access.  

This setup has been working well for several years with my Trac 0.12 installation, but has been inconsistent with Trac 1.6.  After much frustration, I finally discovered this issue reported by the Django project:

https://code.djangoproject.com/ticket/26158

The issue that I am having is very similar to this one, in that there are several cookies regularly set by a large ERP system used by my organization that are presented to my Trac installation.  There is at least one invalid or unnamed cookie set at a higher level (.iu.edu), while my Trac installation runs at a subdomain (.prvt.controller.iu.edu).  

When I remove the cookies for the .iu.edu domain, or when running Trac in an incognito window, it behaves normally.  But when the .iu.edu cookies are present, Trac (or really Python) fails to parse any cookies at all, leading to a redirect loop when trying to initially login.  The Django project ticket noted that python 3 will fail to parse any cookies at all when only one invalid cookie is found.  Their workaround for this issue was to write their own cookie parsing routine, and remove references to SimpleCookie and BaseCookie:

https://github.com/django/django/commit/93a135d111c2569d88d65a3f4ad9e6d9ad291452

Would it be possible to implement a similar fix for Trac to manually parse cookies instead of relying on the python3 cookie code that seems to discard all cookies if only one cookie is "bad"?  I am tempted to try to implement a similar change in Trac as the Django project, but my python coding skills need some improvement.  

Thanks for any advice regarding how to best implement a fix for this issue.

Chris

[Jun Omae]

Hi,

> https://code.djangoproject.com/ticket/26158 <https://code.djangoproject.com/ticket/26158>
> The issue that I am having is very similar to this one, in that there are several cookies regularly set by a large ERP system used by my organization that are presented to my Trac installation.  There is at least one invalid or unnamed cookie set at a higher level (.iu.edu <http://iu.edu>), while my Trac installation runs at a subdomain (.prvt.controller.iu.edu <http://prvt.controller.iu.edu>).  
>
> When I remove the cookies for the .iu.edu <http://iu.edu> domain, or when running Trac in an incognito window, it behaves normally.  But when the .iu.edu <http://iu.edu> cookies are present, Trac (or really Python) fails to parse any cookies at all, leading to a redirect loop when trying to initially login.  The Django project ticket noted that python 3 will fail to parse any cookies at all when only one invalid cookie is found.  Their workaround for this issue was to write their own cookie parsing routine, and remove references to SimpleCookie and BaseCookie:
> https://github.com/django/django/commit/93a135d111c2569d88d65a3f4ad9e6d9ad291452 <https://github.com/django/django/commit/93a135d111c2569d88d65a3f4ad9e6d9ad291452>

>
> Would it be possible to implement a similar fix for Trac to manually parse cookies instead of relying on the python3 cookie code that seems to discard all cookies if only one cookie is "bad"?  I am tempted to try to implement a similar change in Trac as the Django project, but my python coding skills need some improvement.  
>
> Thanks for any advice regarding how to best implement a fix for this issue.
>
> Chris

Thanks for the investigating.

I tested how Google Chrome and Firefox behave with such unnamed cookies: the browsers don't ignore such cookies, but send them to the server.

I think this is an issue of http.cookies in Python, but it doesn't look like it will be fixed soon.... I consider that Trac should add work around for it.

Could you please create new ticket for the issue on https://trac.edgewall.org?

Quick fix:

[[[
diff --git a/trac/web/api.py b/trac/web/api.py
index 7f8b59bdc..fa888f0b0 100644
--- a/trac/web/api.py
+++ b/trac/web/api.py
@@ -612,7 +612,13 @@ class RequestDone(TracBaseError):


class Cookie(SimpleCookie):
+
+ _separator_re = re.compile(r'\s*;\s*', re.ASCII)
+
def load(self, rawdata, ignore_parse_errors=False):
+ # Remove unnamed cookies
+ rawdata = '; '.join(item for item in self._separator_re.split(rawdata)
+ if '=' in item)
if ignore_parse_errors:
self.bad_cookies = []
self._BaseCookie__set = self._loose_set
]]]

--
Jun Omae <jun...@gmail.com> (大前 潤)

[Chris Shelton]

Jun,

Thank you for your response.  I just tried your quick fix, and unfortunately I am still seeing the login redirection errors when I have the additional cookies present from the higher level domain. 

I think that python3 SimpleCookie and/or BaseCookie are also failing to parse anything when any cookie has an invalid character present, such as a double quote, comma, semicolon or backslash.  Would it be possible to rework the actual cookie parsing code to avoid the use of SimpleCookie or BaseCookie for processing cookies received from the browser?

Chris

--
You received this message because you are subscribed to the Google Groups "Trac Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to trac-users+...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/trac-users/3f6248a8-8519-4b40-ad14-8c5733144c18%40gmail.com.

[Jun Omae]

On 2025/03/08 0:02, Chris Shelton wrote:
> Jun,
>
> Thank you for your response.  I just tried your quick fix, and unfortunately I am still seeing the login redirection errors when I have the additional cookies present from the higher level domain. 
>
> I think that python3 SimpleCookie and/or BaseCookie are also failing to parse anything when any cookie has an invalid character present, such as a double quote, comma, semicolon or backslash.  Would it be possible to rework the actual cookie parsing code to avoid the use of SimpleCookie or BaseCookie for processing cookies received from the browser?
>
> Chris

Could you please share your cookies, masking any sensitive information such as session ids? I'd like to clarify what's causing the issue.

According your investigation, you said that it caused by the unnamed cookies. So that, the patch tries to remove such unnamed cookies, but it doesn't seem to be caused the issue.

[Chris Shelton]

Jun,

After some additional review of the cookies in my environment, I believe that the presence of forward slashes or spaces in the values of cookies are what is causing the issue that I am seeing.  When I manually remove each cookie that contained any spaces or forward slashes in the value of the cookie from my browser session, the login redirect loop stops and Trac behaves normally, until these cookies come back via accessing a university wide ERP system.  

The attached text file contains a slightly obfuscated set of my cookies, with several of the values replaced with X's.  I believe that any cookie with a value that contains any of the "invalid characters" for cookie names, as mentioned here https://docs.python.org/3/library/http.cookies.html is causing this failure to parse any of the cookies in my session.

In case it helps, my Trac environment will be running on the domain of test.prvt.controller.iu.edu.  The cookies from our ERP system all have a domain of .iu.edu.  Lastly, I have just opened https://trac.edgewall.org/ticket/13876 for this issue.  Thanks again for your help!

Chris

--
You received this message because you are subscribed to the Google Groups "Trac Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to trac-users+...@googlegroups.com.

To view this discussion visit https://groups.google.com/d/msgid/trac-users/e996f187-3158-4b0b-a3cb-7a2c875b5005%40gmail.com.

[Jun Omae]

Thanks for the submitting this issue on trac.edgewall.org.


Investigating your cookies, this is caused by whitespaces in the value of the cookies, not unnamed cookies. Also, the issue has been filed at https://github.com/python/cpython/issues/75637 7 years ago, but still not fixed.

Could you please try the following workaround?

[[[

diff --git a/trac/web/api.py b/trac/web/api.py
index 7f8b59bdc..f2b9b4519 100644
--- a/trac/web/api.py
+++ b/trac/web/api.py
@@ -612,15 +612,20 @@ class RequestDone(TracBaseError):

 class Cookie(SimpleCookie):
+
+    _separator_re = re.compile(r'\s*;\s*', re.ASCII)
+
     def load(self, rawdata, ignore_parse_errors=False):

         if ignore_parse_errors:
             self.bad_cookies = []
             self._BaseCookie__set = self._loose_set

-        SimpleCookie.load(self, rawdata)
-        if ignore_parse_errors:
+            for item in self._separator_re.split(rawdata):
+                super().load(item)
             self._BaseCookie__set = self._strict_set
             for key in self.bad_cookies:
                 del self[key]
+        else:
+            super().load(rawdata)

     _strict_set = BaseCookie._BaseCookie__set

]]]

[Chris Shelton]

Jun,

This revised workaround seems to fix the issue that I was having, at least in my initial testing.  I will also add a comment on the ticket to indicate that this fix is working for me.  

Chris

--

You received this message because you are subscribed to the Google Groups "Trac Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to trac-users+...@googlegroups.com.

To view this discussion visit https://groups.google.com/d/msgid/trac-users/8c3b2c38-b0c9-4802-ad76-b0d947fcf3d7n%40googlegroups.com.