[Tickets #12590] Re: Not possible to login without cookies

[noreply at bugs.horde.org]

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/12590
------------------------------------------------------------------------------
  Ticket             | 12590
  Updated By         | Michael Slusarz <slusarz at horde.org>
  Summary            | Not possible to login without cookies
  Queue              | IMP
  Version            | Git master
  Type               | Bug
  State              | Assigned
-Priority           | 2. Medium
+Priority           | 3. High
  Milestone          |
  Patch              |
-Owners             |
+Owners             | Horde Developers, Michael Slusarz
------------------------------------------------------------------------------


Michael Slusarz <slusarz at horde.org> (2013-08-20 22:11) wrote:

Yuck yuck yuck.

This opened up a WHOLE can of worms today.  Let's see if I can recap...

It is true that non-cookie based sessions didn't work (not sure how I  
got this work a few days ago.  Thinking that I must not have disabled  
cookies entirely on the server).  Long story short - this is the  
long-standing issue that when IMP is used as the authentication  
handler, the password is first encoded using the secret key BEFORE the  
session is started.  This isn't a deal for cookie-based sessions  
(since the auth key was changed at the beginning of login.php, this  
value is used for the entire authentication access and doesn't change  
when the session ID is reset).  But non-cookie based sessions is  
problematic since the secret key is entirely tied to the session ID,  
which changes halfway through that request.

If that wasn't enough, I then realized that non-cookie based sessions  
can't handle our session regeneration code either - this code was  
added to prevent CSRF attacks.  When the session ID changes, all data  
encrypted using the old session ID would be invalidated, causing a  
logout (again, this isn't an issue with cookie-based sessions since  
the auth key doesn't change when the session ID does).

To prevent the latter, it is necessary to tie the session backend with  
storage of secret-encrypted data.  We need to track all session data  
keys that contain encrypted data, so we can re-encrypt whenever the  
session ID changes (this is only necessary on non-cookie based  
sessions).  This has the benefit of making storage encrypted data  
easier: now to store encrypted data you just need to add the  
Horde_Session::ENCRYPT mask when set()'ing the session data.  There is  
no need to know that the data is encrypted when get()'ing the data -  
this is all transparent, since we necessarily need to track the keys  
that are encrypted for the reasons described above.

Part 2 is shifting the encryption of password information in IMP from  
authentication time to serialization time.  By the time we serialize  
data, we necessarily need to have created the session.  This is  
accomplished by storing all password data via session (necessary since  
the secret key may change during the session).  This also required  
changing the way passwords are stored in the Horde_Imap_Client  
serialized object.  Before, we were passing the secret key needed to  
encrypt/decrypt the password.  But this suffers from the same problems  
as mentioned above.  Instead, this method has been deprecated and we  
now allow the 'password' parameter to be a callable object that  
returns the password.  This allows maximum flexibility - storage of  
the password can be accomplished in any manner and is not limited to  
what Horde_Imap_Client can do.

Long story short ... this should fix non-cookie based sessions.   
Additionally, this should fix the intermittent issues users reported  
about their passwords being "lost" when authenticated via IMP.  The  
combination of these 2 fixes are extremely important, and why this  
change -- although invasive -- needs to be immediately applied rather  
than waiting for another point release.

That being said, because of the invasive nature of these changes, I  
would like feedback/testing to make sure this works for everyone.  The  
code can be found in the "bug_12590" branch I have pushed to  
git.horde.org.  It would be great if people (especially devs) can look  
this over so this can be merged ASAP.