#10453 closed enhancement (fixed)
[PATCH] Add support for HttpOnly session cookies
| Reported by: | Owned by: | Remy Blank | |
|---|---|---|---|
| Priority: | normal | Milestone: | 1.0 |
| Component: | general | Version: | |
| Severity: | normal | Keywords: | patch |
| Cc: | Branch: | ||
| Release Notes: |
Enable the |
||
| API Changes: | |||
| Internal Changes: | |||
Description
To improve the session cookie security, allow creating cookies with HttpOnly flag (see: http://en.wikipedia.org/wiki/HTTP_cookie#HttpOnly_cookie)
Patch included - it can be successfully applied on both 0.12.x and trunk.
Attachments (3)
Change History (15)
by , 13 years ago
| Attachment: | httponly_cookies.patch added |
|---|
follow-up: 2 comment:1 by , 13 years ago
| Keywords: | patch added |
|---|---|
| Milestone: | → next-major-0.1X |
The Wikipedia article is funny:
This restriction mitigates but does not eliminate the threat of session cookie theft via Cross-site scripting.
"Mitigates but does not eliminate." So, is this really useful?
Still, thanks for the patch.
comment:2 by , 13 years ago
Replying to rblank:
The Wikipedia article is funny:
This restriction mitigates but does not eliminate the threat of session cookie theft via Cross-site scripting.
"Mitigates but does not eliminate." So, is this really useful?
Surely "reduced" risk is better than otherwise? Is there any reason not to apply the attribute (applies only to session cookies anyway)? I would like to see this added, especially as the patch is relatively trivial.
comment:3 by , 13 years ago
| Milestone: | next-major-0.1X → 0.13 |
|---|---|
| Owner: | set to |
Having learned a bit more about HTTP-only cookies, I think this patch is a good idea. It should probably even default to True.
One thing I'm not sure yet, should the httponly attribute be set on all cookies, or only the auth cookie?
comment:4 by , 13 years ago
I'm not sure if this matters, but apparently Python 2.5 did not support httponly. E.g. Django seems to jump through quite a few hoops because of this.
by , 13 years ago
| Attachment: | 10453-httponly-cookies-r11008.patch added |
|---|
Use httponly cookies by default.
comment:5 by , 13 years ago
Good catch, thanks! No, I don't want to go to that much trouble. Let's just activate the feature with 2.6 or higher.
10453-httponly-cookies-r11008.patch enhances httponly_cookies.patch by limiting the feature to Python ≥ 2.6, setting the attribute on all cookies, and enabling it by default. This should be fairly safe, and only sites using plugins that require access to Trac cookies from JavaScript should disable the feature.
Thoughts?
comment:6 by , 13 years ago
I wonder if there's any downside on having this setting enabled (performance or limitations for the TH:XmlRpcPlugin). If yes, they should be documented somewhere (at least mentioned briefly in the option doc). If there are no downsides, then perhaps we could just use this unconditionally and save the trouble for an additional option.
comment:7 by , 13 years ago
As far as performance is concerned, I see only the additional few bytes for the httponly flag in the cookies, so an increase of at most ~40 bytes in the responses. I would say this is negligible. I don't see how it could affect the th:XmlRpcPlugin, as it only affects JavaScript code, and that plugin doesn't have any.
The only possible downside that I can see is if some plugin actually wants to access any of the cookies in JavaScript for legitimate reasons (I couldn't find any reasons off the top of my head, but there may be), then that plugin would be broken.
I agree that it would be desirable to get rid of the option, so I'll research on trac-hacks to see if I can find any plugins accessing cookies from JavaScript.
by , 13 years ago
| Attachment: | 10453-httponly-cookies-unconditional-r11008.patch added |
|---|
Enable httponly cookie attribute unconditionally with Python ≥ 2.6.
comment:8 by , 13 years ago
| Release Notes: | modified (diff) |
|---|
There are no .js or .html files in all of trac-hacks that reference any of the cookies defined by Trac (trac_auth, trac_session, trac_form_token). So I would say it's pretty safe to enable the httponly attribute. 10453-httponly-cookies-unconditional-r11008.patch does that.
Ok to apply?
follow-up: 12 comment:11 by , 12 years ago
This project https://projects.developer.nokia.com/TracKanbanBoard relies on cookie visibility. Is there a config option to turn this off?
comment:12 by , 12 years ago
Replying to santeri.toikka@…:
This project https://projects.developer.nokia.com/TracKanbanBoard relies on cookie visibility. Is there a config option to turn this off?
The answer is in previous ticket comments: No, or better no more. Boolean option 'httponly_cookies' has been considered, but no use case has been found at that time.
Could you think of a way to re-factored TracKanbanBoard to not rely on the check of 'trac_auth' and 'trac_form_token' cookie values as indication for authentication status? If not, trac developers certainly have to consider applying the enhanced patch with configuration option instead of the simplified on, that eventually got applied and went into Trac 1.0.
Patch for adding httponly cookie support