From 80f60ffb3699280501b722ead61142b9ef42c8e6 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Fri, 6 May 2022 08:42:19 -0400 Subject: [PATCH] refactor: Move log_sensitive to edx-django-utils (#30349) It was copied there in 4.7.0 (openedx/edx-django-utils#209) so it can be used in more IDAs. Includes dropping dependency on PyNacl, which was only in use by that module. --- common/djangoapps/util/log_sensitive.py | 208 ------------------ .../util/tests/test_log_sensitive.py | 34 --- .../djangoapps/safe_sessions/middleware.py | 2 +- requirements/edx/base.in | 3 +- requirements/edx/base.txt | 5 +- requirements/edx/development.txt | 7 +- requirements/edx/testing.txt | 8 +- 7 files changed, 15 insertions(+), 252 deletions(-) delete mode 100644 common/djangoapps/util/log_sensitive.py delete mode 100644 common/djangoapps/util/tests/test_log_sensitive.py diff --git a/common/djangoapps/util/log_sensitive.py b/common/djangoapps/util/log_sensitive.py deleted file mode 100644 index 6495c81bf2..0000000000 --- a/common/djangoapps/util/log_sensitive.py +++ /dev/null @@ -1,208 +0,0 @@ -""" -Utilities for logging sensitive debug information such as authentication tokens. - -Usage: - -1. Generate keys using ``python3 -m common.djangoapps.util.log_sensitive gen-keys`` -2. Follow the instructions it prints out, and pay close attention to the warning - at the end of the output -3. When logging sensitive information, use like so:: - - logger.info( - "Received invalid auth token %s in Authorization header", - encrypt_for_log(token, getattr(settings, 'YOUR_DEBUG_PUBLIC_KEY', None)) - ) - - This will log a message like:: - - Received invalid auth token [encrypted: ZXI...fFo=|IYS...1KA==] in Authorization header - -4. If you need to decrypt one of these messages, save the encrypted portion - to file, retrieve the securely held private key, and run - ``python3 -m common.djangoapps.util.log_sensitive decrypt --help`` - for instructions. -""" - -from base64 import b64decode, b64encode - -import click -from nacl.public import Box, PrivateKey, PublicKey - -# Background: -# -# The NaCl "Box" construction provides asymmetric encryption, allowing -# the sender to encrypt something for a recipient without having a -# shared secret. This encryption is authenticated, meaning that the -# recipient verifies that the message matches the sender's public key -# (proof of sender). But it's also *repudiable* authentication; the -# design allows both the sender and the receiver to read (or have -# created!) the encrypted message, so the receipient can't prove to -# anyone *else* that the sender was the author. -# -# Why we use ephemeral sender keys: -# -# The Box is normally an ideal construction to use for -# communications. However, we don't want the logger to be able to read -# the messages it writes, especially not messages from a different -# server instance or from days or weeks ago. Only developers (or -# others) in possession of the recipient keypair should be able to -# read it, not anyone who compromises a server at some later -# date. Luckily, we also don't care about authenticating the logged -# messages as truly being from the server! The solution is for each -# server to create a fresh public/private keypair at startup and to -# include a copy of the public key in any encrypted logs it writes. - - -# Generate an ephemeral private key for the logger to use during this -# logging session. -logger_private_key = PrivateKey.generate() - - -def encrypt_for_log(message, reader_public_key_b64): - """ - Encrypt a message so that it can be logged using the given public key, - but only read by someone possessing the matching private key. The - public key is provided in base64. - - A separate keypair should be used for each recipient or purpose. - - 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()) - - pubkey = logger_private_key.public_key - combined = b64encode(bytes(pubkey)).decode() + '|' + b64encode(encrypted).decode() - # The goal of this framing text is to make it always clear in log - # messages that the information is encrypted - return f"[encrypted: {combined}]" - - -def decrypt_log_message(encrypted_message, reader_private_key_b64): - """ - Decrypt a message using the private key that has been stored somewhere - secure and *not* on the server. - """ - reader_private_key = PrivateKey(b64decode(reader_private_key_b64)) - sender_public_key_data, encrypted_raw = \ - [b64decode(part) for part in encrypted_message.split('|', 1)] - return Box(reader_private_key, PublicKey(sender_public_key_data)).decrypt(encrypted_raw).decode() - - -def generate_reader_keys(): - """ - Utility method for generating a public/private keypair for use with these - logging functions. - """ - reader_private_key = PrivateKey.generate() - return { - 'public': b64encode(bytes(reader_private_key.public_key)).decode(), - 'private': b64encode(bytes(reader_private_key)).decode(), - } - - -@click.group() -def cli(): - pass - - -@click.command('gen-keys', help="Generate keypair") -def cli_gen_keys(): - """ - Generate and print a keypair for handling sensitive log messages. - """ - reader_keys = generate_reader_keys() - public_64 = reader_keys['public'] - private_64 = reader_keys['private'] - print( - "This is your PUBLIC key, which should be included in the server's " - "configuration. Create a separate setting (and keypair) for each " - "distinct project or team. This value does not need special protection:" - "\n\n" - f" settings. = \"{public_64}\"" - "\n\n" - "This is your PRIVATE key, which must never be present on the server " - "and should instead be kept encrypted in a separate, safe place " - "such as a password manager:" - "\n\n" - f" \"{private_64}\" (private)" - "\n\n" - "WARNING: Before logging anything sensitive, get a legal/compliance review to " - "ensure this is acceptable in your organization. Encryption is not " - "generally a replacement for retention policies or other privacy " - "safeguards; using this utility does not automatically make sensitive " - "information safe to handle." - ) - - -@click.command('decrypt', help="""Decrypt a logged message. - -If possible, use bash process indirection to keep the private key from -touching disk or shell history unencrypted. The safest way is to keep -the private key in an encrypted file: - - --private-key-file <(gpg2 --decrypt auth-logging-key.enc) - -Alternatively, you could copy it from a password manager to your -clipboard and use a CLI clipboard tool to retrieve it: - -\b - --private-key-file <(xsel -bo) # Linux - --private-key-file <(pbpaste) # Mac - -Another option is to somehow get the private key into an environment -variable and echo it out: - - --private-key-file <(echo "$PRIVATE_KEY") - -The same techniques can also be used for the encrypted message data, -which is less sensitive but should also be handled with care. -""") -@click.option( - '--private-key-file', type=click.File('r'), required=True, - help="Path to file containing reader's private key in Base64", -) -@click.option( - '--message-file', type=click.File('r'), required=True, - help="Path to file containing encrypted message, or - for stdin", -) -def cli_decrypt(private_key_file, message_file): - """ - Decrypt a message and print it to stdout. - """ - print(decrypt_log_message(message_file.read(), private_key_file.read())) - - -@click.command('encrypt', help="Encrypt a one-off message (for testing)") -@click.option('--public-key', help="Reader's public key, in Base64") -@click.option( - '--message-file', type=click.File('r'), required=True, - help="Path to file containing message to encrypt, or - for stdin", -) -def cli_encrypt(public_key, message_file): - """ - Encrypt a message to the provided public key and print it to stdout. - - This is just intended for use when testing or experimenting with the decrypt command. - """ - print(encrypt_for_log(message_file.read(), public_key)) - - -cli.add_command(cli_gen_keys) -cli.add_command(cli_decrypt) -cli.add_command(cli_encrypt) - -if __name__ == '__main__': - cli() diff --git a/common/djangoapps/util/tests/test_log_sensitive.py b/common/djangoapps/util/tests/test_log_sensitive.py deleted file mode 100644 index 7def5b1256..0000000000 --- a/common/djangoapps/util/tests/test_log_sensitive.py +++ /dev/null @@ -1,34 +0,0 @@ -""" -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'] - reader_private_64 = reader_keys['private'] - - to_log = encrypt_for_log("Testing testing 1234", reader_public_64) - re_base64 = r'[a-zA-Z0-9/+=]' - assert re.fullmatch(f'\\[encrypted: {re_base64}+\\|{re_base64}+\\]', to_log) - - to_decrypt = to_log.partition('[encrypted: ')[2].rstrip(']') - - decrypted = decrypt_log_message(to_decrypt, reader_private_64) - assert decrypted == "Testing testing 1234" - - # Also check that decryption still works if someone accidentally - # copies in the trailing framing "]" character, just as a - # nice-to-have. (base64 module should handle this already, since - # it stops reading at the first invalid base64 character.) - decrypted_again = decrypt_log_message(to_decrypt + ']', reader_private_64) - assert decrypted_again == "Testing testing 1234" diff --git a/openedx/core/djangoapps/safe_sessions/middleware.py b/openedx/core/djangoapps/safe_sessions/middleware.py index 01144cf3b0..6892da7577 100644 --- a/openedx/core/djangoapps/safe_sessions/middleware.py +++ b/openedx/core/djangoapps/safe_sessions/middleware.py @@ -91,10 +91,10 @@ from django.http import HttpResponse from django.utils.crypto import get_random_string from django.utils.deprecation import MiddlewareMixin from edx_django_utils.cache import RequestCache +from edx_django_utils.logging import encrypt_for_log from edx_django_utils.monitoring import set_custom_attribute from edx_toggles.toggles import SettingToggle -from common.djangoapps.util.log_sensitive import encrypt_for_log from openedx.core.djangoapps.user_authn.cookies import delete_logged_in_cookies from openedx.core.lib.mobile_utils import is_request_from_mobile_app diff --git a/requirements/edx/base.in b/requirements/edx/base.in index 6e8b1c0723..d4b0d6407b 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -79,7 +79,7 @@ edx-celeryutils edx-completion edx-django-release-util # Release utils for the edx release pipeline edx-django-sites-extensions -edx-django-utils>=4.0.0 # Utilities for cache, monitoring, and plugins +edx-django-utils>=4.7.0 # Utilities for cache, monitoring, and plugins edx-drf-extensions edx-enterprise edx-milestones @@ -134,7 +134,6 @@ pyjwkest PyJWT>=1.6.3 pylti1p3 # Required by content_libraries core library to suport LTI 1.3 launches pymongo # MongoDB driver -PyNaCl # User-friendly cryptography (wrapper and bindings for libsodium) pynliner # Inlines CSS styles into HTML for email notifications python-dateutil python-Levenshtein diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index a5f0ee9b1f..c665945835 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -132,6 +132,7 @@ click==8.1.2 # click-plugins # click-repl # code-annotations + # edx-django-utils # nltk # user-util click-didyoumean==0.3.0 @@ -445,7 +446,7 @@ edx-django-release-util==1.2.0 # blockstore edx-django-sites-extensions==4.0.0 # via -r requirements/edx/base.in -edx-django-utils==4.6.0 +edx-django-utils==4.7.0 # via # -r requirements/edx/base.in # blockstore @@ -824,7 +825,7 @@ pymongo==3.10.1 # mongodbproxy # mongoengine pynacl==1.5.0 - # via -r requirements/edx/base.in + # via edx-django-utils pynliner==0.8.0 # via -r requirements/edx/base.in pyparsing==3.0.8 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index bde99e6831..eb3e3f4a0e 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -176,6 +176,7 @@ click==8.1.2 # click-plugins # click-repl # code-annotations + # edx-django-utils # edx-lint # nltk # pact-python @@ -558,7 +559,7 @@ edx-django-release-util==1.2.0 # blockstore edx-django-sites-extensions==4.0.0 # via -r requirements/edx/testing.txt -edx-django-utils==4.6.0 +edx-django-utils==4.7.0 # via # -r requirements/edx/testing.txt # blockstore @@ -1127,7 +1128,9 @@ pymongo==3.10.1 # mongodbproxy # mongoengine pynacl==1.5.0 - # via -r requirements/edx/testing.txt + # via + # -r requirements/edx/testing.txt + # edx-django-utils pynliner==0.8.0 # via -r requirements/edx/testing.txt pyparsing==3.0.8 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index f505330494..e3ecb0df1f 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -79,7 +79,6 @@ attrs==21.4.0 # blockstore # edx-ace # openedx-events - # outcome # pytest babel==2.10.1 # via @@ -170,6 +169,7 @@ click==8.1.2 # click-plugins # click-repl # code-annotations + # edx-django-utils # edx-lint # nltk # pact-python @@ -541,7 +541,7 @@ edx-django-release-util==1.2.0 # blockstore edx-django-sites-extensions==4.0.0 # via -r requirements/edx/base.txt -edx-django-utils==4.6.0 +edx-django-utils==4.7.0 # via # -r requirements/edx/base.txt # blockstore @@ -1062,7 +1062,9 @@ pymongo==3.10.1 # mongodbproxy # mongoengine pynacl==1.5.0 - # via -r requirements/edx/base.txt + # via + # -r requirements/edx/base.txt + # edx-django-utils pynliner==0.8.0 # via -r requirements/edx/base.txt pyparsing==3.0.8