#7515 closed (fixed)
Add .clear() and .destroy() to session objects
| Reported by: | mrts | Owned by: | Malcolm Tredinnick | 
|---|---|---|---|
| Component: | contrib.sessions | Version: | dev | 
| Severity: | Keywords: | ||
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no | 
| Needs tests: | no | Patch needs improvement: | no | 
| Easy pickings: | no | UI/UX: | no | 
Description
Reliable session clearing is required to avoid fragile (another thread may delete a key and cause an exception in the loop given in the snippet) and cumbersome tricks like http://www.djangosnippets.org/snippets/681/ .
See also http://groups.google.com/group/django-developers/browse_thread/thread/fbcfa88c997d1bb3 .
Note that this doesn't reset the session key, only the data associated with the key. Other frameworks tend to reset the key as well. I believe this is redundant, but feel free to correct me. The reasoning behind that claim is as follows:
session key -> sensitive data clear session, but don't reset the key session key -> no data => therefore no sensitive information leaked, as the key per se is public, only the data set behind it is private
Attachments (5)
Change History (18)
by , 17 years ago
| Attachment: | session_clear.diff added | 
|---|
comment:1 by , 17 years ago
| Owner: | changed from to | 
|---|---|
| Status: | new → assigned | 
| Triage Stage: | Unreviewed → Accepted | 
comment:2 by , 17 years ago
| Needs documentation: | set | 
|---|
comment:3 by , 17 years ago
The "proof" behind the claim should read as follows (where -> designates mapping and '=>' implication):
session key X -> sensitive data for user A clear session, but don't reset the key session key X -> no data reuse session key for user B session key X -> sensitive data for user B => therefore no sensitive information leaked, as the key per se is public, only the data set behind it is private
Attaching docs as per Jacob's request.
by , 17 years ago
| Attachment: | session_clear-with_docs.diff added | 
|---|
comment:4 by , 17 years ago
| Needs documentation: | unset | 
|---|
by , 17 years ago
| Attachment: | session_clear-with_docs2.diff added | 
|---|
Rephrased docs as per suggestions on the IRC.
comment:5 by , 17 years ago
| Triage Stage: | Accepted → Ready for checkin | 
|---|
comment:6 by , 17 years ago
| Triage Stage: | Ready for checkin → Design decision needed | 
|---|
One could argue that not resetting the session key is not exception safe:
- session key X -> sensitive data for user A
- clear session, but don't reset the key
- session changed, empty session needs to be cleared from db
- db connection has died, exception, session.save()fails
- old key remains in browser, will be reused for user B
- db comes online again
- user B visits site and gets access to user A's sensitive data as the cookie and session key have not changed
So, I'd propose leaving clear() as is and adding reset() or destroy() that 
- generates a new key
- calls clear()intry/catch
- assigns the new key to the new session in finallyand re-raises the exception that was thrown
- assures that the old cookie is removed from browser and perhaps a new one sent regardless of exceptions thrown
Obviously, if B has maliciously stolen A's session this doesn't help. It's only good for the common inadvertent case.
by , 17 years ago
| Attachment: | clear_and_destroy.diff added | 
|---|
Exception-safe destroy() added to request.session, with tests and docs
comment:7 by , 17 years ago
| Summary: | Add .clear() to session objects → Add .clear() and .destroy() to session objects | 
|---|
by , 17 years ago
| Attachment: | clear_and_destroy2.diff added | 
|---|
Remove redundant .save()s from tests and update to r8158
comment:8 by , 17 years ago
| Triage Stage: | Design decision needed → Accepted | 
|---|
comment:9 by , 17 years ago
| Owner: | changed from to | 
|---|---|
| Status: | assigned → new | 
comment:10 by , 17 years ago
comment:11 by , 17 years ago
comment:12 by , 17 years ago
| Resolution: | → fixed | 
|---|---|
| Status: | new → closed | 
I'm not going to worry about the attempt at exception-proofing mentioned in comment 6. That only trades one race condition for some other ones. The random numbers could collide. The network between the server and the browser could fail to deliver the message. The user could turn off their browser or computer before the reply gets back, etc. There's no way to completely guarantee that the updated cookie will reach their browser, so there's always a low-probability chance of failure.
new method, new docs ;)