From 1e985d549481a2ca0e03440e410912b4e2b49271 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Sep 04 2015 17:53:30 +0000 Subject: [PATCH 1/5] Implement automatic database cleanup Signed-off-by: Patrick Uiterwijk Reviewed-by: Rob Crittenden --- diff --git a/ipsilon/install/ipsilon-server-install b/ipsilon/install/ipsilon-server-install index 0625125..bf086f9 100755 --- a/ipsilon/install/ipsilon-server-install +++ b/ipsilon/install/ipsilon-server-install @@ -97,6 +97,7 @@ def install(plugins, args): 'publicdatadir': args['public_data_dir'], 'wellknowndir': args['wellknown_dir'], 'sysuser': args['system_user'], + 'cleanup_interval': args['cleanup_interval'], 'ipsilondir': BINDIR, 'staticdir': STATICDIR, 'cachedir': CACHEDIR, @@ -376,7 +377,11 @@ def parse_args(plugins): parser.add_argument('--transaction-dburi', help='Transaction database URI (override template)') parser.add_argument('--samlsessions-dburi', - help='SAML 2 sessions database URI (override template)') + help='SAML 2 sessions database URI (override ' + + 'template)') + parser.add_argument('--cleanup-interval', default=30, + help='Interval between cleaning up stale database ' + + 'entries (in minutes, default: 30 minutes)') lms = [] diff --git a/ipsilon/providers/saml2idp.py b/ipsilon/providers/saml2idp.py index 2b61fdf..d4f3a31 100644 --- a/ipsilon/providers/saml2idp.py +++ b/ipsilon/providers/saml2idp.py @@ -354,12 +354,6 @@ Provides SAML 2.0 authentication infrastructure. """ self.sessionfactory = SAMLSessionFactory( database_url=self.get_config_value('session database url') ) - # Schedule cleanups - # pylint: disable=protected-access - bt = cherrypy.process.plugins.BackgroundTask( - 60, self.sessionfactory._ss.remove_expired_sessions - ) - bt.start() # Init IDP data try: idp = IdentityProvider(self, diff --git a/ipsilon/util/data.py b/ipsilon/util/data.py index 8d2a1d5..3f4ccff 100644 --- a/ipsilon/util/data.py +++ b/ipsilon/util/data.py @@ -13,6 +13,7 @@ import ConfigParser import os import uuid import logging +import time CURRENT_SCHEMA_VERSION = 2 @@ -297,7 +298,13 @@ class FileQuery(Log): class Store(Log): + # Static, Store-level variables _is_upgrade = False + __cleanups = {} + + # Static, class-level variables + # Either set this to False, or implement _cleanup, in child classes + _should_cleanup = True def __init__(self, config_name=None, database_url=None): if config_name is None and database_url is None: @@ -318,6 +325,60 @@ class Store(Log): if not self._is_upgrade: self._check_database() + if self._should_cleanup: + self._schedule_cleanup() + + def _schedule_cleanup(self): + store_name = self.__class__.__name__ + if self.is_readonly: + # No use in cleanups on a readonly database + self.debug('Not scheduling cleanup for %s due to readonly' % + store_name) + return + if store_name in Store.__cleanups: + # This class was already scheduled, skip + return + self.debug('Scheduling cleanups for %s' % store_name) + # Check once every minute whether we need to clean + task = cherrypy.process.plugins.BackgroundTask( + 60, self._maybe_run_cleanup) + task.start() + Store.__cleanups[store_name] = task + + def _maybe_run_cleanup(self): + # Let's see if we need to do cleanup + last_clean = self.load_options('dbinfo').get('%s_last_clean' % + self.__class__.__name__, + {}) + time_diff = cherrypy.config.get('cleanup_interval', 30) * 60 + next_ts = int(time.time()) - time_diff + self.debug('Considering cleanup for %s: %s. Next at: %s' + % (self.__class__.__name__, last_clean, next_ts)) + if ('timestamp' not in last_clean or + int(last_clean['timestamp']) <= next_ts): + # First store the current time so that other servers don't start + self.save_options('dbinfo', '%s_last_clean' + % self.__class__.__name__, + {'timestamp': int(time.time()), + 'removed_entries': -1}) + + # Cleanup has been long enough ago, let's run + self.debug('Cleaning up for %s' % self.__class__.__name__) + removed_entries = self._cleanup() + self.debug('Cleaned up %i entries for %s' % + (removed_entries, self.__class__.__name__)) + self.save_options('dbinfo', '%s_last_clean' + % self.__class__.__name__, + {'timestamp': int(time.time()), + 'removed_entries': removed_entries}) + + def _cleanup(self): + # The default cleanup is to do nothing + # This function should return the number of rows it cleaned up. + # This information may be used to automatically tune the clean period. + self.error('Cleanup for %s not implemented' % + self.__class__.__name__) + return 0 def _code_schema_version(self): # This function makes it possible for separate plugins to have @@ -580,6 +641,7 @@ class Store(Log): class AdminStore(Store): + _should_cleanup = False def __init__(self): super(AdminStore, self).__init__('admin.config.db') @@ -638,6 +700,7 @@ class AdminStore(Store): class UserStore(Store): + _should_cleanup = False def __init__(self, path=None): super(UserStore, self).__init__('user.prefs.db') @@ -723,7 +786,7 @@ class SAML2SessionStore(Store): raise ValueError("Multiple entries returned") return data.keys()[0] - def remove_expired_sessions(self): + def _cleanup(self): # pylint: disable=protected-access table = SqlQuery(self._db, self.table, UNIQUE_DATA_TABLE)._table sel = select([table.columns.uuid]). \ @@ -731,7 +794,7 @@ class SAML2SessionStore(Store): table.c.value <= datetime.datetime.now())) # pylint: disable=no-value-for-parameter d = table.delete().where(table.c.uuid.in_(sel)) - d.execute() + return d.execute().rowcount def get_data(self, idval=None, name=None, value=None): return self.get_unique_data(self.table, idval, name, value) diff --git a/quickrun.py b/quickrun.py index c3d7c73..4bd5802 100755 --- a/quickrun.py +++ b/quickrun.py @@ -68,6 +68,7 @@ def config(workdir): 'sessopt': 'path', 'sessval': os.path.join(workdir, 'sessions'), 'secure': 'False', + 'cleanup_interval': 1, }) conf = os.path.join(workdir, 'ipsilon.conf') with open(conf, 'w+') as f: diff --git a/templates/install/ipsilon.conf b/templates/install/ipsilon.conf index 57ee2ea..f68c12a 100644 --- a/templates/install/ipsilon.conf +++ b/templates/install/ipsilon.conf @@ -3,6 +3,7 @@ debug = ${debugging} tools.log_request_response.on = False template_dir = "templates" cache_dir = "${cachedir}" +cleanup_interval = ${cleanup_interval} db.conn.log = False log.screen = ${debugging} From 2b17119bb97eba45030d18f590624c2b2a9f257e Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Sep 04 2015 17:53:34 +0000 Subject: [PATCH 2/5] Implement cleeanup for TranStore Signed-off-by: Patrick Uiterwijk Reviewed-by: Rob Crittenden --- diff --git a/ipsilon/util/data.py b/ipsilon/util/data.py index 3f4ccff..52fde62 100644 --- a/ipsilon/util/data.py +++ b/ipsilon/util/data.py @@ -740,9 +740,10 @@ class TranStore(Store): def __init__(self, path=None): super(TranStore, self).__init__('transactions.db') + self.table = 'transactions' def _initialize_schema(self): - q = self._query(self._db, 'transactions', UNIQUE_DATA_TABLE, + q = self._query(self._db, self.table, UNIQUE_DATA_TABLE, trans=False) q.create() q._con.close() # pylint: disable=protected-access @@ -751,7 +752,7 @@ class TranStore(Store): if old_version == 1: # In schema version 2, we added indexes and primary keys # pylint: disable=protected-access - table = self._query(self._db, 'transactions', UNIQUE_DATA_TABLE, + table = self._query(self._db, self.table, UNIQUE_DATA_TABLE, trans=False)._table self._db.add_constraint(table.primary_key) for index in table.indexes: @@ -760,6 +761,17 @@ class TranStore(Store): else: raise NotImplementedError() + def _cleanup(self): + # pylint: disable=protected-access + table = SqlQuery(self._db, self.table, UNIQUE_DATA_TABLE)._table + in_one_hour = datetime.datetime.now() - datetime.timedelta(hours=1) + sel = select([table.columns.uuid]). \ + where(and_(table.c.name == 'origintime', + table.c.value <= in_one_hour)) + # pylint: disable=no-value-for-parameter + d = table.delete().where(table.c.uuid.in_(sel)) + return d.execute().rowcount + class SAML2SessionStore(Store): From 24fa1f2acd9cb84342064ec59b311968353fd0ae Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Sep 04 2015 17:53:36 +0000 Subject: [PATCH 3/5] Implement cleanup for sessions Signed-off-by: Patrick Uiterwijk Reviewed-by: Rob Crittenden --- diff --git a/ipsilon/util/sessions.py b/ipsilon/util/sessions.py index 86113a1..5a9854b 100644 --- a/ipsilon/util/sessions.py +++ b/ipsilon/util/sessions.py @@ -4,6 +4,7 @@ import base64 from cherrypy.lib.sessions import Session from ipsilon.util.data import Store, SqlQuery import threading +import datetime try: import cPickle as pickle except ImportError: @@ -36,6 +37,14 @@ class SessionStore(Store): else: raise NotImplementedError() + def _cleanup(self): + # pylint: disable=protected-access + table = SqlQuery(self._db, 'sessions', SESSION_TABLE)._table + # pylint: disable=no-value-for-parameter + d = table.delete().where(table.c.expiration_time + <= datetime.datetime.now()) + return d.execute().rowcount + class SqlSession(Session): From 3ffc77559c269b85385350166f5392cab654c429 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Sep 04 2015 17:53:38 +0000 Subject: [PATCH 4/5] Make it possible to enable database query echoing Signed-off-by: Patrick Uiterwijk Reviewed-by: Rob Crittenden --- diff --git a/ipsilon/util/data.py b/ipsilon/util/data.py index 52fde62..0981c52 100644 --- a/ipsilon/util/data.py +++ b/ipsilon/util/data.py @@ -69,7 +69,10 @@ class SqlStore(BaseStore): # It's not possible to share connections for SQLite between # threads, so let's use the SingletonThreadPool for them pool_args = {'poolclass': SingletonThreadPool} - self._dbengine = create_engine(engine_name, **pool_args) + self._dbengine = create_engine(engine_name, + echo=cherrypy.config.get('db.echo', + False), + **pool_args) self.is_readonly = False def add_constraint(self, constraint): diff --git a/templates/install/ipsilon.conf b/templates/install/ipsilon.conf index f68c12a..f69ac9d 100644 --- a/templates/install/ipsilon.conf +++ b/templates/install/ipsilon.conf @@ -5,6 +5,7 @@ template_dir = "templates" cache_dir = "${cachedir}" cleanup_interval = ${cleanup_interval} db.conn.log = False +db.echo = False log.screen = ${debugging} base.mount = "/${instance}" From 11bbbe3ac6a0842599ab2e5110427758ebaa5573 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Sep 04 2015 17:53:41 +0000 Subject: [PATCH 5/5] Implement cleanup for OpenIDStore Signed-off-by: Patrick Uiterwijk Reviewed-by: Rob Crittenden --- diff --git a/ipsilon/providers/openid/store.py b/ipsilon/providers/openid/store.py index 0eaee0a..3a45f19 100644 --- a/ipsilon/providers/openid/store.py +++ b/ipsilon/providers/openid/store.py @@ -65,18 +65,29 @@ class OpenIDStore(Store, OpenIDStoreInterface): return True + def _cleanup(self): + res1 = self.cleanupNonces() + res2 = self.cleanupAssociations() + return res1 + res2 + def cleanupNonces(self): nonces = self.get_unique_data('nonce') + cleaned = 0 for iden in nonces: if nonces[iden]['timestamp'] < (time() - NonceSKEW): + cleaned += 1 self.del_unique_data('nonce', iden) + return cleaned def cleanupAssociations(self): assocs = self.get_unique_data('association') + cleaned = 0 for iden in assocs: if ((int(assocs[iden]['issued']) + int(assocs[iden]['lifetime'])) < time()): + cleaned += 1 self.del_unique_data('association', iden) + return cleaned def _initialize_schema(self): q = self._query(self._db, 'association', UNIQUE_DATA_TABLE,