From a1b09c0b8db623701ed7be3bb1a2997a0aea813b Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Mon, 7 Feb 2022 16:00:56 +0000 Subject: [PATCH] fix: More resilience when calling `encrypt_for_log` with missing key (#29878) It's likely that someone will at some point enable encrypted logging but forget to deploy the config change that sets the key; if this happens, we should gracefully return a warning rather than raise an exception. Along the same lines, make sure that safe-sessions won't raise an exception if the setting is missing, and document the suggested use of getattr. --- common/djangoapps/util/log_sensitive.py | 11 ++++++++++- common/djangoapps/util/tests/test_log_sensitive.py | 6 ++++++ openedx/core/djangoapps/safe_sessions/middleware.py | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/util/log_sensitive.py b/common/djangoapps/util/log_sensitive.py index 8874b7f518..6495c81bf2 100644 --- a/common/djangoapps/util/log_sensitive.py +++ b/common/djangoapps/util/log_sensitive.py @@ -10,7 +10,7 @@ Usage: logger.info( "Received invalid auth token %s in Authorization header", - encrypt_for_log(token, settings.) + encrypt_for_log(token, getattr(settings, 'YOUR_DEBUG_PUBLIC_KEY', None)) ) This will log a message like:: @@ -69,7 +69,16 @@ def encrypt_for_log(message, reader_public_key_b64): Returns a string "|" wrapped in some framing text "[encrypted: ...]"; the inner string can be decrypted with decrypt_log_message. + + For ease of use, key may be None or empty; a warning message will be + returned instead of data, encrypted or otherwise. """ + # If no key was configured, don't raise an error. This method is + # expected to be called from debugging code, so it shouldn't blow + # up on misconfiguration. + if not reader_public_key_b64: + return '[encryption failed, no key]' + reader_public_key = PublicKey(b64decode(reader_public_key_b64)) encrypted = Box(logger_private_key, reader_public_key).encrypt(message.encode()) diff --git a/common/djangoapps/util/tests/test_log_sensitive.py b/common/djangoapps/util/tests/test_log_sensitive.py index a13305c09a..7def5b1256 100644 --- a/common/djangoapps/util/tests/test_log_sensitive.py +++ b/common/djangoapps/util/tests/test_log_sensitive.py @@ -3,9 +3,15 @@ Tests for util.logging """ import re + from common.djangoapps.util.log_sensitive import decrypt_log_message, encrypt_for_log, generate_reader_keys +def test_encryption_no_key(): + to_log = encrypt_for_log("Testing testing 1234", None) + assert to_log == '[encryption failed, no key]' + + def test_encryption_round_trip(): reader_keys = generate_reader_keys() reader_public_64 = reader_keys['public'] diff --git a/openedx/core/djangoapps/safe_sessions/middleware.py b/openedx/core/djangoapps/safe_sessions/middleware.py index d79e92efed..01144cf3b0 100644 --- a/openedx/core/djangoapps/safe_sessions/middleware.py +++ b/openedx/core/djangoapps/safe_sessions/middleware.py @@ -765,7 +765,7 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin): """ # NOTE: request.headers seems to pick up initial values, but won't adjust as the request object is edited. # For example, the session cookie will likely be the safe session version. - return encrypt_for_log(str(request.headers), settings.SAFE_SESSIONS_DEBUG_PUBLIC_KEY) + return encrypt_for_log(str(request.headers), getattr(settings, 'SAFE_SESSIONS_DEBUG_PUBLIC_KEY', None)) def obscure_token(value: Union[str, None]) -> Union[str, None]: