Item10061: Cookies not properly forced to be secure if HTTPS connection

pencil
Priority: Normal
Current State: Closed
Released In: 1.1.3
Target Release: patch
Applies To: Engine
Component:
Branches:
Reported By: DaveHayes
Waiting For:
Last Change By: KennethLavrsen
Despite SERVER_PORT being 443, Foswiki fails to set the "secure" value on cookies sent back from Foswiki.

I've traced this down as far as finding that the Foswiki::Request::cookie() subroutine is never really being called with a defined $value. I even tried a login and a refresh of the wiki page after dumping cookies in my browser.

Is there something I am missing here? There are plenty of workarounds on the browser side, but I would like to be extra safe.

-- DaveHayes - 20 Nov 2010

Well I did miss another place, in Foswiki/Validation/getCookie() you folks also create a cookie, but no attempt is made to see if the session is secure at that point.

More as I find it. big grin

-- DaveHayes - 20 Nov 2010

Also in Foswiki/LoginManager/_addSessionCookieToResponse() and again no attempt to see if the transport (not the session, my mistake above) is secure or not.

-- DaveHayes - 20 Nov 2010

The following patch appears to fix the problem. It's a hack to be sure, likely the check for a secure transport should be abstracted to the engine code and called through the engine, but I bet this works for all cases anyway:

--- a/lib/Foswiki/LoginManager.pm
+++ b/lib/Foswiki/LoginManager.pm
@@ -814,7 +814,8 @@ sub _addSessionCookieToResponse {
         -value    => $this->{_cgisession}->id(),
         -path     => '/',
         -domain   => $Foswiki::cfg{Sessions}{CookieRealm} || '',
-        -httponly => 1
+        -httponly => 1,
+   -secure => ( ((uc( $ENV{HTTPS} ) eq 'ON') || ($ENV{SERVER_PORT} == 443)) ? 1 : 0),
     );
 
     # An expiry time is only set if the session has the REMEMBER variable
diff --git a/lib/Foswiki/Validation.pm b/lib/Foswiki/Validation.pm
index b9be95e..0ced8ab 100644
--- a/lib/Foswiki/Validation.pm
+++ b/lib/Foswiki/Validation.pm
@@ -150,6 +150,7 @@ sub getCookie {
         -value => $secret,
         -path  => '/',
         -httponly => 0,    # we *want* JS to be able to read it!
+   -secure => ( ((uc( $ENV{HTTPS} ) eq 'ON') || ($ENV{SERVER_PORT} == 443)) ? 1 : 0),
     );
 
     return $cookie;

-- DaveHayes - 20 Nov 2010

it sure does cry out for an abstraction :/

-- SvenDowideit - 13 Dec 2010

We already have such an abstraction: Foswiki::Request::secure(). %ENV is guaranteed to exist only in CGI environments (and at best, FastCGI).

I'm reopening this to make the change.

-- GilmarSantosJr - 27 Feb 2011

 
Topic revision: r19 - 16 Apr 2011, KennethLavrsen
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy