From b766b9fa7005530d14a491dac86edddf5253c636 Mon Sep 17 00:00:00 2001 From: Scott Cantor Date: Tue, 9 Oct 2012 03:07:22 +0000 Subject: [PATCH] https://issues.shibboleth.net/jira/browse/SSPCPP-507 --- odbc-store/odbc-store.cpp | 52 ++++++++++++++++++++-------- shibsp/impl/StorageServiceSessionCache.cpp | 55 +++++++++++++++++++++++++----- 2 files changed, 84 insertions(+), 23 deletions(-) diff --git a/odbc-store/odbc-store.cpp b/odbc-store/odbc-store.cpp index 4357276..48bbe8e 100644 --- a/odbc-store/odbc-store.cpp +++ b/odbc-store/odbc-store.cpp @@ -63,7 +63,7 @@ using namespace boost; using namespace std; #define PLUGIN_VER_MAJOR 1 -#define PLUGIN_VER_MINOR 0 +#define PLUGIN_VER_MINOR 1 #define LONGDATA_BUFLEN 16384 @@ -84,7 +84,7 @@ CREATE TABLE strings ( context varchar(255) not null, id varchar(255) not null, expires datetime not null, - version smallint not null, + version int not null, value varchar(255) not null, PRIMARY KEY (context, id) ) @@ -93,7 +93,7 @@ CREATE TABLE texts ( context varchar(255) not null, id varchar(255) not null, expires datetime not null, - version smallint not null, + version int not null, value text not null, PRIMARY KEY (context, id) ) @@ -207,6 +207,7 @@ namespace { SQLHENV m_henv; string m_connstring; long m_isolation; + bool m_wideVersion; vector m_retries; }; @@ -271,7 +272,7 @@ namespace { ODBCStorageService::ODBCStorageService(const DOMElement* e) : m_log(Category::getInstance("XMLTooling.StorageService")), m_caps(XMLHelper::getAttrInt(e, 255, contextSize), XMLHelper::getAttrInt(e, 255, keySize), XMLHelper::getAttrInt(e, 255, stringSize)), m_cleanupInterval(XMLHelper::getAttrInt(e, 900, cleanupInterval)), - cleanup_thread(nullptr), shutdown(false), m_henv(SQL_NULL_HENV), m_isolation(SQL_TXN_SERIALIZABLE) + cleanup_thread(nullptr), shutdown(false), m_henv(SQL_NULL_HENV), m_isolation(SQL_TXN_SERIALIZABLE), m_wideVersion(false) { #ifdef _DEBUG xmltooling::NDC ndc("ODBCStorageService"); @@ -321,6 +322,11 @@ ODBCStorageService::ODBCStorageService(const DOMElement* e) : m_log(Category::ge m_log.crit("unknown database version: %d.%d", v.first, v.second); throw XMLToolingException("Unknown database version for ODBC StorageService."); } + + if (v.first > 1 || v.second > 0) { + m_log.info("using 32-bit int type for version fields in tables"); + m_wideVersion = true; + } // Load any retry errors to check. e = XMLHelper::getNextSiblingElement(e, RetryOnError); @@ -542,20 +548,30 @@ int ODBCStorageService::readRow(const char *table, const char* context, const ch } SQLSMALLINT ver; + SQLINTEGER widever; SQL_TIMESTAMP_STRUCT expiration; - SQLBindCol(stmt, 1, SQL_C_SSHORT, &ver, 0, nullptr); + if (m_wideVersion) + SQLBindCol(stmt, 1, SQL_C_SLONG, &widever, 0, nullptr); + else + SQLBindCol(stmt, 1, SQL_C_SSHORT, &ver, 0, nullptr); if (pexpiration) SQLBindCol(stmt, 2, SQL_C_TYPE_TIMESTAMP, &expiration, 0, nullptr); - if ((sr = SQLFetch(stmt)) == SQL_NO_DATA) + if ((sr = SQLFetch(stmt)) == SQL_NO_DATA) { + if (m_log.isDebugEnabled()) + m_log.debug("search returned no data (t=%s, c=%s, k=%s)", table, context, key); return 0; + } if (pexpiration) *pexpiration = timeFromTimestamp(expiration); - if (version == ver) + if (version == (m_wideVersion ? widever : ver)) { + if (m_log.isDebugEnabled()) + m_log.debug("versioned search detected no change (t=%s, c=%s, k=%s)", table, context, key); return version; // nothing's changed, so just echo back the version + } if (pvalue) { SQLLEN len; @@ -570,7 +586,7 @@ int ODBCStorageService::readRow(const char *table, const char* context, const ch } } - return ver; + return (m_wideVersion ? widever : ver); } int ODBCStorageService::updateRow(const char *table, const char* context, const char* key, const char* value, time_t expiration, int version) @@ -608,15 +624,23 @@ int ODBCStorageService::updateRow(const char *table, const char* context, const } SQLSMALLINT ver; - SQLBindCol(stmt, 1, SQL_C_SSHORT, &ver, 0, nullptr); + SQLINTEGER widever; + if (m_wideVersion) + SQLBindCol(stmt, 1, SQL_C_SLONG, &widever, 0, nullptr); + else + SQLBindCol(stmt, 1, SQL_C_SSHORT, &ver, 0, nullptr); if ((sr = SQLFetch(stmt)) == SQL_NO_DATA) { return 0; } // Check version? - if (version > 0 && version != ver) { + if (version > 0 && version != (m_wideVersion ? widever : ver)) { return -1; } + else if ((m_wideVersion && widever == INT_MAX) || (!m_wideVersion && ver == 32767)) { + m_log.error("record version overflow (t=%s, c=%s, k=%s)", table, context, key); + throw IOException("Version overflow, record in ODBC StorageService could not be updated."); + } SQLFreeHandle(SQL_HANDLE_STMT, stmt); stmt = getHSTMT(conn); @@ -651,11 +675,11 @@ int ODBCStorageService::updateRow(const char *table, const char* context, const else sr = SQLBindParam(stmt, 1, SQL_C_CHAR, SQL_VARCHAR, 255, 0, const_cast(value), &b_ind); if (!SQL_SUCCEEDED(sr)) { - m_log.error("SQLBindParam failed (context = %s)", context); + m_log.error("SQLBindParam failed (value = %s)", value); log_error(stmt, SQL_HANDLE_STMT); throw IOException("ODBC StorageService failed to update record."); } - m_log.debug("SQLBindParam succeeded (context = %s)", context); + m_log.debug("SQLBindParam succeeded (value = %s)", value); } int attempts = 3; @@ -668,10 +692,10 @@ int ODBCStorageService::updateRow(const char *table, const char* context, const return 0; // went missing? else if (SQL_SUCCEEDED(sr)) { m_log.debug("SQLExecute of update succeeded"); - return ver + 1; + return (m_wideVersion ? widever : ver) + 1; } - m_log.error("update of record failed (t=%s, c=%s, k=%s", table, context, key); + m_log.error("update of record failed (t=%s, c=%s, k=%s)", table, context, key); logres = log_error(stmt, SQL_HANDLE_STMT); } while (attempts && logres.first); diff --git a/shibsp/impl/StorageServiceSessionCache.cpp b/shibsp/impl/StorageServiceSessionCache.cpp index f1af3f6..31c225a 100644 --- a/shibsp/impl/StorageServiceSessionCache.cpp +++ b/shibsp/impl/StorageServiceSessionCache.cpp @@ -133,7 +133,9 @@ namespace shibsp { const set* indexes, time_t expires, vector& sessions - ); + ) { + return _logout(app, issuer, nameid, indexes, expires, sessions, 0); + } bool matches( const Application& app, const HTTPRequest& request, @@ -203,7 +205,16 @@ namespace shibsp { private: #ifndef SHIBSP_LITE // maintain back-mappings of NameID/SessionIndex -> session key - void insert(const char* key, time_t expires, const char* name, const char* index); + void insert(const char* key, time_t expires, const char* name, const char* index, short attempts=0); + vector::size_type _logout( + const Application& app, + const EntityDescriptor* issuer, + const saml2::NameID& nameid, + const set* indexes, + time_t expires, + vector& sessions, + short attempts + ); bool stronglyMatches(const XMLCh* idp, const XMLCh* sp, const saml2::NameID& n1, const saml2::NameID& n2) const; LogoutEvent* newLogoutEvent(const Application& app) const; @@ -560,6 +571,7 @@ void StoredSession::validate(const Application& app, const char* client_addr, ti // We may need to write back a new address into the session using a versioned update loop. if (client_addr) { + short attempts = 0; do { const char* saddr = nullptr; if (strchr(client_addr, ':')) @@ -608,6 +620,10 @@ void StoredSession::validate(const Application& app, const char* client_addr, ti } else if (ver < 0) { // Out of sync. + if (++attempts > 10) { + m_cache->m_log.error("failed to bind client address, update attempts exceeded limit"); + 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); if (!ver) { @@ -654,6 +670,7 @@ void StoredSession::addAttributes(const vector& attributes) m_cache->m_log.debug("adding attributes to session (%s)", getID()); int ver; + short attempts = 0; do { DDF attr; DDF attrs = m_obj["attributes"]; @@ -696,6 +713,10 @@ void StoredSession::addAttributes(const vector& attributes) } else if (ver < 0) { // Out of sync. + if (++attempts > 10) { + m_cache->m_log.error("failed to update stored session, update attempts exceeded limit"); + 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); if (!ver) { @@ -781,6 +802,7 @@ void StoredSession::addAssertion(Assertion* assertion) throw IOException("Attempted to insert duplicate assertion ID into session."); int ver; + short attempts = 0; do { DDF token = DDF(nullptr).string(id.get()); m_obj["assertions"].add(token); @@ -814,6 +836,10 @@ void StoredSession::addAssertion(Assertion* assertion) } else if (ver < 0) { // Out of sync. + if (++attempts > 10) { + m_cache->m_log.error("failed to update stored session, update attempts exceeded limit"); + 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); if (!ver) { @@ -1018,8 +1044,11 @@ void SSCache::test() m_storage->deleteString("SessionCacheTest", temp.get()); } -void SSCache::insert(const char* key, time_t expires, const char* name, const char* index) +void SSCache::insert(const char* key, time_t expires, const char* name, const char* index, short attempts) { + if (attempts > 10) + throw IOException("Exceeded retry limit."); + string dup; unsigned int storageLimit = m_storage_lite->getCapabilities().getKeySize(); if (strlen(name) > storageLimit) { @@ -1061,12 +1090,12 @@ void SSCache::insert(const char* key, time_t expires, const char* name, const ch ver = m_storage_lite->updateText("NameID", name, out.str().c_str(), max(expires, recordexp), ver); if (ver <= 0) { // Out of sync, or went missing, so retry. - return insert(key, expires, name, index); + return insert(key, expires, name, index, attempts + 1); } } else if (!m_storage_lite->createText("NameID", name, out.str().c_str(), expires)) { // Hit a dup, so just retry, hopefully hitting the other branch. - return insert(key, expires, name, index); + return insert(key, expires, name, index, attempts + 1); } } @@ -1293,13 +1322,14 @@ bool SSCache::matches( return false; } -vector::size_type SSCache::logout( +vector::size_type SSCache::_logout( const Application& app, const saml2md::EntityDescriptor* issuer, const saml2::NameID& nameid, const set* indexes, time_t expires, - vector& sessionsKilled + vector& sessionsKilled, + short attempts ) { #ifdef _DEBUG @@ -1308,6 +1338,8 @@ vector::size_type SSCache::logout( if (!m_storage) throw ConfigurationException("SessionCache logout requires a StorageService."); + else if (attempts > 10) + throw IOException("Exceeded retry limit."); auto_ptr_char entityID(issuer ? issuer->getEntityID() : nullptr); auto_ptr_char name(nameid.getName()); @@ -1363,12 +1395,12 @@ vector::size_type SSCache::logout( ver = m_storage_lite->updateText("Logout", name.get(), lout.str().c_str(), max(expires, oldexp), ver); if (ver <= 0) { // Out of sync, or went missing, so retry. - return logout(app, issuer, nameid, indexes, expires, sessionsKilled); + return _logout(app, issuer, nameid, indexes, expires, sessionsKilled, attempts + 1); } } else if (!m_storage_lite->createText("Logout", name.get(), lout.str().c_str(), expires)) { // Hit a dup, so just retry, hopefully hitting the other branch. - return logout(app, issuer, nameid, indexes, expires, sessionsKilled); + return _logout(app, issuer, nameid, indexes, expires, sessionsKilled, attempts + 1); } obj.destroy(); @@ -1996,6 +2028,7 @@ void SSCache::receive(DDF& in, ostream& out) // We may need to write back a new address into the session using a versioned update loop. if (client_addr) { + short attempts = 0; m_log.info("binding session (%s) to new client address (%s)", key, client_addr); do { // We have to reconstitute the session object ourselves. @@ -2043,6 +2076,10 @@ void SSCache::receive(DDF& in, ostream& out) } if (ver < 0) { // Out of sync. + if (++attempts > 10) { + m_log.error("failed to bind client address, update attempts exceeded limit"); + throw IOException("Unable to update stored session, exceeded retry limit."); + } m_log.warn("storage service indicates the record is out of sync, updating with a fresh copy..."); sessionobj["version"].integer(sessionobj["version"].integer() - 1); ver = m_storage->readText(key, "session", &record); -- 2.1.4