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.
This commit is contained in:
Tim McCormack
2022-02-07 16:00:56 +00:00
committed by GitHub
parent 5708787527
commit a1b09c0b8d
3 changed files with 17 additions and 2 deletions

View File

@@ -10,7 +10,7 @@ Usage:
logger.info(
"Received invalid auth token %s in Authorization header",
encrypt_for_log(token, settings.<YOUR_DEBUG_PUBLIC_KEY>)
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 <sender public key> "|" <ciphertext> 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())

View File

@@ -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']

View File

@@ -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]: