Fixed
Pinned fields
Click on the next to a field label to start pinning.
Details
Details
Assignee
Scott Cantor
Scott CantorReporter
voetsjoeba+protectnetwork@gmail.com
voetsjoeba+protectnetwork@gmail.comComponents
Fix versions
Affects versions
Created May 11, 2016 at 6:59 PM
Updated June 29, 2016 at 4:22 PM
Resolved May 31, 2016 at 5:32 PM
When using the memcached storage service, versioned session reads may fail arbitrarily due to a combination of 2 issues:
MemcacheStorageService::readString fails to write out the entry's expiration timestamp if a versioned read is used and the stored version is not newer.
The logic in StorageServiceSessionCache does not initialize the lastAccess stack variable prior to passing it as an out parameter to receive the value, and assumes it is present after the call.
int MemcacheStorageService::readString(const char* context, const char* key, string* pvalue, time_t* pexpiration, int version) { ... 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; } return rec_version; }void SSCache::receive(DDF& in, ostream& out) { ... string record; time_t lastAccess; 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."); } // timeout decision logic using lastAccess ...The result is that when a versioned read is done and the value from Memcached is not newer, the lastAccess variable contains arbitrary stack garbage and causes random session expiration decisions.
The particular case I ran into was with the out-of-process touch:: handler in SSCache::receive at StorageServiceSessionCache.cpp:1998, but the same logic is repeated multiple times throughout.
I imagine both sides need fixing: the Memcached storage service should return the expiration timestamp unconditionally (as is the case for MemoryStorageService), and the calling logic should initialize the lastAccess values to consistently cause an expiration decision failure if the storage service did not update them (and perhaps log an error if an initial value can be chosen that is known to be impossible/nonsensical and checked for afterwards?).
Also, for future storage service implementations, perhaps the StorageService readText/String contract in XMLTooling could be more explicit about which values are and are not expected to be written when faced with versioned reads? The documentation on StorageService::readString currently reads "if > 0, only copy back data if newer than supplied version", but it's not clear whether "data" refers to the value, or the expiration timestamp, or both.