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 Aug 31, 2023
1 parent 213bcf0 commit 65a042f
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Zooming canvas when scrooling comments list in an issue (<https://github.com/opencv/cvat/pull/6758>)
- Issues can be created many times when initial submit (<https://github.com/opencv/cvat/pull/6758>)
- A race condition during initial `secret_key.py` creation
(<https://github.com/opencv/cvat/pull/6775>)

### Security
- TDB
Expand Down
40 changes: 33 additions & 7 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,43 @@
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:
chars = 'abcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*(-_=+)'
f.write("SECRET_KEY = '{}'\n".format(get_random_string(50, chars)))

chars = 'abcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*(-_=+)'
secret_key_fname = 'secret_key.py' # nosec

f = None
try:
with tempfile.NamedTemporaryFile(
mode='wt', dir=keys_dir, prefix=secret_key_fname + ".", delete=False,
) as f:
f.write("SECRET_KEY = '{}'\n".format(get_random_string(50, chars)))

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
finally:
if f: # f can be None if the NamedTemporaryFile constructor fails.
os.unlink(f.name)

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 65a042f

Please sign in to comment.