[CherryPy] #874: [PATCH] Fix potential denial of service in session handling

[CherryPy]

#874: [PATCH] Fix potential denial of service in session handling
-------------------------------------+--------------------------------------
Reporter: filter-...@mbox.bz | Owner: no_mind
Type: defect | Status: new
Priority: normal | Milestone:
Component: sessions | Keywords: session cookie
-------------------------------------+--------------------------------------
CherryPy with 'tools.sessions.on = True' will currently create a new
session for each request that does not come with a valid session cookie.
This is undesirable because every session consumes server ressources (RAM,
diskspace or slots in a LRU) and a malicious attack, or simply a high-
traffic situation, can lead to a denial of service for legitimate users
due to ressource starvation.

For example, a simple apachebench (ab) performing normal GET-requests can
create thousands of sessions per second without breaking a sweat.

The attached patch mitigates the problem creating sessions only when
absolutely necessary, that is when data has been written to the session,
e.g. via ''cherrypy.session['foo'] = bar''.

This gives full control over the session lifecycle to the application
developer, who must still pay attention to not create sessions carelessly.

As a rule of thumb: Create sessions only after a successful login or
captcha verification. Pages that are freely accessible for hammering
should never create a session.

There is one backwards-incompatible change in this Changeset:
Custom Session implementations (including subclasses of !RamSession,
!MemcachedSession etc.) that override !__init!__() must add a check to the
constructor to bail out if the session "disappears during initialization".

See an example:

{{{
class MyRamSession(RamSession)
def __init__( self, id=None, **kwargs ):
RamSession.__init__( self, id, **kwargs )

### this block is new
if self.id is None:
# this was only a test for existence, bail out
return

# do your thing here as usual...
}}}

As you can see this is not a big change but one you must be aware of or
your custom Session impl will break.

Other than that the patch should be fully backwards compatible.

These are the new semantics for all possible cases:

{{{
Request : No session cookie
Controller: No write to the session
Result : No session is created, no cookie is sent

Request : No session cookie
Controller: Writes to the session
Result : Session is created, browser cookie sent

Request : Valid session cookie (session exists)
Controller: Writes to the session
Result : Session is maintained as usual

Request : Valid session cookie (session exists)
Controller: No write to the session
Result : Session is maintained as usual

Request : Invalid session cookie (invalid, expired or non-existing
sessionid)
Controller: No write to the session
Result : No session is created, browser cookie is cleared

Request : Invalid session cookie (invalid, expired or non-existing
sessionid)
Controller: Writes to the session
Result : Session is created, browser cookie is updated

}}}


TESTING:
I have confirmed all cases manually with firefox/firebug against my
subclass of !MemcachedSession and a vanilla !RamSession. It works. The
automatic tests unfortunately crash & burn, no idea what's going on there
(fumanchu, can you take a look?).

Feedback and improvements welcome...

--
Ticket URL: <http://www.cherrypy.org/ticket/874>
CherryPy <http://www.cherrypy.org>
CherryPy - a pythonic, object-oriented HTTP framework

[CherryPy]

#874: [PATCH] Fix potential denial of service in session handling
-------------------------------------+--------------------------------------
Reporter: filter-...@mbox.bz | Owner: no_mind
Type: defect | Status: new
Priority: normal | Milestone:

Component: sessions | Resolution:
Keywords: session cookie |
-------------------------------------+--------------------------------------

[CherryPy]

#874: [PATCH] Fix potential denial of service in session handling
-------------------------------------+--------------------------------------
Reporter: filter-...@mbox.bz | Owner: no_mind
Type: defect | Status: new
Priority: normal | Milestone:

Component: sessions | Resolution:
Keywords: session cookie |
-------------------------------------+--------------------------------------

Old description:

> CherryPy with 'tools.sessions.on = True' will currently create a new
> session for each request that does not come with a valid session cookie.
> This is undesirable because every session consumes server ressources
> (RAM, diskspace or slots in a LRU) and a malicious attack, or simply a

> high-traffic situation, can lead to a denial of service for legitimate

> All unit tests pass and I have confirmed all cases manually with
> firefox/firecookie against my subclass of !MemcachedSession and a vanilla
> !RamSession.
>
> Feedback and improvements welcome...

New description:

CherryPy with 'tools.sessions.on = True' will currently create a new

session for each request that arrives without a valid session cookie. This

is undesirable because every session consumes server ressources (RAM,
diskspace or slots in a LRU) and a malicious attack, or simply a high-

traffic situation, can lead to denial of service for legitimate users.

For perspective: a single malicious client-host can enforce the creation
of thousands of sessions per second using nothing but a simple request-
generator like ab2 (apachebench).

