From e1a9b2390a36f3ee28c30047ce6899ca59a380a1 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Tue, 5 Sep 2023 23:02:00 +0300 Subject: [PATCH] Fix a race condition with initial secret_key.py creation (#6775) Currently, it's possible (even if unlikely) for multiple backend processes to create and use different Django `SECRET_KEY` values, because the following scenario can happen: * process 1: fail to import `secret_key.py` * process 2: fail to import `secret_key.py` * process 1: generate a new `secret_key.py` * process 1: import `secret_key.py` * process 2: generate a new `secret_key.py` * process 2: import `secret_key.py` Fix this by making it so that `secret_key.py` is created atomically, and never overwritten if it already exists. In addition, only generate the secret key if the import fails due to the module not being found, since other failure reasons suggest incorrect configuration or data corruption, and so require administrator attention. --- CHANGELOG.md | 2 ++ cvat/settings/base.py | 35 ++++++++++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6e2c86867f7..c8435644fe7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Running deep learning models on non-jpeg compressed tif images () - Paddings on tasks/projects/models pages () - Memory leak in the logging system () +- A race condition during initial `secret_key.py` creation + () ### Security - TDB diff --git a/cvat/settings/base.py b/cvat/settings/base.py index eebcfeacbeea..c1cceeaace0b 100644 --- a/cvat/settings/base.py +++ b/cvat/settings/base.py @@ -21,6 +21,7 @@ import shutil import subprocess import sys +import tempfile from datetime import timedelta from distutils.util import strtobool from enum import Enum @@ -40,18 +41,42 @@ ALLOWED_HOSTS = os.environ.get('ALLOWED_HOSTS', 'localhost,127.0.0.1').split(',') INTERNAL_IPS = ['127.0.0.1'] -try: - sys.path.append(BASE_DIR) - from keys.secret_key import SECRET_KEY # pylint: disable=unused-import -except ImportError: +def generate_secret_key(): + """ + Creates secret_key.py in such a way that multiple processes calling + this will all end up with the same key (assuming that they share the + same "keys" directory). + """ from django.utils.crypto import get_random_string keys_dir = os.path.join(BASE_DIR, 'keys') if not os.path.isdir(keys_dir): os.mkdir(keys_dir) - with open(os.path.join(keys_dir, 'secret_key.py'), 'w') as f: + + secret_key_fname = 'secret_key.py' # nosec + + with tempfile.NamedTemporaryFile( + mode='wt', dir=keys_dir, prefix=secret_key_fname + ".", + ) as f: chars = 'abcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*(-_=+)' f.write("SECRET_KEY = '{}'\n".format(get_random_string(50, chars))) + + # Make sure the file contents are written before we link to it + # from the final location. + f.flush() + + try: + os.link(f.name, os.path.join(keys_dir, secret_key_fname)) + except FileExistsError: + # Somebody else created the secret key first. + # Discard ours and use theirs. + pass + +try: + sys.path.append(BASE_DIR) + from keys.secret_key import SECRET_KEY # pylint: disable=unused-import +except ModuleNotFoundError: + generate_secret_key() from keys.secret_key import SECRET_KEY