Fix invalid signature handling

Previous implementation would throw invalid signature errors
on unrelated cookies, which is undesirable

This changes the interface of SignedCookie a bit which could
affect external callers. Any recommendations on this? We could
also make a new ValidatingSignedCookie class or something that
does this and deprecate the old one.

beaker/docs/modules/session.rst

@@ -1,4 +1,4 @@
-:mod:`beaker.session` -- Session classes 
+:mod:`beaker.session` -- Session classes
 ========================================
 
 .. automodule:: beaker.session
@@ -13,3 +13,4 @@ Module Contents
 .. autoclass:: SessionObject
    :members: persist, get_by_id, accessed
 .. autoclass:: SignedCookie
+.. autodata:: InvalidSignature

beaker/session.py

@@ -9,7 +9,19 @@
 from beaker.exceptions import BeakerException, InvalidCryptoBackendError
 from beaker.cookie import SimpleCookie
 
-__all__ = ['SignedCookie', 'Session']
+__all__ = ['SignedCookie', 'Session', 'InvalidSignature']
+
+
+class _InvalidSignatureType(object):
+    """Returned from SignedCookie when the value's signature was invalid."""
+    def __nonzero__(self):
+        return False
+
+    def __bool__(self):
+        return False
+
+
+InvalidSignature = _InvalidSignatureType()
 
 
 try:
@@ -42,11 +54,10 @@ def _session_id():
             return raw_id.replace('+', '-').replace('/', '_').rstrip('=')
 
 
-class SignedCookie(SimpleCookie):
+class _SignedCookie(SimpleCookie):
     """Extends python cookie to give digital signature support"""
-    def __init__(self, secret, input=None, invalidate_corrupt=True):
+    def __init__(self, secret, input=None):
         self.secret = secret.encode('UTF-8')
-        self.invalidate_corrupt = invalidate_corrupt
         http_cookies.BaseCookie.__init__(self, input)
 
     def value_decode(self, val):
@@ -60,19 +71,13 @@ def value_decode(self, val):
         invalid_bits = 0
         input_sig = val[:40]
         if len(sig) != len(input_sig):
-            if self.invalidate_corrupt:
-                return None, val
-            else:
-                raise BeakerException("Invalid signature")
+            return InvalidSignature, val
 
         for a, b in zip(sig, input_sig):
             invalid_bits += a != b
 
         if invalid_bits:
-            if self.invalidate_corrupt:
-                return None, val
-            else:
-                raise BeakerException("Invalid signature")
+            return InvalidSignature, val
         else:
             return val[40:], val
 
@@ -162,22 +167,24 @@ def __init__(self, request, id=None, invalidate_corrupt=False,
             cookieheader = request.get('cookie', '')
             if secret:
                 try:
-                    self.cookie = SignedCookie(
+                    self.cookie = _SignedCookie(
                         secret,
                         input=cookieheader,
-                        invalidate_corrupt=self.invalidate_corrupt,
                     )
                 except http_cookies.CookieError:
-                    self.cookie = SignedCookie(
+                    self.cookie = _SignedCookie(
                         secret,
                         input=None,
-                        invalidate_corrupt=self.invalidate_corrupt,
                     )
             else:
                 self.cookie = SimpleCookie(input=cookieheader)
 
             if not self.id and self.key in self.cookie:
-                self.id = self.cookie[self.key].value
+                cookie_data = self.cookie[self.key].value
+                # Should we check invalidate_corrupt here?
+                if cookie_data is InvalidSignature:
+                    cookie_data = None
+                self.id = cookie_data
 
         self.is_new = self.id is None
         if self.is_new:
@@ -544,16 +551,14 @@ def __init__(self, request, key='beaker.session.id', timeout=None,
                                   "Session.")
 
         try:
-            self.cookie = SignedCookie(
+            self.cookie = _SignedCookie(
                 validate_key,
                 input=cookieheader,
-                invalidate_corrupt=self.invalidate_corrupt,
             )
         except http_cookies.CookieError:
-            self.cookie = SignedCookie(
+            self.cookie = _SignedCookie(
                 validate_key,
                 input=None,
-                invalidate_corrupt=self.invalidate_corrupt,
             )
 
         self['_id'] = _session_id()
@@ -564,6 +569,8 @@ def __init__(self, request, key='beaker.session.id', timeout=None,
             self.is_new = False
             try:
                 cookie_data = self.cookie[self.key].value
+                if cookie_data is InvalidSignature:
+                    raise BeakerException("Invalid signature")
                 self.update(self._decrypt_data(cookie_data))
                 self._path = self.get('_path', '/')
             except Exception as e:

tests/test_session.py

@@ -379,6 +379,18 @@ def test_invalidate_empty_cookie():
     assert "foo" not in dict(session)
 
 
+@with_setup(setup_cookie_request)
+def test_unrelated_cookie():
+    kwargs = {'validate_key': 'test_key', 'encrypt_key': 'encrypt'}
+    session = get_cookie_session(**kwargs)
+    session['foo'] = 'bar'
+    session.save()
+
+    COOKIE_REQUEST['cookie_out'] = COOKIE_REQUEST['cookie_out'] + '; some.other=cookie'
+    session = get_cookie_session(id=session.id, invalidate_corrupt=False, **kwargs)
+    assert "foo" in dict(session)
+
+
 @with_setup(setup_cookie_request)
 def test_invalidate_invalid_signed_cookie():
     kwargs = {'validate_key': 'test_key', 'encrypt_key': 'encrypt'}