The attached patch mitigates the problem by creating sessions only when
absolutely necessary, that is when data has been written to the session-
dict, e.g. via ''cherrypy.session['foo'] = bar''.

This gives full control over the session lifecycle to the application
developer, who must still pay attention to not create sessions carelessly.

As a rule of thumb: Create sessions only after a successful login or
captcha verification. Pages that are freely accessible for hammering
should never create a session.

There is one backwards-incompatible change in this Changeset:
Custom Session implementations (including subclasses of !RamSession,
!MemcachedSession etc.) that override !__init!__() must add a check to the
constructor to bail out if the session "disappears during initialization".

See an example:

{{{
class MyRamSession(RamSession)
def __init__( self, id=None, **kwargs ):
RamSession.__init__( self, id, **kwargs )

### this block is new
if self.id is None:
# this was only a test for existence, bail out
return

# do your thing here as usual...
}}}

As you can see this is not a big change but one you must be aware of or

your custom Session impl may break.

Other than that the patch should be fully backwards compatible.

These are the new semantics for all possible cases:

{{{
Request : No session cookie
Controller: No write to the session
Result : No session is created, no cookie is sent

Request : No session cookie
Controller: Writes to the session
Result : Session is created, browser cookie sent

Request : Valid session cookie (session exists)
Controller: Writes to the session
Result : Session is maintained as usual

Request : Valid session cookie (session exists)
Controller: No write to the session
Result : Session is maintained as usual

Request : Invalid session cookie (invalid, expired or non-existing
sessionid)
Controller: No write to the session
Result : No session is created, browser cookie is cleared

Request : Invalid session cookie (invalid, expired or non-existing
sessionid)
Controller: Writes to the session
Result : Session is created, browser cookie is updated

}}}


TESTING:

All unit tests pass and I have confirmed all cases manually with
firefox/firecookie against my subclass of !MemcachedSession and a vanilla
!RamSession.

[CherryPy]

#874: [PATCH] Fix potential denial of service in session handling
-------------------------------------+--------------------------------------
Reporter: filter-...@mbox.bz | Owner: no_mind
Type: defect | Status: new
Priority: normal | Milestone:

Component: sessions | Resolution:
Keywords: session cookie |
-------------------------------------+--------------------------------------
Old description:

> CherryPy with 'tools.sessions.on = True' will currently create a new

> session for each request that arrives without a valid session cookie.

> This is undesirable because every session consumes server ressources
> (RAM, diskspace or slots in a LRU) and a malicious attack, or simply a

> high-traffic situation, can lead to denial of service for legitimate

> users.
>
> For perspective: a single malicious client-host can enforce the creation
> of thousands of sessions per second using nothing but a simple request-
> generator like ab2 (apachebench).
>

> The attached patch mitigates the problem by creating sessions only when
> absolutely necessary, that is when data has been written to the session-
> dict, e.g. via ''cherrypy.session['foo'] = bar''.

>
> This gives full control over the session lifecycle to the application
> developer, who must still pay attention to not create sessions
> carelessly.
>
> As a rule of thumb: Create sessions only after a successful login or
> captcha verification. Pages that are freely accessible for hammering
> should never create a session.
>
> There is one backwards-incompatible change in this Changeset:
> Custom Session implementations (including subclasses of !RamSession,
> !MemcachedSession etc.) that override !__init!__() must add a check to
> the constructor to bail out if the session "disappears during
> initialization".
>
> See an example:
>
> {{{
> class MyRamSession(RamSession)
> def __init__( self, id=None, **kwargs ):
> RamSession.__init__( self, id, **kwargs )
>
> ### this block is new
> if self.id is None:
> # this was only a test for existence, bail out
> return
>
> # do your thing here as usual...
> }}}
>
> As you can see this is not a big change but one you must be aware of or

> your custom Session impl may break.

>
> Other than that the patch should be fully backwards compatible.
>
> These are the new semantics for all possible cases:
>
> {{{
> Request : No session cookie
> Controller: No write to the session
> Result : No session is created, no cookie is sent
>
> Request : No session cookie
> Controller: Writes to the session
> Result : Session is created, browser cookie sent
>
> Request : Valid session cookie (session exists)
> Controller: Writes to the session
> Result : Session is maintained as usual
>
> Request : Valid session cookie (session exists)
> Controller: No write to the session
> Result : Session is maintained as usual
>
> Request : Invalid session cookie (invalid, expired or non-existing
> sessionid)
> Controller: No write to the session
> Result : No session is created, browser cookie is cleared
>
> Request : Invalid session cookie (invalid, expired or non-existing
> sessionid)
> Controller: Writes to the session
> Result : Session is created, browser cookie is updated
>
> }}}
>

