Edgewall Software
Modify

Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#10453 closed enhancement (fixed)

[PATCH] Add support for HttpOnly session cookies

Reported by: juha.mustonen@… Owned by: Remy Blank
Priority: normal Milestone: 1.0
Component: general Version:
Severity: normal Keywords: patch
Cc: Branch:
Release Notes:

Enable the httponly attribute on all cookies (for Python ≥ 2.6).

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)

httponly_cookies.patch (1.3 KB ) - added by juha.mustonen@… 9 years ago.
Patch for adding httponly cookie support
10453-httponly-cookies-r11008.patch (2.9 KB ) - added by Remy Blank 9 years ago.
Use httponly cookies by default.
10453-httponly-cookies-unconditional-r11008.patch (2.1 KB ) - added by Remy Blank 9 years ago.
Enable httponly cookie attribute unconditionally with Python ≥ 2.6.

Download all attachments as: .zip

Change History (15)

by juha.mustonen@…, 9 years ago

Attachment: httponly_cookies.patch added

Patch for adding httponly cookie support

comment:1 by Remy Blank, 9 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.

in reply to:  1 comment:2 by py.hieroglyph@…, 9 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 Remy Blank, 9 years ago

Milestone: next-major-0.1X0.13
Owner: set to Remy Blank

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 Peter Suter, 9 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 Remy Blank, 9 years ago

Use httponly cookies by default.

comment:5 by Remy Blank, 9 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 Christian Boos, 9 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 Remy Blank, 9 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 Remy Blank, 9 years ago

Enable httponly cookie attribute unconditionally with Python ≥ 2.6.

comment:8 by Remy Blank, 9 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?

comment:9 by Peter Suter, 9 years ago

Looks good to me!

comment:10 by Remy Blank, 9 years ago

Resolution: fixed
Status: newclosed

Patch applied in [11012].

comment:11 by santeri.toikka@…, 7 years ago

This project https://projects.developer.nokia.com/TracKanbanBoard relies on cookie visibility. Is there a config option to turn this off?

in reply to:  11 comment:12 by Steffen Hoffmann, 7 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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.