httponly for cookies

Bug #367393 reported by stephan_hofmockel
6
Affects Status Importance Assigned to Milestone
Zope 2
Fix Released
Undecided
Tres Seaver

Bug Description

Hi,

in our application we are useing the 'HTTPOnly' attribute for cookies (http://www.owasp.org/index.php/HTTPOnly)
to protect the cookie from JavaScript access.

We currently patch the Zope2 like the changes in the attachment. The diff in the attachment is done against Zope2-2.12.0a4 and does two things

1) HTTPResponse knows the cookie attribute 'HTTPOnly' and add it to the header
2) Add a option in BrowserIdManager.py to autoadd the 'HTTPOnly' attritube for cookies used by sessions

Is there a chance, that the 'HTTPOnly'-attribute will be added to Zope in the future ???

Cheers,
 Stephan

PS: I dont know the 'formalism' how to add a feature-request for Zope2. Please criticise me if this way is wrong.

Tags: feature zope
Revision history for this message
stephan_hofmockel (dreagonfly) wrote :
Revision history for this message
Andreas Jung (ajung) wrote :

Your patch is totally unreadable (including by reading the HTML sources).
The patch is possibly of interest - the chance are bigger to have it included in Zope 2.12 with some tests :-)
However we need a readable version of the patch first.

Revision history for this message
Tres Seaver (tseaver) wrote :
Revision history for this message
Tres Seaver (tseaver) wrote :

This patch is really two separate changes:

 - Adding support to ZPulbisher.HTTPResponse for an extra type of "special"
   cookie. Thiis part certainly needs tests, but should be uncontroversial.

 - Using that added support from within the BrowserIDManager code. I'm not
   quite sure what the benefit is here, given the nature of the contents of that
   cookie. Can the OP elaborate why the browser ID cookie needs this feature?

Revision history for this message
stephan_hofmockel (dreagonfly) wrote :

Sorry for the upload crap :(

 1) How should a good test look like for HTTPResponse ?? My current test calls HTTPResponse._cookie_list() directly and parses the output with regex.

 2) This attribute prevents cookie access from JavaScript. So it is more difficult for malicious JavaScript code to hijack the session, with sending the current session-cookie to the attacker.
You are right, most of the time data in a session is not critical and a attacker with a valid sessionID gains nothing.
Unfortunately our application saves critical data in the session and uses (not only) this "special"-cookie for protection.

I know this feature depends heavily on browser support and assists the "save critical data in a session" programming style. Nevertheless sometimes its unavoidable and other applications outside can profit from this additional attribute.

Revision history for this message
Tres Seaver (tseaver) wrote : Re: [Bug 367393] Re: httponly for cookies

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

stephan_hofmockel wrote:
> Sorry for the upload crap :(
>
> 1) How should a good test look like for HTTPResponse ?? My current test
> calls HTTPResponse._cookie_list() directly and parses the output with
> regex.

Unfortunately, there aren't good tests for 'setCookie' at this point
(nothing which exercies the 'Domain', 'Max-Age', 'Comment', or Secure'
features). I have added such tests to the trunk and backported them;
they should serve as an example. See:

 http://svn.zope.org/Zope/branches/2.11/?rev=99537&view=rev

> 2) This attribute prevents cookie access from JavaScript. So it is more difficult for malicious JavaScript code to hijack the session, with sending the current session-cookie to the attacker.
> You are right, most of the time data in a session is not critical and a attacker with a valid sessionID gains nothing.
> Unfortunately our application saves critical data in the session and uses (not only) this "special"-cookie for protection.
>
> I know this feature depends heavily on browser support and assists the
> "save critical data in a session" programming style. Nevertheless
> sometimes its unavoidable and other applications outside can profit from
> this additional attribute.

A couple of things:

- - The actual browser ID itself can't be precious data: it is a largely
  opaque token, made up of both a random int and a timestamp.

- - Javascript-generated requests made from the same browser will still
  carry along the browser ID cookie, even though the feature might
  prevent JS from knowing the value of the cookie.

- - Javascript doesn't have access to cookies from domains other than
  those which match the current request; where is the attack vector
  which would expose a browser ID cookie to a malicious off-site
  script? Is the theory that there are browsers in the wild which
  have broken sandboxing (so that JS can see cookies from other domains)
  but which implement this feature correctly?

- - Cookies for non-HTTPS requests are *still* visible in clear to
  man-in-the middle observers, and could therefore still be for
  replay attacks.

All that being said, I'm fine with landing both parts of the patch, once
we have tests for the 'setCookie' / '_cookies_list' parts.

Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 <email address hidden>
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJ9bpr+gerLs4ltQ4RAnMXAKDQvstQTBnItf9hiAxLrl5SXxz+0gCgnsEf
oWSgM8Z+hnBRqFBsN66Q3Lo=
=kgkS
-----END PGP SIGNATURE-----

Revision history for this message
stephan_hofmockel (dreagonfly) wrote :

Here is a second version of the patch

there are two changes which do not affect the discussed feature
 - in line 390 of BrowserIdManager.py the 'declareProtected' call does not address the right function
 - add two tests for the 'Secure'-Cookie attribute

I am not sure if the tests in testBrowserIdManager fulfil the 'zope test-standards'

Revision history for this message
Tres Seaver (tseaver) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

stephan_hofmockel wrote:
> Here is a second version of the patch
>
> there are two changes which do not affect the discussed feature
> - in line 390 of BrowserIdManager.py the 'declareProtected' call does not address the right function
> - add two tests for the 'Secure'-Cookie attribute
>
> I am not sure if the tests in testBrowserIdManager fulfil the 'zope
> test-standards'

They look fine to me. I will do the merge.

 status inprogress
 assign tseaver

Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 <email address hidden>
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJ9zGV+gerLs4ltQ4RAlLpAKCoJkflRn/h2SJOs7vAKV9ee32CdACgvCbE
Pj+yqCJLT+S1FWKJkSVue4g=
=JaoU
-----END PGP SIGNATURE-----

Changed in zope2:
status: New → In Progress
Revision history for this message
Tres Seaver (tseaver) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 status fixcommitted

Committed in two parts:

- - The HTTPResponse change enabling the 'HTTPOnly' attribute,
  http://svn.zope.org/Zope/trunk/?rev=99561&view=rev

- - Using that attribute for BrowserIdManager's cookie,
  http://svn.zope.org/Zope/trunk/?rev=99562&view=rev

Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 <email address hidden>
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJ9zSS+gerLs4ltQ4RAnR0AKCOd+KqeUkBcm+RlEphzc74nR//agCeJ1S4
1hYIWvL/fNXBvad9F3+XF0E=
=tjst
-----END PGP SIGNATURE-----

Changed in zope2:
status: In Progress → Fix Committed
assignee: nobody → tseaver
status: Fix Committed → In Progress
Revision history for this message
Andreas Jung (ajung) wrote :

Why "in progress"?

Revision history for this message
Tres Seaver (tseaver) wrote :

Released in 2.12.0b1.

Changed in zope2:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.