Refactor SessionDB constructor and use secure defaults

CHANGELOG.md

@@ -13,11 +13,11 @@ The format is based on the [KeepAChangeLog] project.
 - [#325]: `oic.oic.claims_match` implementation refactored.
 - [#368]: `oic.oauth2.Client.construct_AccessTokenRequest()` as well as `oic.oic.Client` are now able to perform proper Resource Owner Password Credentials Grant
 - [#374]: Made the to_jwe/from_jwe methods of Message accept list of keys value of parameter keys.
+- [#387]: Refactored the `oic.utils.sdb.SessionDB` constructor API.
 - [#380]: Made cookie_path and cookie_domain configurable via Provider like the cookie_name.
 - [#386]: An exception will now be thrown if a sub claim received from the userinfo endpoint is not the same as a sub claim previously received in an ID Token.
 - [#392]: Made sid creation simpler and faster
 
-
 ### Fixed
 - [#313]: Catch exception correctly
 - [#319]: Fix sanitize on strings starting with "B" or "U"
@@ -30,9 +30,10 @@ The format is based on the [KeepAChangeLog] project.
 
 ### Security
 - [#349]: Changed crypto algorithm used by `oic.utils.sdb.Crypt` for token encryption to Fernet. Old stored tokens are incompatible.
-- [#363]: Fixed IV reuse for CookieDealer class. Replaced the encrypt-then-mac construction with a proper AEAD (AES-SIV). 
+- [#363]: Fixed IV reuse for CookieDealer class. Replaced the encrypt-then-mac construction with a proper AEAD (AES-SIV).
 
 [#313]: https://github.com/OpenIDC/pyoidc/issues/313
+[#387]: https://github.com/OpenIDC/pyoidc/pull/387
 [#318]: https://github.com/OpenIDC/pyoidc/pull/318
 [#319]: https://github.com/OpenIDC/pyoidc/pull/319
 [#324]: https://github.com/OpenIDC/pyoidc/pull/324

oauth_example/as/as.py

@@ -198,7 +198,8 @@ def application(self, environ, start_response):
 
     # This is where session information is stored
     # This serve is stateful.
-    from oic.utils.sdb import SessionDB
+    from oic import rndstr
+    from oic.utils.sdb import SessionDB, DefaultToken
 
     # Parse the command arguments
     parser = argparse.ArgumentParser()
@@ -277,11 +278,15 @@ def application(self, environ, start_response):
         oas.jwks_uri = "{}/{}".format(oas.baseurl, jwks_file_name)
 
     # Initiate the SessionDB
+    _code = DefaultToken(rndstr(32), rndstr(32), typ='A', lifetime=600)
     _token = JWTToken('T', oas.keyjar, {'code': 3600, 'token': 900},
                       iss=config.issuer, sign_alg='RS256')
     _refresh_token = JWTToken('R', oas.keyjar, {'': 86400}, iss=config.issuer,
                               sign_alg='RS256')
-    oas.sdb = SessionDB(config.SERVICE_URL, token_factory=_token,
+    oas.sdb = SessionDB(config.SERVICE_URL,
+                        db={},
+                        code_factory=_code,
+                        token_factory=_token,
                         refresh_token_factory=_refresh_token)
 
     # set some parameters
@@ -326,7 +331,7 @@ def application(self, environ, start_response):
             oas.keyjar.add_kb(iss, kb)
 
     _app = Application(oas)
-    
+
     # Initiate the web server
     SRV = wsgiserver.CherryPyWSGIServer(('0.0.0.0', args.port),
                                         _app.application)

oidc_example/op1/claims_provider.py

@@ -226,7 +226,7 @@ def application(environ, start_response):
     from cherrypy.wsgiserver import ssl_builtin
 
     from oic.oic.claims_provider import ClaimsServer
-    from oic.utils.sdb import SessionDB
+    from oic.utils.sdb import create_session_db
 
     parser = argparse.ArgumentParser()
     parser.add_argument('-v', dest='verbose', action='store_true')
@@ -241,7 +241,10 @@ def application(environ, start_response):
     # in memory session storage
 
     config = json.loads(open(args.config).read())
-    OAS = ClaimsServer(config["issuer"], SessionDB(), cdb, userinfo,
+    sdb = create_session_db(config["issuer"],
+                            config["SESSION_KEY"],
+                            password="changethis")
+    OAS = ClaimsServer(config["issuer"], sdb, cdb, userinfo,
                        verify_client)
 
     if "keys" in config:

oidc_example/op1/oc_config.py.example

@@ -22,6 +22,10 @@ AUTHORIZATION = {
 COOKIENAME= 'pyoic'
 COOKIETTL = 4*60 # 4 hours
 SYM_KEY = "SoLittleTime,Got"
+
+# Encryption Key and HMAC key to 
+SESSION_KEY = "a"*64
+
 SERVER_CERT = "certs/server.crt"
 SERVER_KEY = "certs/server.key"
 #CERT_CHAIN="certs/chain.pem"

oidc_example/op1/oc_server.py

@@ -481,7 +481,8 @@ def new_trace_log(self, key):
     #from cherrypy.wsgiserver import ssl_builtin
     from cherrypy.wsgiserver import ssl_pyopenssl
 
-    from oic.utils.sdb import SessionDB
+    from oic import rndstr
+    from oic.utils.sdb import create_session_db
 
     parser = argparse.ArgumentParser()
     parser.add_argument('-v', dest='verbose', action='store_true')
@@ -532,18 +533,23 @@ def new_trace_log(self, key):
     else:
         kwargs = {"verify_ssl": True}
 
+    # In-Memory SessionDB issuing DefaultTokens
+    sdb = create_session_db(config.baseurl,
+                            secret=rndstr(32),
+                            password=rndstr(32))
+
     if args.test:
         URLS.append((r'tracelog', trace_log))
-        OAS = TestProvider(config.issuer, SessionDB(config.baseurl), cdb, ac,
+        OAS = TestProvider(config.issuer, sdb, cdb, ac,
                            None, authz, config.SYM_KEY)
     elif args.XpressConnect:
         from XpressConnect import XpressConnectProvider
 
-        OAS = XpressConnectProvider(config.issuer, SessionDB(config.baseurl),
+        OAS = XpressConnectProvider(config.issuer, sdb,
                                     cdb, ac, None, authz, verify_client,
                                     config.SYM_KEY)
     else:
-        OAS = Provider(config.issuer, SessionDB(config.baseurl), cdb, ac, None,
+        OAS = Provider(config.issuer, sdb, cdb, ac, None,
                        authz, verify_client, config.SYM_KEY, **kwargs)
 
 

oidc_example/op2/server.py

@@ -233,7 +233,7 @@ def token(self, environ, start_response):
 
     # noinspection PyUnusedLocal
     def authorization(self, environ, start_response):
-        return wsgi_wrapper(environ, start_response, 
+        return wsgi_wrapper(environ, start_response,
                             self.oas.authorization_endpoint, logger=logger)
 
     # noinspection PyUnusedLocal
@@ -367,7 +367,8 @@ def application(self, environ, start_response):
     from cherrypy import wsgiserver
     from cherrypy.wsgiserver.ssl_builtin import BuiltinSSLAdapter
 
-    from oic.utils.sdb import SessionDB
+    from oic import rndstr
+    from oic.utils.sdb import create_session_db
 
     parser = argparse.ArgumentParser()
     parser.add_argument('-v', dest='verbose', action='store_true')
@@ -545,7 +546,12 @@ def application(self, environ, start_response):
     else:
         pass
 
-    OAS = Provider(_issuer, SessionDB(_issuer), cdb, ac, None,
+    # In-Memory non persistent SessionDB
+    sdb = create_session_db(_issuer,
+                            secret=rndstr(32),
+                            password=rndstr(32))
+
+    OAS = Provider(_issuer, sdb, cdb, ac, None,
                    authz, verify_client, config.SYM_KEY, **kwargs)
     OAS.baseurl = _issuer
 

oidc_example/op3/server.py

@@ -12,6 +12,8 @@
 import importlib
 from mako.lookup import TemplateLookup
 
+from oic import rndstr
+
 from oic.oic.provider import AuthorizationEndpoint
 from oic.oic.provider import EndSessionEndpoint
 from oic.oic.provider import Provider
@@ -35,7 +37,7 @@
 from cherrypy import wsgiserver
 from cherrypy.wsgiserver.ssl_builtin import BuiltinSSLAdapter
 
-from oic.utils.sdb import SessionDB
+from oic.utils.sdb import create_session_db
 
 
 
@@ -368,9 +370,14 @@ def application(self, environ, start_response):
     authz = AuthzHandling()
     clientDB = shelve_wrapper.open(config.CLIENTDB)
 
+    # In-Memory non-persistent SessionDB issuing DefaultTokens
+    sessionDB = create_session_db(config.ISSUER,
+                                  secret=rndstr(32),
+                                  password=rndstr(32))
+
     provider = Provider(
         name=config.ISSUER,                            # name
-        sdb=SessionDB(config.ISSUER),                  # session database.
+        sdb=sessionDB,                                 # session database.
         cdb=clientDB,                                  # client database
         authn_broker=authnBroker,                      # authn broker
         userinfo=None,                                 # user information

oidc_example/simple_op/src/provider/server/server.py

@@ -33,7 +33,7 @@
 from oic.utils.http_util import get_or_post
 from oic.utils.http_util import get_post
 from oic.utils.keyio import keyjar_init
-from oic.utils.sdb import SessionDB
+from oic.utils.sdb import create_session_db
 from oic.utils.userinfo import UserInfo
 from oic.utils.webfinger import OIC_ISSUER
 from oic.utils.webfinger import WebFinger
@@ -199,7 +199,9 @@ def main():
     userinfo = UserInfo(i)
 
     client_db = {}
-    provider = Provider(issuer, SessionDB(issuer), client_db, authn_broker,
+    session_db = create_session_db(issuer,
+                                   secret=rndstr(32), password=rndstr(32))
+    provider = Provider(issuer, session_db, client_db, authn_broker,
                         userinfo, AuthzHandling(), verify_client, None)
     provider.baseurl = issuer
     provider.symkey = rndstr(16)

src/oic/utils/sdb.py

@@ -12,6 +12,7 @@
 from cryptography.fernet import Fernet
 
 from oic import rndstr
+from oic.exception import ImproperlyConfigured
 from oic.oic import AuthorizationRequest
 from oic.utils.time_util import time_sans_frac
 from oic.utils.time_util import utc_time_sans_frac
@@ -339,33 +340,68 @@ def remove(self, token):
         self._db.pop(token)
 
 
+def create_session_db(base_url, secret, password, db=None,
+                      token_expires_in=3600, grant_expires_in=600,
+                      refresh_token_expires_in=86400):
+    """
+    Convenience wrapper for SessionDB construction
+
+    Using this you can create a very basic non persistant
+    session database that issues opaque DefaultTokens.
+
+    :param base_url: Same as base_url parameter of `SessionDB`.
+    :param secret: Secret to pass to `DefaultToken` class.
+    :param password: Secret key to pass to `DefaultToken` class.
+    :param db: Storage for the session data, usually a dict.
+    :param token_expires_in: Expiry time for access tokens in seconds.
+    :param grant_expires_in: Expiry time for access codes in seconds.
+    :param refresh_token_expires_in: Expiry time for refresh tokens.
+
+    :return: A constructed `SessionDB` object.
+    """
+
+    code_factory = DefaultToken(secret, password, typ='A',
+                                lifetime=grant_expires_in)
+    token_factory = DefaultToken(secret, password, typ='T',
+                                 lifetime=token_expires_in)
+    db = {} if db is None else db
+
+    return SessionDB(
+        base_url, db,
+        refresh_db=None,
+        code_factory=code_factory,
+        token_factory=token_factory,
+        refresh_token_expires_in=refresh_token_expires_in,
+        refresh_token_factory=None,
+    )
+
+
 class SessionDB(object):
-    def __init__(self, base_url, db=None, secret="Ab01FG65",
-                 token_expires_in=3600, password="4-amino-1H-pyrimidine-2-one",
-                 grant_expires_in=600, seed="", refresh_db=None,
+    def __init__(self, base_url, db, refresh_db=None,
                  refresh_token_expires_in=86400,
                  token_factory=None, code_factory=None,
                  refresh_token_factory=None):
+
         self.base_url = base_url
-        if db is None:
-            db = {}
         self._db = db
 
-        self.token_factory = {}
+        # TODO: uid2sid should have a persistence option too.
+        self.uid2sid = {}
+
+        self.token_factory = {
+            'code': code_factory,
+            'access_token': token_factory,
+        }
 
         self.token_factory_order = ['code', 'access_token']
-        if code_factory:
-            self.token_factory['code'] = code_factory
-        else:
-            self.token_factory['code'] = DefaultToken(secret, password, typ='A',
-                                                      lifetime=grant_expires_in)
-        if token_factory:
-            self.token_factory['access_token'] = token_factory
-        else:
-            self.token_factory['access_token'] = DefaultToken(
-                secret, password, typ='T', lifetime=token_expires_in)
+
+        # TODO: This should simply be a factory like all the others too,
+        #       even for the default case.
 
         if refresh_token_factory:
+            if refresh_db:
+                raise ImproperlyConfigured(
+                    "Only use one of refresh_db or refresh_token_factory")
             self._refresh_db = None
             self.token_factory['refresh_token'] = refresh_token_factory
             self.token_factory_order.append('refresh_token')
@@ -376,8 +412,6 @@ def __init__(self, base_url, db=None, secret="Ab01FG65",
 
         self.access_token = self.token_factory['access_token']
         self.token = self.access_token
-        self.uid2sid = {}
-        self.seed = seed or secret
 
     def _get_token_key(self, item, order=None):
         if order is None:

tests/conftest.py

@@ -5,14 +5,44 @@
 from oic.oic.provider import Provider
 from oic.utils.authn.client import verify_client
 from oic.utils.authz import AuthzHandling
-from oic.utils.sdb import SessionDB
+from oic.utils.sdb import create_session_db
 
 
 @pytest.fixture
-def provider():
-    issuer = "https://op.foo.com"
+def session_db_factory():
+    def fac(issuer):
+        return create_session_db(issuer,
+                                 secret='supersecret', password='badpassword')
+    return fac
+
+
+@pytest.fixture
+def session_db(session_db_factory):
+    return session_db_factory("https://op.example.com")
+
+
+@pytest.fixture
+def fake_oic_server(session_db_factory):
+    from tests.fakeoicsrv import MyFakeOICServer
+
+    def fac(name):
+        return MyFakeOICServer(name, session_db_factory=session_db_factory)
+    return fac
+
+
+@pytest.fixture
+def mitm_server(session_db_factory):
+    from tests.mitmsrv import MITMServer
+
+    def fac(name):
+        return MITMServer(name, session_db_factory=session_db_factory)
+    return fac
+
+
+@pytest.fixture
+def provider(session_db):
+    issuer = "https://op.example.com"
     client_db = {}
-    session_db = SessionDB(issuer),
     verification_function = verify_client
     authz_handler = AuthzHandling()
     symkey = None

tests/fakeoicsrv.py

@@ -16,7 +16,6 @@
 from oic.oic.message import RegistrationResponse
 from oic.oic.message import TokenErrorResponse
 from oic.utils.sdb import AuthnEvent
-from oic.utils.sdb import SessionDB
 from oic.utils.time_util import utc_time_sans_frac
 from oic.utils.webfinger import WebFinger
 
@@ -51,9 +50,9 @@ def __getitem__(self, item):
 
 
 class MyFakeOICServer(Server):
-    def __init__(self, name=""):
+    def __init__(self, name="", session_db_factory=None):
         Server.__init__(self)
-        self.sdb = SessionDB(name)
+        self.sdb = session_db_factory(name)
         self.name = name
         self.client = {}
         self.registration_expires_in = 3600