[#13284] Horde_Secret: Only store key in cookies if cookies are in use
Summary Horde_Secret: Only store key in cookies if cookies are in use
Queue Horde Framework Packages
Queue Version Git master
Type Bug
State Assigned
Priority 1. Low
Owners Horde Developers, slusarz@horde.org
Requester thomas.jarosch@intra2net.com
Created 2014-06-23 (1917 days ago)
Due
Updated 2016-02-02 (1328 days ago)
Assigned 2014-07-04 (1906 days ago)
Resolved
Milestone
Patch Yes

Comments
Thomas Jarosch <thomas.jarosch@intra2net.com> 2014-06-23 14:19:05
Hi,

Horde_Secret currently stores the generated key in a cookie even when 
cookies are not used for the session id. This happens in setKey() and 
getKey().

The problem is later on in clearKey(): That one removes the key cookie 
only if session cookies are in use, too.

The attached patch fixes clearKey() and also avoids setting the cookie 
at all for non-cookie sessions.

Cheers,
Thomas


Git Commit <commits@lists.horde.org> 2014-06-24 22:12:05
Changes have been made in Git (master):

commit 6c501804b267e1559cb16731aaaef9f976ec25fb
Author: Michael M Slusarz <slusarz@horde.org>
Date:   Tue Jun 24 16:06:29 2014 -0600

     [mms] Only store keys in cookie if cookies are in use (Bug 
#13284; thomas.jarosch@intra2net.com).

  framework/Secret/lib/Horde/Secret.php |   24 +++++++++++++-----------
  framework/Secret/package.xml          |    2 ++
  2 files changed, 15 insertions(+), 11 deletions(-)

http://github.com/horde/horde/commit/6c501804b267e1559cb16731aaaef9f976ec25fb

Michael Slusarz <slusarz@horde.org> 2014-06-24 22:12:42
Horde_Secret 2.0.3.

Thomas Jarosch <thomas.jarosch@intra2net.com> 2014-06-25 07:28:01
> Horde_Secret 2.0.3.

nice, you even eliminated the $set variable altogether :)
clearKey() looks also much better than my implementation.


Git Commit <commits@lists.horde.org> 2014-07-04 12:15:10
Changes have been made in Git (master):

commit 512a25022a1fa00659372bada8997402a7da01b8
Author: Jan Schneider <jan@horde.org>
Date:   Fri Jul 4 14:14:08 2014 +0200

     Revert "[mms] Only store keys in cookie if cookies are in use 
(Bug #13284; thomas.jarosch@intra2net.com)."

     This reverts commit 6c501804b267e1559cb16731aaaef9f976ec25fb.

     This completely broke authentication with any DAV access.

     Conflicts:
             framework/Secret/package.xml

  framework/Secret/lib/Horde/Secret.php |   24 +++++++++++-------------
  1 files changed, 11 insertions(+), 13 deletions(-)

http://github.com/horde/horde/commit/512a25022a1fa00659372bada8997402a7da01b8

Thomas Jarosch <thomas.jarosch@intra2net.com> 2014-07-04 13:30:34
Side note: Cookies are officially not supported for WebDAV sessions (yunosh)

See also:
http://comments.gmane.org/gmane.comp.php.sabredav/65

"2. Don't use sessions in WebDAV. They are not supported in most 
clients, and generally a terrible idea. HTTP is supposed to be 
stateless. Only when your client is a browser, a (session-)cookie is 
acceptable."

and

http://stackoverflow.com/questions/14499686/mac-os-x-does-not-send-cookies-to-webdav-resource

We probably need to come up with a more clever storage mechanism.
Funny the previous code worked at all for DAV.

Wild guess: The webdav access generates a new "session id" on every 
page access since it does not transport the session id cookie. This 
breaks Horde_Secret because it can no longer decrypt the data of the 
previous page access.