[Zope-PAS] PluggableAuthService colon-in-cookie bug

[Daniel Doerr]

Hey guys,

recently I discovered a bug (or a feature???!!) in the way credentials 
are stored in cookies with PluggableAuthService. When somebody includes 
a colon in his password, the authentication for this user doesn't work 
anymore because of this code in 
PluggableAuthService.plugins.CookieAuthHelper.extractCredentials() 
line 122:

            cookie_val = decodestring(unquote(cookie))
            login, password = cookie_val.split(':')

.. which will fail if there are more but one colons in cookie_val. So, 
basically, nobody with a colon in his loginname or password can login 
at zope anymore. My first suggestion of bugfixing this unwanted 
behaviour was

            cookie_val = decodestring(unquote(cookie))
            login = cookie_val[:cookie_val.find(':')]
            password = cookie_val[cookie_val.find(':')+1:]

.. but then I realized that there also can be colons in the loginname 
as well since it shouldn't be part of PAS' job to decide whether a 
loginname or password is valid or not (and, in fact, PAS does not 
check the validity of the credentials before deciding to join or split 
them by a colon...).

So I wrote a bugfix, which solves this problem by encoding the 
loginname and password before delivering these to credentials-update 
plugins (which happens in PluggableAuthService.updateCredentials line 
1080). In addition, credentials have to be separately decoded in 
CookieAuthHelper.extractCredentials.

Patch for PluggableAuthService.py:
28a29
> from base64 import encodestring
1080c1081,1083
<             updater.updateCredentials(request, response, login, 
new_password)
---
>             updater.updateCredentials(request, response, \
>                     encodestring(login), \
>                     encodestring(new_password))


Patch for plugins/CookieAuthHelper.py
125,126c125,126
<             creds['login'] = login
<             creds['password'] = password
---
>             creds['login'] = decodestring(login)
>             creds['password'] = decodestring(password)

These bugfixes work very well but being aware that these are bugfixes on 
two different levels of the authentication process, I do not see any other 
possibility to fix this problem, because CookieAuthHandler extracts 
credentials from the request as well, which IMHO shouldn't be part of 
this plugin either...

After trying to get in contact with Tres Seaver directly, I finally found 
this awesome news group to post on.. If somebody can help me out 
explaining this esoteric behaviour of PAS or can give me an advice to 
avoid this problem I would be very grateful!

Regards,
-dany