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

Priority:	CurrentState:	AppliesTo:	Component:	WaitingFor:
Normal	Closed	Engine

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.

-- 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