Item10061: Cookies not properly forced to be secure if HTTPS connection
Priority: Normal
Current State: Closed
Released In: 1.1.3
Target Release: patch
Applies To: Engine
Component:
Branches:
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