> TESTING:

> All unit tests pass and I have confirmed all cases manually with
> firefox/firecookie against my subclass of !MemcachedSession and a vanilla
> !RamSession.
>

> Feedback and improvements welcome...

New description:

CherryPy with 'tools.sessions.on = True' will currently create a new

session for each request that arrives without a valid session cookie. This

is undesirable because every session consumes server ressources (RAM,
diskspace or slots in a LRU) and a malicious attack, or simply a high-

traffic situation, can lead to denial of service for legitimate users.

For perspective: a single malicious client-host can enforce the creation
of thousands of sessions per second using nothing but a simple request-
generator like ab2 (apachebench).

The attached patch mitigates the problem by creating sessions only when
absolutely necessary, that is when data has been written to the session-
dict, e.g. via ''cherrypy.session['foo'] = bar''.

This gives full control over the session lifecycle to the application
developer, who must still pay attention to not create sessions carelessly.

As a rule of thumb: Create sessions only after a successful login or
captcha verification. Pages that are freely accessible for hammering
should never create a session.

There is one backwards-incompatible change in this Changeset:
Custom Session implementations (including subclasses of !RamSession,

!MemcachedSession etc. that override !__init!__()) must add a check to the
constructor that bails out if the session "disappears during

initialization".

See an example:

{{{
class MyRamSession(RamSession)
def __init__( self, id=None, **kwargs ):
RamSession.__init__( self, id, **kwargs )

### this block is new
if self.id is None:
# this was only a test for existence, bail out
return

# do your thing here as usual...
}}}

Other than that the patch should be fully backwards compatible.

These are the new semantics for all possible cases:

{{{
Request : No session cookie
Controller: No write to the session
Result : No session is created, no cookie is sent

Request : No session cookie
Controller: Writes to the session
Result : Session is created, browser cookie sent

Request : Valid session cookie (session exists)
Controller: Writes to the session
Result : Session is maintained as usual

Request : Valid session cookie (session exists)
Controller: No write to the session
Result : Session is maintained as usual

Request : Invalid session cookie (invalid, expired or non-existing
sessionid)
Controller: No write to the session
Result : No session is created, browser cookie is cleared

Request : Invalid session cookie (invalid, expired or non-existing
sessionid)
Controller: Writes to the session
Result : Session is created, browser cookie is updated

}}}


TESTING:

All unit tests pass and I have confirmed all cases manually with
firefox/firecookie against my subclass of !MemcachedSession and a vanilla
!RamSession.

[CherryPy]

#874: [PATCH] Fix potential denial of service in session handling
-------------------------------------+--------------------------------------
Reporter: filter-...@mbox.bz | Owner: no_mind
Type: defect | Status: new
Priority: normal | Milestone:

Component: sessions | Resolution:
Keywords: session cookie |
-------------------------------------+--------------------------------------

Comment (by fumanchu):

Hm. You say "create sessions only after a successful login or captcha
verification", but I can't tell whether your list of cases is
authenticated or not or a mix of both. It looks like both. Can you double
the number of cases, so that each one has both a verified auth case and an
unverified case? That would help greatly.

[CherryPy]

#874: [PATCH] Fix potential denial of service in session handling
-------------------------------------+--------------------------------------
Reporter: filter-...@mbox.bz | Owner: no_mind
Type: defect | Status: new
Priority: normal | Milestone:

Component: sessions | Resolution:
Keywords: session cookie |
-------------------------------------+--------------------------------------

Changes (by moe):

* cc: => filter-...@mbox.bz

Comment:

Sorry, authentication is completely irrelevant here. I should maybe have
left that suggestion out of the original text to avoid confusion.

The purpose of the patch is to have cherrypy only create a session when
the app consciously decides to create one (by writing to the session dict)
and not for ''every'' request (as it is now).

That's really all there is to it.
It doesn't matter at all ''why'' the app decides to create a session,
that's up to the app developer. It only matters that cherrypy will not
create a session when it doesn't need to.

[CherryPy]

#874: [PATCH] Fix potential denial of service in session handling
-------------------------------------+--------------------------------------

Reporter: filter-...@mbox.bz | Owner: JohnWell
Type: defect | Status: new
Priority: lowest | Milestone:

Component: sessions | Resolution:
Keywords: session cookie |
-------------------------------------+--------------------------------------

Changes (by guest):

* cc: filter-...@mbox.bz =>
* owner: no_mind => JohnWell
* priority: normal => lowest

Comment:

I am facing almost the same problem