diff --git a/openedx/core/djangoapps/safe_sessions/middleware.py b/openedx/core/djangoapps/safe_sessions/middleware.py index 2541780a6e..1f8e07166c 100644 --- a/openedx/core/djangoapps/safe_sessions/middleware.py +++ b/openedx/core/djangoapps/safe_sessions/middleware.py @@ -3,22 +3,29 @@ This module defines SafeSessionMiddleware that makes use of a SafeCookieData that cryptographically binds the user to the session id in the cookie. -The implementation is inspired by the proposal in the following paper: -http://www.cse.msu.edu/~alexliu/publications/Cookie/cookie.pdf +The implementation is inspired in part by the proposal in the paper + +but deviates in a number of ways; mostly it just uses the technique +of an intermediate key for HMAC. Note: The proposed protocol protects against replay attacks by +use of channel binding—specifically, by incorporating the session key used in the SSL connection. However, this does not suit our needs since we want the ability to reuse the -same cookie over multiple SSL connections. So instead, we mitigate +same cookie over multiple browser sessions, and in any case the server +will be behind a load balancer and won't have access to the correct +SSL session information. So instead, we mitigate replay attacks by enforcing session cookie expiration (via TimestampSigner) and assuming SESSION_COOKIE_SECURE (see below). We use django's built-in Signer class, which makes use of a built-in -salted_hmac function that derives a usage-specific key from the +salted_hmac function that derives an intermediate key from the server's SECRET_KEY, as proposed in the paper. -Note: The paper proposes deriving a usage-specific key from the +Note: The paper proposes deriving an intermediate key from the session's expiration time in order to protect against volume attacks. +(Note that these hypothetical attacks would only succeed if HMAC-SHA1 +were found to be weak, and there is presently no indication of this.) However, since django does not always use an expiration time, we instead use a random key salt to prevent volume attacks. @@ -30,28 +37,25 @@ of the session cookies. Django instead relies on the browser to honor session cookie expiration. The resulting safe cookie data that gets stored as the value in the -session cookie is a tuple of: - ( - version, - session_id, - key_salt, - signature - ) +session cookie is: - where signature is: - signed_data : base64(HMAC_SHA1(signed_data, usage_key)) + version '|' session_id '|' key_salt '|' signed_hash - where signed_data is: - H(version | session_id | user_id) : timestamp +where signed_hash is a structure incorporating the following value and +a MAC (via TimestampSigner): - where usage_key is: - SHA1(key_salt + 'signer' + settings.SECRET_KEY) + SHA256(version '|' session_id '|' user_id '|') + +TimestampSigner uses HMAC-SHA1 to derive a key from key_salt and the +server's SECRET_KEY; see django.core.signing for more details on the +structure of the output (which takes the form of colon-delimited +Base64.) Note: We assume that the SESSION_COOKIE_SECURE setting is set to TRUE to prevent inadvertent leakage of the session cookie to a person-in-the-middle. The SESSION_COOKIE_SECURE flag indicates to the browser that the cookie should be sent only over an -SSL-protected channel. Otherwise, a session hijacker could copy +SSL-protected channel. Otherwise, a connection eavesdropper could copy the entire cookie and use it to impersonate the victim. Custom Attributes: @@ -121,7 +125,7 @@ class SafeCookieData: session_id (string): Unique and unguessable session identifier to which this safe cookie data is bound. key_salt (string): A securely generated random string that - is used to derive a usage-specific secret key for + is used to derive an intermediate secret key for signing the safe cookie data to protect against volume attacks. signature (string): Cryptographically created signature @@ -185,9 +189,10 @@ class SafeCookieData: def sign(self, user_id): """ - Computes the signature of this safe cookie data. - A signed value of hash(version | session_id | user_id):timestamp - with a usage-specific key derived from key_salt. + Signs the safe cookie data for this user using a timestamped signature + and an intermediate key derived from key_salt and server's SECRET_KEY. + Value under signature is the hexadecimal string from + SHA256(version '|' session_id '|' user_id '|'). """ data_to_sign = self._compute_digest(user_id) self.signature = signing.dumps(data_to_sign, salt=self.key_salt) @@ -214,7 +219,7 @@ class SafeCookieData: def _compute_digest(self, user_id): """ - Returns hash(version | session_id | user_id |) + Returns SHA256(version '|' session_id '|' user_id '|') hex string. """ hash_func = sha256() for data_item in [self.version, self.session_id, user_id]: diff --git a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py index a34fdc71a3..b8a23c567d 100644 --- a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py +++ b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py @@ -201,3 +201,38 @@ class TestSafeCookieData(TestSafeSessionsLogMixin, TestCase): digest = self.safe_cookie_data._compute_digest(self.user_id) setattr(self.safe_cookie_data, field_name, incorrect_field_value) assert digest != self.safe_cookie_data._compute_digest(self.user_id) + + #---- Test roundtrip with pinned values ----# + + def test_pinned_values(self): + """ + Compute a cookie with all inputs held constant and assert that the + exact output never changes. This protects against unintentional + changes to the algorithm. + """ + user_id = '8523' + session_id = 'SSdtIGEgc2Vzc2lvbiE' + a_random_string = 'HvGnjXf1b3jU' + timestamp = 1626895850 + + module = 'openedx.core.djangoapps.safe_sessions.middleware' + with patch(f"{module}.signing.time.time", return_value=timestamp): + with patch(f"{module}.get_random_string", return_value=a_random_string): + safe_cookie_data = SafeCookieData.create(session_id, user_id) + serialized_value = str(safe_cookie_data) + + # **IMPORTANT**: If a change to the algorithm causes this test + # to start failing, you will either need to allow both the old + # and new format or all users will become logged out upon + # deploy of the changes. + # + # Also assumes SECRET_KEY is '85920908f28904ed733fe576320db18cabd7b6cd' + # (set in lms or cms.envs.test) + assert serialized_value == ( + "1" + "|SSdtIGEgc2Vzc2lvbiE" + "|HvGnjXf1b3jU" + "|ImExZWZiNzVlZGFmM2FkZWZmYjM4YjI0ZmZkOWU4MzExODU0MTk4NmVlNGRiYzBlODdhYWUzOGM5MzVlNzk4NjUi" + ":1m6Hve" + ":OMhY2FL2pudJjSSXChtI-zR8QVA" + )