Re: [Mailman-Developers] cookies
[Ricardo Kustner]
Hi,
On Tue, May 09, 2000 at 09:12:49PM +0200, Harald Meland wrote:
Please write a patch which puts the string "Cookie could not be set" on the web page so that I can see that pressing submit will not work :-)
i think thats a good point... it would safe some user questions if MM tells exactly why the authorisation failed.
While I agree that such a warning would be nice, I don't think it's possible to do such things with cookies.
it's possible to set a test cookie to see if cookies are enabled...
Ahh, I didn't even think of using multiple cookies :)
If I understand you correctly, you're proposing something like this:
Whenever Mailman is about to write a login page (i.e. the user is not already authenticated), it first issues a
Set-Cookie: Mailman_cookie_test="This cookie is only used to test whether your browser will be able to authenticate with Mailman"; Version=1
HTTP header (If other Mailman cookies set attributes like Path or Domain, the test cookie should mirror these to make the test reflect real usage).
Next, once the user has pressed the "Let me in..." button, Mailman checks whether the Cookie has been sent back. If it hasn't, authentication fails (as the user won't be able to make any changes anyway), and Mailman instructs the user to enable Cookies in her browser before retrying login.
If the test Cookie is present, Mailman should issue a
Set-Cookie: Mailman_cookie_test="clickety click"; Max-Age=0; Version=1
HTTP header (to delete the test cookie, so that the test cookie isn't later confused with test cookies for login attempts at other lists).
Finally, Mailman proceeds with password authentication as usual, possibly resulting in an authentication cookie.
Hmmmm... I guess the test cookie should contain info on what list it is for, as well.
Have I understood you correctly? Does anyone think that implementing this (apart from my misunderstandings, of course :) would be a bad thing?
And, while we're talking about cookies: Does anyone know whether switching from the cookie attribute "Expires" (which was part of the original Netscape cookie proposal) to the RFC2109 cookie attribute "Max-Age" is likely to cause any problems?
I've had a look at Cookie.py, and the value part of the Expires attribute isn't enclosed in double quotes (in accordance with the original Netscape cookie proposal), which I believe might confuse Mailman in some situations where the browser sends back more than one cookie.
Of course, if there are any (major) browsers in use out there that doesn't understand Max-Age, it would be a bad idea to change Mailman.
Hi,
On Wed, May 10, 2000 at 11:04:11PM +0200, Harald Meland wrote:
i think thats a good point... it would safe some user questions if MM tells exactly why the authorisation failed.
While I agree that such a warning would be nice, I don't think it's possible to do such things with cookies.
it's possible to set a test cookie to see if cookies are enabled...
Ahh, I didn't even think of using multiple cookies :) If I understand you correctly, you're proposing something like this: Have I understood you correctly? Does anyone think that implementing this (apart from my misunderstandings, of course :) would be a bad thing?
I haven't really thought about how it could be implemented exactly, but since you can set multiple cookies at once, i was thinking about something like this:
- at the first access to the login page (which shows the password entry) a test cookie is set
- after the password is submitted, a first check is done to see if the test cookie survived... if it's not there, it's safe to assume cookies are not working with this browser... if the test cookie exists, the password is checked, and a auth cookie is generated (exactly like it works now)
you used 2 test cookies in your example, but i'm not sure if that's necessary... or maybe I could forget something though :) also, AFIAK, deleting a cookie can be done by re-setting it with an empty value (though i believe there are some old versions of either IE or Netscape which have a bug with this feature)
And, while we're talking about cookies: Does anyone know whether switching from the cookie attribute "Expires" (which was part of the original Netscape cookie proposal) to the RFC2109 cookie attribute "Max-Age" is likely to cause any problems?
I'm not sure, but I don't trust browsers to be RFC compliant... after all they always make up their own standards and expect others to follow their innovations :)
Of course, if there are any (major) browsers in use out there that doesn't understand Max-Age, it would be a bad idea to change Mailman.
this is the first time I've heard about Max-Age
Ricardo.
--
>> And, while we're talking about cookies: Does anyone know
>> whether switching from the cookie attribute "Expires" (which
>> was part of the original Netscape cookie proposal) to the
>> RFC2109 cookie attribute "Max-Age" is likely to cause any
>> problems?
It appears to, at least with NS.
I've been trying to fix the cookie authentication problem, and have been experimenting with various things, including max-age vs expires. As near as I can tell, with NS 4.73 (and probably earlier), if I set max-age but not expires, NS never saves the cookie to disk. I.e. it treats it like a session cookie, sending it just fine when requested until the browser is exited. The cookie is lost when NS is started up the next time. This is not desirable.
I've only played with MSIE a little bit and it seems to make no difference with it. Neither does it make a difference with Lynx. So, I'm not going to make this change.
Now, what could possibly be confusing some browsers, and causing the unexpected re-authorization problems, might be related to binary data in the cookie value and improper quoting. What Cookie.py does is, if the value is not a string, it pickles the object and sets the value to the pickled representation. This will contain 8-bit data, including possibly quote characters, semi-colons, etc. I have a feeling that some of these combinations are just not handled correctly by some browsers. Subsequent log-ins succeed because the current time is different enough that the cookie value doesn't contain any of those wacky characters.
So I've made two changes. First, I'm using sha instead of md5 to generate the hash of password+current_time+expires_time -- I don't think this'll make any real difference. Second, I'm `hexlifying' the cookie value. This guarantees that the value will contain only characters [0-9][a-f] so no quoting should be necessary and there will be no strange characters to confuse things.
My access to browsers if very limited so I have no idea if this fixes things or not. For NS4.73, MSIE5, and Lynx, it doesn't seem to make the situation /worse/.
I'll be checking these changes in over the next hour or so, so please do an update and see what you think.
-Barry
So I've made two changes. First, I'm using sha instead of md5 to generate the hash of password+current_time+expires_time -- I don't think this'll make any real difference. Second, I'm `hexlifying' the cookie value. This guarantees that the value will contain only characters [0-9][a-f] so no quoting should be necessary and there will be no strange characters to confuse things.
OK, I'm probably missing something again, but this looks completely broken to me. unhexlify is using int(c, 16), but there is no such 2-argument int that I can find in Python 1.5.2.
Barry, the following patch made cookies work again, but they were utterly broken before this patch, and I don't understand how you could have thought they were not...so is there a 2-argument int() in some unreleased Python or something?...
*** Utils.py.orig Thu Jul 20 02:57:17 2000 --- Utils.py Thu Jul 20 02:57:44 2000
*** 627,631 **** def unhexlify(s): acc = [] for i in range(0, len(s), 2): ! acc.append(chr(int(s[i], 16)*16 + int(s[i+1], 16))) return string.join(acc, '') --- 627,631 ---- def unhexlify(s): acc = [] for i in range(0, len(s), 2): ! acc.append(chr(string.atol(s[i], 16)*16 + string.atol(s[i+1], 16))) return string.join(acc, '')
On Thu, Jul 20, 2000 at 03:04:53AM -0700, Dan Mick wrote:
OK, I'm probably missing something again, but this looks completely broken to me. unhexlify is using int(c, 16), but there is no such 2-argument int that I can find in Python 1.5.2.
Indeed, there isn't. You should change it to string.atoi(). (string.atol() will return longs instead of ints)
Barry, the following patch made cookies work again, but they were utterly broken before this patch, and I don't understand how you could have thought they were not...so is there a 2-argument int() in some unreleased Python or something?...
Yes. Python 2.0 has a 2-argument int().
"DM" == Dan Mick Dan.Mick@west.sun.com writes:
DM> OK, I'm probably missing something again, but this looks
DM> completely broken to me. unhexlify is using int(c, 16), but
DM> there is no such 2-argument int that I can find in Python
DM> 1.5.2.
DM> Barry, the following patch made cookies work again, but they
DM> were utterly broken before this patch, and I don't understand
DM> how you could have thought they were not...so is there a
DM> 2-argument int() in some unreleased Python or something?...
Ack! Yup, the next version of Python will have an int() that accepts two arguments. In fact, the entire string module will be largely unnecessary (string objects have methods). Try this patch, which should also be moderately faster.
Thanks, -Barry
-------------------- snip snip -------------------- Index: Utils.py =================================================================== RCS file: /cvsroot/mailman/mailman/Mailman/Utils.py,v retrieving revision 1.93 diff -u -r1.93 Utils.py --- Utils.py 2000/07/19 20:20:39 1.93 +++ Utils.py 2000/07/20 12:47:03 @@ -615,9 +615,9 @@ # unhexlify(hexlify(s)) == s def hexlify(s): acc = []
- def munge(byte, acc=acc, a=ord('a'), z=ord('0')):
if byte > 9: acc.append(byte+a-10)else: acc.append(byte+z)
- def munge(byte, append=acc.append, a=ord('a'), z=ord('0')):
if byte > 9: append(byte+a-10)
for c in s: hi, lo = divmod(ord(c), 16) munge(hi)else: append(byte+z)
@@ -626,6 +626,9 @@
def unhexlify(s): acc = []
- append = acc.append
- # In Python 2.0, we can use the int() built-in
- int16 = string.atol for i in range(0, len(s), 2):
acc.append(chr(int(s[i], 16)*16 + int(s[i+1], 16)))
return string.join(acc, '')append(chr(int16(s[i], 16)*16 + int16(s[i+1], 16)))
participants (5)
-
bwarsaw＠beopen.com -
Dan Mick -
Harald Meland -
Ricardo Kustner -
Thomas Wouters