[shibboleth-sp2] 36/89: SSPCPP-699 - Versioned session reads broken with memcached storage
Ferenc Wágner
wferi at moszumanska.debian.org
Thu Sep 1 09:24:06 UTC 2016
This is an automated email from the git hooks/post-receive script.
wferi pushed a commit to branch master
in repository shibboleth-sp2.
commit 92380630c4c151f5bf3205ed816a06ef018c52b2
Author: Scott Cantor <cantor.2 at osu.edu>
Date: Tue May 31 13:30:08 2016 -0400
SSPCPP-699 - Versioned session reads broken with memcached storage
https://issues.shibboleth.net/jira/browse/SSPCPP-699
---
memcache-store/memcache-store.cpp | 21 ++++++++--------
shibsp/impl/StorageServiceSessionCache.cpp | 39 +++++++++++++++++++++---------
2 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/memcache-store/memcache-store.cpp b/memcache-store/memcache-store.cpp
index 60e2802..4dc6f2f 100644
--- a/memcache-store/memcache-store.cpp
+++ b/memcache-store/memcache-store.cpp
@@ -496,20 +496,19 @@ int MemcacheStorageService::readString(const char* context, const char* key, str
if (!found)
return 0;
+ mc_record rec;
+ if (pexpiration || pvalue)
+ deserialize(value, rec);
+
+ if (pexpiration)
+ *pexpiration = rec.expiration;
+
if (version && rec_version <= (uint32_t)version)
return version;
- if (pexpiration || pvalue) {
- mc_record rec;
- deserialize(value, rec);
-
- if (pexpiration)
- *pexpiration = rec.expiration;
-
- if (pvalue)
- *pvalue = rec.value;
- }
-
+ if (pvalue)
+ *pvalue = rec.value;
+
return rec_version;
}
diff --git a/shibsp/impl/StorageServiceSessionCache.cpp b/shibsp/impl/StorageServiceSessionCache.cpp
index 1e6739d..b6d6642 100644
--- a/shibsp/impl/StorageServiceSessionCache.cpp
+++ b/shibsp/impl/StorageServiceSessionCache.cpp
@@ -528,7 +528,7 @@ void StoredSession::validate(const Application& app, const char* client_addr, ti
// Versioned read, since we already have the data in hand if it's current.
string record;
- time_t lastAccess;
+ time_t lastAccess = 0;
int curver = m_obj["version"].integer();
int ver = m_cache->m_storage->readText(getID(), "session", &record, &lastAccess, curver);
if (ver == 0) {
@@ -537,6 +537,10 @@ void StoredSession::validate(const Application& app, const char* client_addr, ti
}
if (timeout) {
+ if (lastAccess == 0) {
+ m_cache->m_log.error("session (ID: %s) did not report time of last access", getID());
+ throw RetryableProfileException("Your session has expired, and you must re-authenticate.");
+ }
// Adjust for expiration to recover last access time and check timeout.
unsigned long cacheTimeout = m_cache->getCacheTimeout(app);
lastAccess -= cacheTimeout;
@@ -619,7 +623,7 @@ void StoredSession::validate(const Application& app, const char* client_addr, ti
throw IOException("Unable to update stored session, exceeded retry limit.");
}
m_cache->m_log.warn("storage service indicates the record is out of sync, updating with a fresh copy...");
- ver = m_cache->m_storage->readText(getID(), "session", &record, nullptr);
+ ver = m_cache->m_storage->readText(getID(), "session", &record);
if (!ver) {
m_cache->m_log.error("readText failed on StorageService for session (%s)", getID());
throw IOException("Unable to read back stored session.");
@@ -712,7 +716,7 @@ void StoredSession::addAttributes(const vector<Attribute*>& attributes)
throw IOException("Unable to update stored session, exceeded retry limit.");
}
m_cache->m_log.warn("storage service indicates the record is out of sync, updating with a fresh copy...");
- ver = m_cache->m_storage->readText(getID(), "session", &record, nullptr);
+ ver = m_cache->m_storage->readText(getID(), "session", &record);
if (!ver) {
m_cache->m_log.error("readText failed on StorageService for session (%s)", getID());
throw IOException("Unable to read back stored session.");
@@ -749,7 +753,7 @@ const Assertion* StoredSession::getAssertion(const char* id) const
return i->second.get();
string tokenstr;
- if (!m_cache->m_storage->readText(getID(), id, &tokenstr, nullptr))
+ if (!m_cache->m_storage->readText(getID(), id, &tokenstr))
throw FatalProfileException("Assertion not found in cache.");
// Parse and bind the document into an XMLObject.
@@ -786,8 +790,8 @@ void StoredSession::addAssertion(Assertion* assertion)
m_cache->m_log.debug("adding assertion (%s) to session (%s)", id.get(), getID());
- time_t exp;
- if (!m_cache->m_storage->readText(getID(), "session", nullptr, &exp))
+ time_t exp = 0;
+ if (!m_cache->m_storage->readText(getID(), "session", nullptr, &exp) || exp == 0)
throw IOException("Unable to load expiration time for stored session.");
ostringstream tokenstr;
@@ -835,7 +839,7 @@ void StoredSession::addAssertion(Assertion* assertion)
throw IOException("Unable to update stored session, exceeded retry limit.");
}
m_cache->m_log.warn("storage service indicates the record is out of sync, updating with a fresh copy...");
- ver = m_cache->m_storage->readText(getID(), "session", &record, nullptr);
+ ver = m_cache->m_storage->readText(getID(), "session", &record);
if (!ver) {
m_cache->m_log.error("readText failed on StorageService for session (%s)", getID());
m_cache->m_storage->deleteText(getID(), id.get());
@@ -1061,7 +1065,7 @@ void SSCache::insert(const char* key, time_t expires, const char* name, const ch
// Since we can't guarantee uniqueness, check for an existing record.
string record;
- time_t recordexp;
+ time_t recordexp = 0;
int ver = m_storage_lite->readText("NameID", name, &record, &recordexp);
if (ver > 0) {
// Existing record, so we need to unmarshall it.
@@ -1623,12 +1627,17 @@ Session* SSCache::find(const Application& app, const char* key, const char* clie
m_log.debug("searching for session (%s)", key);
DDF obj;
- time_t lastAccess;
+ time_t lastAccess = 0;
string record;
int ver = m_storage->readText(key, "session", &record, &lastAccess);
if (!ver)
return nullptr;
+ if (lastAccess = 0) {
+ m_log.error("session (ID: %s) did not report time of last access", key);
+ throw RetryableProfileException("Your session has expired, and you must re-authenticate.");
+ }
+
m_log.debug("reconstituting session and checking validity");
istringstream in(record);
@@ -1941,7 +1950,7 @@ void SSCache::receive(DDF& in, ostream& out)
// Do an unversioned read.
string record;
- time_t lastAccess;
+ time_t lastAccess = 0;
if (!m_storage->readText(key, "session", &record, &lastAccess)) {
m_log.debug("session not found in cache (%s)", key);
DDF ret(nullptr);
@@ -1949,6 +1958,10 @@ void SSCache::receive(DDF& in, ostream& out)
out << ret;
return;
}
+ else if (lastAccess == 0) {
+ m_log.error("session (ID: %s) did not report time of last access", key);
+ throw RetryableProfileException("Your session has expired, and you must re-authenticate.");
+ }
// Adjust for expiration to recover last access time and check timeout.
unsigned long cacheTimeout = getCacheTimeout(*app);
@@ -1995,13 +2008,17 @@ void SSCache::receive(DDF& in, ostream& out)
// Do a read. May be unversioned if we need to bind a new client address.
string record;
- time_t lastAccess;
+ time_t lastAccess = 0;
int curver = in["version"].integer();
int ver = m_storage->readText(key, "session", &record, &lastAccess, client_addr ? 0 : curver);
if (ver == 0) {
m_log.info("session (ID: %s) no longer in storage", key);
throw RetryableProfileException("Your session has expired, and you must re-authenticate.");
}
+ else if (lastAccess == 0) {
+ m_log.error("session (ID: %s) did not report time of last access", key);
+ throw RetryableProfileException("Your session has expired, and you must re-authenticate.");
+ }
// Adjust for expiration to recover last access time and check timeout.
unsigned long cacheTimeout = getCacheTimeout(*app);
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-shibboleth/shibboleth-sp2.git
More information about the Pkg-shibboleth-devel
mailing list