From 52c6bf04194445975ababe1fcaa9c9a6d7f899b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaime=20Pe=CC=81rez?= <jaime.perez@uninett.no>
Date: Mon, 4 Jul 2016 15:42:00 +0200
Subject: [PATCH] bugfix: Make sure SimpleSAML_Session::getSessionFromRequest()
always raises an exception when a transient session is used due to a
misconfiguration or a temporary failure fetching an existing session.
Transient sessions are just an exceptional event, and they shouldn't be treated as regular sessions. Therefore, if we are trying to get the current session and end up with a transient one, that's because an error occurred and we should raise an exception. Since exceptions due to secure cookies trying to be set via an insecure channel are likely to be misconfigurations, we treat them like that, raising a SimpleSAML\Error\CriticalConfigurationError.
Additionally, we capture exceptions in the SimpleSAML\Logger::flush() method, ensuring the error reported in #413 doesn't happen again.
This resolves #356.
---
lib/SimpleSAML/Logger.php | 8 +++++++-
lib/SimpleSAML/Session.php | 13 +++++++++++--
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/lib/SimpleSAML/Logger.php b/lib/SimpleSAML/Logger.php
index a483385f4..f838db1df 100644
--- a/lib/SimpleSAML/Logger.php
+++ b/lib/SimpleSAML/Logger.php
@@ -267,7 +267,13 @@ class Logger
*/
public static function flush()
{
- $s = \SimpleSAML_Session::getSessionFromRequest();
+ try {
+ $s = \SimpleSAML_Session::getSessionFromRequest();
+ } catch (\Exception $e) {
+ // loading session failed. We don't care why, at this point we have a transient session, so we use that
+ self::error('Cannot load or create session: '.$e->getMessage());
+ $s = \SimpleSAML_Session::getSessionFromRequest();
+ }
self::$trackid = $s->getTrackID();
self::$shuttingDown = true;
diff --git a/lib/SimpleSAML/Session.php b/lib/SimpleSAML/Session.php
index 6d325d900..2ffd0e77d 100644
--- a/lib/SimpleSAML/Session.php
+++ b/lib/SimpleSAML/Session.php
@@ -205,8 +205,8 @@ class SimpleSAML_Session
* session here. Therefore, use just a transient session and throw the exception for someone else to handle
* it.
*/
- SimpleSAML\Logger::error('Error loading session: '.$e->getMessage());
self::useTransientSession($e);
+ SimpleSAML\Logger::error('Error loading session: '.$e->getMessage());
}
// if getSession() found it, use it
@@ -229,8 +229,17 @@ class SimpleSAML_Session
self::load(new SimpleSAML_Session());
} catch (\SimpleSAML\Error\CannotSetCookie $e) {
// can't create a regular session because we can't set cookies. Use transient.
- SimpleSAML\Logger::error('Error creating session: '.$e->getMessage());
+ $c = SimpleSAML_Configuration::getInstance();
self::useTransientSession();
+
+ if ($e->getCode() === \SimpleSAML\Error\CannotSetCookie::SECURE_COOKIE) {
+ throw new \SimpleSAML\Error\CriticalConfigurationError(
+ $e->getMessage(),
+ null,
+ $c->toArray()
+ );
+ }
+ SimpleSAML\Logger::error('Error creating session: '.$e->getMessage());
}
// we must have a session now, either regular or transient
--
GitLab