You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mod_python-dev@quetz.apache.org by "Graham Dumpleton (JIRA)" <ji...@apache.org> on 2006/10/02 09:34:19 UTC
[jira] Created: (MODPYTHON-191) Tampering with signed cookies.
Tampering with signed cookies.
------------------------------
Key: MODPYTHON-191
URL: http://issues.apache.org/jira/browse/MODPYTHON-191
Project: mod_python
Issue Type: Bug
Components: core
Affects Versions: 3.2.10
Reporter: Graham Dumpleton
As reported by Andy Pearce in:
http://mail-archives.apache.org/mod_mbox/httpd-python-dev/200609.mbox/%3c44F824E2.4040304@jgassociates.ca%3e
Andy Pearce wrote:
>
> Hi,
>
> I think I might have spotted a slight bug in Session.py. When the
> 'secret' parameter is supplied to use the SignedCookie class, it appears
> that __init__ of BaseSession doesn't check the return type of
> get_cookies().
>
> If I understand the SignedCookie docs correctly, if the cookie value
> doesn't match its signature, it simply returns the contents as a Cookie
> rather than a SignedCookie (indicating that the user tampered with their
> cookie before sending it back).
>
> However, there is no check in BaseSession's __init__ that the return of
> get_cookies() is a SignedCookie in the case that 'secret' is supplied.
>
> Perhaps a minor point, but it would seem to make the option of using
> SignedCookies rather pointless, since the signature isn't being checked.
> Presumably if the cookie has been tampered with, your only safe option
> is to throw it away and generate a new one. I think this can be achieved
> by changing the lines:
>
> if cookies.has_key(session_cookie_name):
> self._sid = cookies[session_cookie_name].value
>
> To something like:
>
> if cookies.has_key(session_cookie_name):
> if not secret or type(cookes[session_cookie_name]) \
> is Cookie.SignedCookie:
> self._sid = cookies[session_cookie_name].value
>
> I'm fairly new to mod_python, so if I'm mistaken then my apologies, and
> a quick explanation of why would be very much appreciated! ^_^
>
> Thanks,
>
> - Andy
>
Is this correct and should the change suggested appropriate?
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Resolved: (MODPYTHON-191) Tampering with signed cookies.
Posted by "Graham Dumpleton (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/MODPYTHON-191?page=all ]
Graham Dumpleton resolved MODPYTHON-191.
----------------------------------------
Resolution: Fixed
> Tampering with signed cookies.
> ------------------------------
>
> Key: MODPYTHON-191
> URL: http://issues.apache.org/jira/browse/MODPYTHON-191
> Project: mod_python
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.10
> Reporter: Graham Dumpleton
> Assigned To: Graham Dumpleton
> Fix For: 3.3
>
>
> As reported by Andy Pearce in:
> http://mail-archives.apache.org/mod_mbox/httpd-python-dev/200609.mbox/%3c44F824E2.4040304@jgassociates.ca%3e
> Andy Pearce wrote:
> >
> > Hi,
> >
> > I think I might have spotted a slight bug in Session.py. When the
> > 'secret' parameter is supplied to use the SignedCookie class, it appears
> > that __init__ of BaseSession doesn't check the return type of
> > get_cookies().
> >
> > If I understand the SignedCookie docs correctly, if the cookie value
> > doesn't match its signature, it simply returns the contents as a Cookie
> > rather than a SignedCookie (indicating that the user tampered with their
> > cookie before sending it back).
> >
> > However, there is no check in BaseSession's __init__ that the return of
> > get_cookies() is a SignedCookie in the case that 'secret' is supplied.
> >
> > Perhaps a minor point, but it would seem to make the option of using
> > SignedCookies rather pointless, since the signature isn't being checked.
> > Presumably if the cookie has been tampered with, your only safe option
> > is to throw it away and generate a new one. I think this can be achieved
> > by changing the lines:
> >
> > if cookies.has_key(session_cookie_name):
> > self._sid = cookies[session_cookie_name].value
> >
> > To something like:
> >
> > if cookies.has_key(session_cookie_name):
> > if not secret or type(cookes[session_cookie_name]) \
> > is Cookie.SignedCookie:
> > self._sid = cookies[session_cookie_name].value
> >
> > I'm fairly new to mod_python, so if I'm mistaken then my apologies, and
> > a quick explanation of why would be very much appreciated! ^_^
> >
> > Thanks,
> >
> > - Andy
> >
> Is this correct and should the change suggested appropriate?
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (MODPYTHON-191) Tampering with signed cookies.
Posted by "Graham Dumpleton (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/MODPYTHON-191?page=all ]
Graham Dumpleton updated MODPYTHON-191:
---------------------------------------
Fix Version/s: 3.3
I have marked this as fix for 3.3 as it seems worrying enough that we should address it. Can someone else provide confirmation that it is something that we should worry about and that change is reasonable.
> Tampering with signed cookies.
> ------------------------------
>
> Key: MODPYTHON-191
> URL: http://issues.apache.org/jira/browse/MODPYTHON-191
> Project: mod_python
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.10
> Reporter: Graham Dumpleton
> Fix For: 3.3
>
>
> As reported by Andy Pearce in:
> http://mail-archives.apache.org/mod_mbox/httpd-python-dev/200609.mbox/%3c44F824E2.4040304@jgassociates.ca%3e
> Andy Pearce wrote:
> >
> > Hi,
> >
> > I think I might have spotted a slight bug in Session.py. When the
> > 'secret' parameter is supplied to use the SignedCookie class, it appears
> > that __init__ of BaseSession doesn't check the return type of
> > get_cookies().
> >
> > If I understand the SignedCookie docs correctly, if the cookie value
> > doesn't match its signature, it simply returns the contents as a Cookie
> > rather than a SignedCookie (indicating that the user tampered with their
> > cookie before sending it back).
> >
> > However, there is no check in BaseSession's __init__ that the return of
> > get_cookies() is a SignedCookie in the case that 'secret' is supplied.
> >
> > Perhaps a minor point, but it would seem to make the option of using
> > SignedCookies rather pointless, since the signature isn't being checked.
> > Presumably if the cookie has been tampered with, your only safe option
> > is to throw it away and generate a new one. I think this can be achieved
> > by changing the lines:
> >
> > if cookies.has_key(session_cookie_name):
> > self._sid = cookies[session_cookie_name].value
> >
> > To something like:
> >
> > if cookies.has_key(session_cookie_name):
> > if not secret or type(cookes[session_cookie_name]) \
> > is Cookie.SignedCookie:
> > self._sid = cookies[session_cookie_name].value
> >
> > I'm fairly new to mod_python, so if I'm mistaken then my apologies, and
> > a quick explanation of why would be very much appreciated! ^_^
> >
> > Thanks,
> >
> > - Andy
> >
> Is this correct and should the change suggested appropriate?
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Work started: (MODPYTHON-191) Tampering with signed cookies.
Posted by "Graham Dumpleton (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/MODPYTHON-191?page=all ]
Work on MODPYTHON-191 started by Graham Dumpleton.
> Tampering with signed cookies.
> ------------------------------
>
> Key: MODPYTHON-191
> URL: http://issues.apache.org/jira/browse/MODPYTHON-191
> Project: mod_python
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.10
> Reporter: Graham Dumpleton
> Assigned To: Graham Dumpleton
> Fix For: 3.3
>
>
> As reported by Andy Pearce in:
> http://mail-archives.apache.org/mod_mbox/httpd-python-dev/200609.mbox/%3c44F824E2.4040304@jgassociates.ca%3e
> Andy Pearce wrote:
> >
> > Hi,
> >
> > I think I might have spotted a slight bug in Session.py. When the
> > 'secret' parameter is supplied to use the SignedCookie class, it appears
> > that __init__ of BaseSession doesn't check the return type of
> > get_cookies().
> >
> > If I understand the SignedCookie docs correctly, if the cookie value
> > doesn't match its signature, it simply returns the contents as a Cookie
> > rather than a SignedCookie (indicating that the user tampered with their
> > cookie before sending it back).
> >
> > However, there is no check in BaseSession's __init__ that the return of
> > get_cookies() is a SignedCookie in the case that 'secret' is supplied.
> >
> > Perhaps a minor point, but it would seem to make the option of using
> > SignedCookies rather pointless, since the signature isn't being checked.
> > Presumably if the cookie has been tampered with, your only safe option
> > is to throw it away and generate a new one. I think this can be achieved
> > by changing the lines:
> >
> > if cookies.has_key(session_cookie_name):
> > self._sid = cookies[session_cookie_name].value
> >
> > To something like:
> >
> > if cookies.has_key(session_cookie_name):
> > if not secret or type(cookes[session_cookie_name]) \
> > is Cookie.SignedCookie:
> > self._sid = cookies[session_cookie_name].value
> >
> > I'm fairly new to mod_python, so if I'm mistaken then my apologies, and
> > a quick explanation of why would be very much appreciated! ^_^
> >
> > Thanks,
> >
> > - Andy
> >
> Is this correct and should the change suggested appropriate?
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (MODPYTHON-191) Tampering with signed cookies.
Posted by "Graham Dumpleton (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/MODPYTHON-191?page=all ]
Graham Dumpleton updated MODPYTHON-191:
---------------------------------------
Fix Version/s: 3.3
This will now be addressed as part of code changes for MODPYTHON-200.
> Tampering with signed cookies.
> ------------------------------
>
> Key: MODPYTHON-191
> URL: http://issues.apache.org/jira/browse/MODPYTHON-191
> Project: mod_python
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.10
> Reporter: Graham Dumpleton
> Fix For: 3.3
>
>
> As reported by Andy Pearce in:
> http://mail-archives.apache.org/mod_mbox/httpd-python-dev/200609.mbox/%3c44F824E2.4040304@jgassociates.ca%3e
> Andy Pearce wrote:
> >
> > Hi,
> >
> > I think I might have spotted a slight bug in Session.py. When the
> > 'secret' parameter is supplied to use the SignedCookie class, it appears
> > that __init__ of BaseSession doesn't check the return type of
> > get_cookies().
> >
> > If I understand the SignedCookie docs correctly, if the cookie value
> > doesn't match its signature, it simply returns the contents as a Cookie
> > rather than a SignedCookie (indicating that the user tampered with their
> > cookie before sending it back).
> >
> > However, there is no check in BaseSession's __init__ that the return of
> > get_cookies() is a SignedCookie in the case that 'secret' is supplied.
> >
> > Perhaps a minor point, but it would seem to make the option of using
> > SignedCookies rather pointless, since the signature isn't being checked.
> > Presumably if the cookie has been tampered with, your only safe option
> > is to throw it away and generate a new one. I think this can be achieved
> > by changing the lines:
> >
> > if cookies.has_key(session_cookie_name):
> > self._sid = cookies[session_cookie_name].value
> >
> > To something like:
> >
> > if cookies.has_key(session_cookie_name):
> > if not secret or type(cookes[session_cookie_name]) \
> > is Cookie.SignedCookie:
> > self._sid = cookies[session_cookie_name].value
> >
> > I'm fairly new to mod_python, so if I'm mistaken then my apologies, and
> > a quick explanation of why would be very much appreciated! ^_^
> >
> > Thanks,
> >
> > - Andy
> >
> Is this correct and should the change suggested appropriate?
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Closed: (MODPYTHON-191) Tampering with signed cookies.
Posted by "Graham Dumpleton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MODPYTHON-191?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Graham Dumpleton closed MODPYTHON-191.
--------------------------------------
> Tampering with signed cookies.
> ------------------------------
>
> Key: MODPYTHON-191
> URL: https://issues.apache.org/jira/browse/MODPYTHON-191
> Project: mod_python
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.10
> Reporter: Graham Dumpleton
> Assigned To: Graham Dumpleton
> Fix For: 3.3
>
>
> As reported by Andy Pearce in:
> http://mail-archives.apache.org/mod_mbox/httpd-python-dev/200609.mbox/%3c44F824E2.4040304@jgassociates.ca%3e
> Andy Pearce wrote:
> >
> > Hi,
> >
> > I think I might have spotted a slight bug in Session.py. When the
> > 'secret' parameter is supplied to use the SignedCookie class, it appears
> > that __init__ of BaseSession doesn't check the return type of
> > get_cookies().
> >
> > If I understand the SignedCookie docs correctly, if the cookie value
> > doesn't match its signature, it simply returns the contents as a Cookie
> > rather than a SignedCookie (indicating that the user tampered with their
> > cookie before sending it back).
> >
> > However, there is no check in BaseSession's __init__ that the return of
> > get_cookies() is a SignedCookie in the case that 'secret' is supplied.
> >
> > Perhaps a minor point, but it would seem to make the option of using
> > SignedCookies rather pointless, since the signature isn't being checked.
> > Presumably if the cookie has been tampered with, your only safe option
> > is to throw it away and generate a new one. I think this can be achieved
> > by changing the lines:
> >
> > if cookies.has_key(session_cookie_name):
> > self._sid = cookies[session_cookie_name].value
> >
> > To something like:
> >
> > if cookies.has_key(session_cookie_name):
> > if not secret or type(cookes[session_cookie_name]) \
> > is Cookie.SignedCookie:
> > self._sid = cookies[session_cookie_name].value
> >
> > I'm fairly new to mod_python, so if I'm mistaken then my apologies, and
> > a quick explanation of why would be very much appreciated! ^_^
> >
> > Thanks,
> >
> > - Andy
> >
> Is this correct and should the change suggested appropriate?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MODPYTHON-191) Tampering with signed cookies.
Posted by "Graham Dumpleton (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/MODPYTHON-191?page=all ]
Graham Dumpleton updated MODPYTHON-191:
---------------------------------------
Fix Version/s: (was: 3.3)
After looking at this, I have come to the conclusion that in the context used it is not an issue. FWIW, the whole idea of allowing a Session to have a secret associated with it and thus have a signed cookie used seems to be a pointless exercise which adds little value except making the length of the cookie string longer and perhaps therefore harder to guess. To understand one needs to look at what happens when one uses a signed cookie.
When one uses a signed cookie, the string stored in the header will be something like:
36c941203a16532574abe5e9fd3b0028some-value
That is, the first 32 characters represent the hash of the value of the cookie. As one can see, the actual value is stored in whatever follows the first 32 characters. The actual value is not encoded in any way and just appears as is.
When cookies are used for sessions, the session ID is itself a value containing 32 characters. This 32 characters are an md5 hash of a string made up of the Apache child process ID, the remote client IP address and two random numbers.
Thus, the session ID would be something like:
b406b258f7bb1b7d655008ae528f5f0d
When one signs this, one gets:
476e3a39cf8027511507224cf3ecff3bb406b258f7bb1b7d655008ae528f5f0d
As you can see, since the value is as is, the second set of 32 characters is still the same session ID.
The original concern expressed was the code even though told to use a signed cookie, will revert to accepting a non signed cookie. That is, if the first 32 characters are the correct hash for the second 32 characters it will reject it. It will instead take the whole value and try and use that as the SID instead.
At this point, two things still need to happen before the session can work. The first is that the whole value must be 32 characters in length and must only contain lower case alpha numeric characters. If it isn't, it will fail the initial precondition check of what constitues a valid session ID.
Even if it gets past that, the value presented as the session ID still has to actually match an active session in the session database. If it doesn't it is flagged as bogus or expired and a new session will be created with a new session ID pushed back to the browser instead.
The outcome of all this is that even if using a signed cookie for the session, that 64 characters are in the string doesn't matter as one would only need to guess the second 32 characters and simply supply that, with it being accepted as an unsigned cookie.
The question then becomes how concerned are you about the chances of someone guessing a 32 character string, that would make you want to use a 64 character string instead.
Thus, this issue certainly doesn't raise any problem that needs to be addressed at this point. As such, am going to remove this from 3.3 task list. We can consider this again later as to whether a check for a signed cookie is a good idea or not, or simply leave it as is and close the issue.
> Tampering with signed cookies.
> ------------------------------
>
> Key: MODPYTHON-191
> URL: http://issues.apache.org/jira/browse/MODPYTHON-191
> Project: mod_python
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.10
> Reporter: Graham Dumpleton
>
> As reported by Andy Pearce in:
> http://mail-archives.apache.org/mod_mbox/httpd-python-dev/200609.mbox/%3c44F824E2.4040304@jgassociates.ca%3e
> Andy Pearce wrote:
> >
> > Hi,
> >
> > I think I might have spotted a slight bug in Session.py. When the
> > 'secret' parameter is supplied to use the SignedCookie class, it appears
> > that __init__ of BaseSession doesn't check the return type of
> > get_cookies().
> >
> > If I understand the SignedCookie docs correctly, if the cookie value
> > doesn't match its signature, it simply returns the contents as a Cookie
> > rather than a SignedCookie (indicating that the user tampered with their
> > cookie before sending it back).
> >
> > However, there is no check in BaseSession's __init__ that the return of
> > get_cookies() is a SignedCookie in the case that 'secret' is supplied.
> >
> > Perhaps a minor point, but it would seem to make the option of using
> > SignedCookies rather pointless, since the signature isn't being checked.
> > Presumably if the cookie has been tampered with, your only safe option
> > is to throw it away and generate a new one. I think this can be achieved
> > by changing the lines:
> >
> > if cookies.has_key(session_cookie_name):
> > self._sid = cookies[session_cookie_name].value
> >
> > To something like:
> >
> > if cookies.has_key(session_cookie_name):
> > if not secret or type(cookes[session_cookie_name]) \
> > is Cookie.SignedCookie:
> > self._sid = cookies[session_cookie_name].value
> >
> > I'm fairly new to mod_python, so if I'm mistaken then my apologies, and
> > a quick explanation of why would be very much appreciated! ^_^
> >
> > Thanks,
> >
> > - Andy
> >
> Is this correct and should the change suggested appropriate?
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Assigned: (MODPYTHON-191) Tampering with signed cookies.
Posted by "Graham Dumpleton (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/MODPYTHON-191?page=all ]
Graham Dumpleton reassigned MODPYTHON-191:
------------------------------------------
Assignee: Graham Dumpleton
> Tampering with signed cookies.
> ------------------------------
>
> Key: MODPYTHON-191
> URL: http://issues.apache.org/jira/browse/MODPYTHON-191
> Project: mod_python
> Issue Type: Bug
> Components: core
> Affects Versions: 3.2.10
> Reporter: Graham Dumpleton
> Assigned To: Graham Dumpleton
> Fix For: 3.3
>
>
> As reported by Andy Pearce in:
> http://mail-archives.apache.org/mod_mbox/httpd-python-dev/200609.mbox/%3c44F824E2.4040304@jgassociates.ca%3e
> Andy Pearce wrote:
> >
> > Hi,
> >
> > I think I might have spotted a slight bug in Session.py. When the
> > 'secret' parameter is supplied to use the SignedCookie class, it appears
> > that __init__ of BaseSession doesn't check the return type of
> > get_cookies().
> >
> > If I understand the SignedCookie docs correctly, if the cookie value
> > doesn't match its signature, it simply returns the contents as a Cookie
> > rather than a SignedCookie (indicating that the user tampered with their
> > cookie before sending it back).
> >
> > However, there is no check in BaseSession's __init__ that the return of
> > get_cookies() is a SignedCookie in the case that 'secret' is supplied.
> >
> > Perhaps a minor point, but it would seem to make the option of using
> > SignedCookies rather pointless, since the signature isn't being checked.
> > Presumably if the cookie has been tampered with, your only safe option
> > is to throw it away and generate a new one. I think this can be achieved
> > by changing the lines:
> >
> > if cookies.has_key(session_cookie_name):
> > self._sid = cookies[session_cookie_name].value
> >
> > To something like:
> >
> > if cookies.has_key(session_cookie_name):
> > if not secret or type(cookes[session_cookie_name]) \
> > is Cookie.SignedCookie:
> > self._sid = cookies[session_cookie_name].value
> >
> > I'm fairly new to mod_python, so if I'm mistaken then my apologies, and
> > a quick explanation of why would be very much appreciated! ^_^
> >
> > Thanks,
> >
> > - Andy
> >
> Is this correct and should the change suggested appropriate?
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira