Skip to content

Commit

Permalink
Fix a race condition with initial secret_key.py creation
Browse files Browse the repository at this point in the history
Currently, it's possible (even if unlikely) for multiple backend processes
to create and use different Django `SECRET_KEY` values, because the
following scenario is possible:

* 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.
  • Loading branch information
SpecLad committed Sep 5, 2023
1 parent a1a4093 commit dc999b3
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 (<https://github.com/opencv/cvat/pull/6789>)
- Paddings on tasks/projects/models pages (<https://github.com/opencv/cvat/pull/6778>)
- Memory leak in the logging system (<https://github.com/opencv/cvat/pull/6804>)
- A race condition during initial `secret_key.py` creation
(<https://github.com/opencv/cvat/pull/6775>)

### Security
- TDB
Expand Down
35 changes: 30 additions & 5 deletions cvat/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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


Expand Down

0 comments on commit dc999b3

Please sign in to comment.