Move session ID encryption, decryption & validation to SessionRefCipher.

4 files changed, 84 insertions, 34 deletions

diff --git a/okupy/accounts/forms.py b/okupy/accounts/forms.py
index 3aa9844..a36cf53 100644
--- a/okupy/accounts/forms.py
+++ b/okupy/accounts/forms.py
@@ -1,12 +1,9 @@
 # vim:fileencoding=utf8:et:ts=4:sts=4:sw=4:ft=python
 
 from django import forms
-from django.contrib.sessions.backends.cache import SessionStore
 
 from .models import OpenID_Attributes
-from ..common.crypto import cipher
-
-import base64
+from ..common.crypto import sessionrefcipher
 
 
 class LoginForm(forms.Form):
@@ -31,17 +28,11 @@ class SSLCertLoginForm(forms.Form):
     login_uri = forms.CharField(max_length=254, widget=forms.HiddenInput())
 
     def clean_session(self):
-        enc_session_id = self.cleaned_data['session']
         try:
-            session_id = cipher.decrypt(
-                    base64.b64decode(enc_session_id), 32)
-        except (TypeError, ValueError):
-            pass
-        else:
-            session = SessionStore(session_key=session_id)
-            if session.get('encrypted_id') == enc_session_id:
-                return session
-        raise forms.ValidationError('Invalid session id')
+            return sessionrefcipher.decrypt(
+                self.cleaned_data['session'])
+        except ValueError:
+            raise forms.ValidationError('Invalid session id')
 
 
 class OTPForm(forms.Form):
diff --git a/okupy/accounts/views.py b/okupy/accounts/views.py
index 9266fe8..246b23c 100644
--- a/okupy/accounts/views.py
+++ b/okupy/accounts/views.py
@@ -34,7 +34,7 @@ from .openid_store import DjangoDBOpenIDStore
 from ..common.ldap_helpers import (get_bound_ldapuser,
                                    set_secondary_password,
                                    remove_secondary_password)
-from ..common.crypto import cipher
+from ..common.crypto import sessionrefcipher
 from ..common.decorators import strong_auth_required, anonymous_required
 from ..common.exceptions import OkupyError
 from ..common.log import log_extra_data
@@ -46,7 +46,6 @@ from ..otp.totp.models import TOTPDevice
 # the following two are for exceptions
 import openid.yadis.discover
 import openid.fetchers
-import base64
 import django_otp
 import io
 import ldap
@@ -175,24 +174,13 @@ def login(request):
         ssl_auth_form = None
         ssl_auth_uri = None
     else:
-        if 'encrypted_id' not in request.session:
-            # .cache_key is a very good property since it ensures
-            # that the cache is actually created, and works from first
-            # request
-            session_id = request.session.cache_key
-
-            # since it always starts with the backend module name
-            # and __init__() expects pure id, we can strip that
-            assert(session_id.startswith('django.contrib.sessions.cache'))
-            session_id = session_id[29:]
-            request.session['encrypted_id'] = base64.b64encode(
-                cipher.encrypt(session_id))
+        encrypted_id = sessionrefcipher.encrypt(request.session)
 
         # TODO: it fails when:
         # 1. site is accessed via IP (auth.127.0.0.1),
         # 2. HTTP used on non-standard port (https://...:8000).
         ssl_auth_form = SSLCertLoginForm({
-            'session': request.session['encrypted_id'],
+            'session': encrypted_id,
             'next': request.build_absolute_uri(next),
             'login_uri': request.build_absolute_uri(request.get_full_path()),
         })
diff --git a/okupy/common/crypto.py b/okupy/common/crypto.py
index a50d3b6..ec537a7 100644
--- a/okupy/common/crypto.py
+++ b/okupy/common/crypto.py
@@ -1,12 +1,14 @@
 # vim:fileencoding=utf8:et:ts=4:sts=4:sw=4:ft=python
 
 from django.conf import settings
+from django.contrib.sessions.backends.cache import SessionStore
 
 from Crypto.Cipher.Blowfish import BlowfishCipher
 from Crypto.Hash.SHA384 import SHA384Hash
 
 import Crypto.Random
 
+import base64
 import binascii
 import struct
 
@@ -55,9 +57,6 @@ class OkupyCipher(object):
         return self.cipher.decrypt(data)[:length]
 
 
-cipher = OkupyCipher()
-
-
 class IDCipher(object):
     """
     A cipher to create 'encrypted database IDs'. It is specifically fit
@@ -76,4 +75,55 @@ class IDCipher(object):
         return id
 
 
+class SessionRefCipher(object):
+    """
+    A cipher to provide encrypted identifiers to sessions.
+
+    The encrypted session ID is stored in session for additional
+    security. Only previous encryption result may be used in decrypt().
+    """
+
+    def encrypt(self, session):
+        """
+        Return an encrypted reference to the session. The encrypted
+        identifier will be stored in the session for verification
+        and caching. Therefore, further calls to this method will reuse
+        the previously cached identifier.
+        """
+
+        if 'encrypted_id' not in session:
+            # .cache_key is a very good property since it ensures
+            # that the cache is actually created, and works from first
+            # request
+            session_id = session.cache_key
+
+            # since it always starts with the backend module name
+            # and __init__() expects pure id, we can strip that
+            session_mod = 'django.contrib.sessions.cache'
+            assert(session_id.startswith(session_mod))
+            session_id = session_id[len(session_mod):]
+            session['encrypted_id'] = base64.b64encode(
+                cipher.encrypt(session_id))
+            session.save()
+        return session['encrypted_id']
+
+    def decrypt(self, eid):
+        """
+        Return the SessionStore to which the encrypted identifier is
+        pointing. Raises ValueError if the identifier is invalid.
+        """
+
+        try:
+            session_id = cipher.decrypt(base64.b64decode(eid), 32)
+        except (TypeError, ValueError):
+            pass
+        else:
+            session = SessionStore(session_key=session_id)
+            if session.get('encrypted_id') == eid:
+                return session
+        raise ValueError('Invalid session id')
+
+
+cipher = OkupyCipher()
 idcipher = IDCipher()
+sessionrefcipher = SessionRefCipher()
diff --git a/okupy/tests/unit/test_cipher.py b/okupy/tests/unit/test_cipher.py
index 8a8f043..bb1be60 100644
--- a/okupy/tests/unit/test_cipher.py
+++ b/okupy/tests/unit/test_cipher.py
@@ -1,9 +1,11 @@
 # vim:fileencoding=utf8:et:ts=4:sts=4:sw=4:ft=python
 
 from Crypto import Random
-from unittest import TestCase
+from unittest import TestCase, SkipTest
 
-from ...common.crypto import cipher
+from django.contrib.sessions.backends.cache import SessionStore
+
+from ...common.crypto import cipher, sessionrefcipher
 
 
 class OkupyCipherTests(TestCase):
@@ -44,3 +46,22 @@ class OkupyCipherTests(TestCase):
         data = self._random_string[:cipher.block_size/2]
         hash = cipher.encrypt(data)[:cipher.block_size/2]
         self.assertRaises(ValueError, cipher.decrypt, hash, len(data))
+
+
+class SessionRefCipherTest(TestCase):
+    def test_encrypt_decrypt(self):
+        session = SessionStore()
+        session['test'] = 'in-test'
+        session.save()
+
+        eid = sessionrefcipher.encrypt(session)
+        sess = sessionrefcipher.decrypt(eid)
+        self.assertEqual(sess.get('test'), 'in-test')
+
+    def test_invalid_base64_raises_valueerror(self):
+        data = 'Azcd^%'
+        self.assertRaises(ValueError, sessionrefcipher.decrypt, data)
+
+    def test_invalid_ciphertext_raises_valueerror(self):
+        data = 'ZHVwYQo='
+        self.assertRaises(ValueError, sessionrefcipher.decrypt, data)