diff --git a/docs/mscolab.rst b/docs/mscolab.rst index 744b89542..58fd9efa3 100644 --- a/docs/mscolab.rst +++ b/docs/mscolab.rst @@ -17,6 +17,14 @@ Description of the variables can be found in comments. .. literalinclude:: samples/config/mscolab/mscolab_settings.py.sample +:code:`SECRET_KEY` signs the login tokens, so anyone who knows it can log in as any user. Use a random key of +your own with at least 32 characters, e.g. from :code:`python -c "import secrets; print(secrets.token_urlsafe(32))"`, +and pass it in the environment variable :code:`MSCOLAB_SECRET_KEY`, which wins over :code:`SECRET_KEY` in +mscolab_settings.py. There is no default: the server and the :code:`mscolab` command refuse to start without a key, +with a shorter key, with fewer than 10 different characters or with :code:`'MySecretKey'` from earlier versions of +this sample. All processes of the server, e.g. gunicorn workers, have to use the same key. :code:`ADMIN_TOKEN`, if +you set it, needs at least 16 characters. + .. _configuration-mscolab: Protecting Login @@ -132,8 +140,9 @@ Then install gunicorn by pixi:: $ pixi global install gunicorn -and start the server by:: +and start the server with your secret key (see :code:`SECRET_KEY` above) by:: + $ export MSCOLAB_SECRET_KEY='' $ gunicorn -b 0.0.0.0:8087 server:app For further options read ``_ diff --git a/docs/samples/config/mscolab/mscolab_settings.py.sample b/docs/samples/config/mscolab/mscolab_settings.py.sample index 5c2935915..757e5e052 100644 --- a/docs/samples/config/mscolab/mscolab_settings.py.sample +++ b/docs/samples/config/mscolab/mscolab_settings.py.sample @@ -9,7 +9,7 @@ This file is part of mss. :copyright: Copyright 2019 Shivashis Padhi - :copyright: Copyright 2019-2025 by the MSS team, see AUTHORS. + :copyright: Copyright 2019-2026 by the MSS team, see AUTHORS. :license: APACHE-2.0, see LICENSE for details. Licensed under the Apache License, Version 2.0 (the "License"); @@ -61,8 +61,16 @@ UPLOAD_FOLDER = os.path.join(DATA_DIR, 'uploads') # Max image/document upload size in mscolab chat (default 2MB) MAX_UPLOAD_SIZE = 2 * 1024 * 1024 -# Set your secret key for token generation -SECRET_KEY = 'MySecretKey' +# Secret key that signs the login tokens and the email confirmation and password reset links. +# Anyone who knows it can log in as any user, so keep it secret and use a key of your own. +# Create one with +# python -c "import secrets; print(secrets.token_urlsafe(32))" +# and pass it to the server in the environment variable MSCOLAB_SECRET_KEY, which wins +# over this file, or set it here: +# SECRET_KEY = '' +# The server does not start without a key, with a key shorter than 32 characters or with +# fewer than 10 different characters. All processes of the server (e.g. gunicorn workers) +# and the mscolab command line tool must use the same key. # looks for a given category for an operation ending with GROUP_POSTFIX # e.g. category = Tex will look for TexGroup diff --git a/mslib/mscolab/app/__init__.py b/mslib/mscolab/app/__init__.py index 665ca2684..f30b5f5f5 100644 --- a/mslib/mscolab/app/__init__.py +++ b/mslib/mscolab/app/__init__.py @@ -57,6 +57,16 @@ # This can be used to set a location by SCRIPT_NAME for testing. e.g. export SCRIPT_NAME=/demo/ SCRIPT_NAME = os.environ.get('SCRIPT_NAME', '/') +# SECRET_KEY values published in earlier versions of the sample mscolab_settings.py +PUBLISHED_SECRET_KEYS = ('MySecretKey',) +# RFC 7518 3.2: an HS256 key must be at least as long as the hash, 256 bits +MIN_SECRET_KEY_LENGTH = 32 +# ADMIN_TOKEN is compared, not used as a key, its default secrets.token_urlsafe(16) has 22 characters +MIN_ADMIN_TOKEN_LENGTH = 16 +# a secret needs at least this many different characters, e.g. 'a' * 32 has one, a random one of 32 about 25 +MIN_SECRET_DISTINCT_CHARACTERS = 10 +SECRET_HINT = f'Create one with: python -c "import secrets; print(secrets.token_urlsafe({MIN_SECRET_KEY_LENGTH}))"' + message, update = release_info.check_for_new_release() if update: @@ -212,6 +222,53 @@ def initialise_db(): logging.info("Database initialised successfully!") +class InsecureSecretError(RuntimeError): + """A secret of the MSColab settings, SECRET_KEY or ADMIN_TOKEN, is missing or can be guessed""" + + +def check_secret(name, secret, min_length, where): + """ + Raises InsecureSecretError if secret can't keep its tokens from being forged + + SECRET_KEY signs the login tokens, the email confirmation and password reset tokens and the Flask + sessions, anyone who knows it can log in as any user. ADMIN_TOKEN lets local processes trigger + socket.io events. where says where the secret is set, for the message. + """ + if secret is None or secret == "" or secret == b"": + raise InsecureSecretError(f"{name} is not set. Set it in {where}. {SECRET_HINT}") + if not isinstance(secret, (str, bytes)): + raise InsecureSecretError(f"{name} must be a string. Set it in {where}. {SECRET_HINT}") + if secret in PUBLISHED_SECRET_KEYS: + raise InsecureSecretError( + f"{name} is the published sample value {secret!r}, with it anyone can log in as any user. " + f"Set a secret of your own in {where}. {SECRET_HINT}") + if len(secret) < min_length: + raise InsecureSecretError( + f"{name} must have at least {min_length} characters. Set it in {where}. {SECRET_HINT}") + if secret != secret.strip(): + raise InsecureSecretError(f"{name} must not start or end with whitespace. Set it in {where}.") + if len(set(secret)) < MIN_SECRET_DISTINCT_CHARACTERS: + raise InsecureSecretError( + f"{name} must have at least {MIN_SECRET_DISTINCT_CHARACTERS} different characters, it can be " + f"guessed. Set it in {where}. {SECRET_HINT}") + + +def check_secrets(app): + """ + Takes SECRET_KEY from the environment variable MSCOLAB_SECRET_KEY if it is set and checks the secrets + + The environment variable wins over mscolab_settings, e.g. over an old settings file with the published + sample key. There is no random default: every process of the server has to sign tokens with the same key, + a random key per process would log users out at random with several workers and on every restart. + """ + secret_key = os.environ.get("MSCOLAB_SECRET_KEY", "").strip() + if secret_key: + app.config["SECRET_KEY"] = secret_key + check_secret("SECRET_KEY", app.config.get("SECRET_KEY"), MIN_SECRET_KEY_LENGTH, + "the environment variable MSCOLAB_SECRET_KEY or in your mscolab_settings") + check_secret("ADMIN_TOKEN", app.config.get("ADMIN_TOKEN"), MIN_ADMIN_TOKEN_LENGTH, "your mscolab_settings") + + def create_app(config_object=mscolab_settings): """Create and configure an MSColab Flask application. @@ -221,9 +278,13 @@ def create_app(config_object=mscolab_settings): The returned app is not connected to a database schema yet, call :func:`initialise_db` within an application context of it to do so. + + :raises InsecureSecretError: if SECRET_KEY or ADMIN_TOKEN is missing or can be guessed, + see :func:`check_secrets`. """ app = Flask(__name__, template_folder=DOCS_TEMPLATES_DIR) app.config.from_object(config_object) + check_secrets(app) # Expose docs path for callers/tests and make it part of Flask config for consistency. app.config['DOCS_SERVER_PATH'] = DOCS_SERVER_PATH app.route = prefix_route(app.route, SCRIPT_NAME) diff --git a/mslib/mscolab/conf.py b/mslib/mscolab/conf.py index 5eaa11ccf..b201c6030 100644 --- a/mslib/mscolab/conf.py +++ b/mslib/mscolab/conf.py @@ -84,8 +84,9 @@ class DefaultSettings: UPLOAD_FOLDER = os.path.join(DATA_DIR, 'uploads') MAX_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MiB - # used to generate and parse tokens - SECRET_KEY = secrets.token_urlsafe(16) + # used to generate and parse tokens. There is no default, the server doesn't start without it. Set it in the + # environment variable MSCOLAB_SECRET_KEY, which wins over this setting, or in your mscolab_settings. + SECRET_KEY = None # used to generate the password token SECURITY_PASSWORD_SALT = secrets.token_urlsafe(16) diff --git a/mslib/mscolab/mscolab.py b/mslib/mscolab/mscolab.py index da6ad367f..e4b0c073a 100644 --- a/mslib/mscolab/mscolab.py +++ b/mslib/mscolab/mscolab.py @@ -41,7 +41,7 @@ from mslib import __version__ from mslib.mscolab import migrations -from mslib.mscolab.app import create_app, create_files +from mslib.mscolab.app import InsecureSecretError, create_app, create_files from mslib.mscolab.seed import seed_data, add_user, add_all_users_default_operation, \ add_all_users_to_all_operations, delete_user from mslib.utils import setup_logging @@ -403,6 +403,15 @@ def handle_sso_metadata_init(repo_exists): def main(): + try: + _main() + except InsecureSecretError as ex: + # a configuration error, no traceback + print(f"mscolab: {ex}", file=sys.stderr) + sys.exit(1) + + +def _main(): parser = argparse.ArgumentParser() parser.add_argument("-v", "--version", help="show version", action="store_true", default=False) diff --git a/mslib/mscolab/server.py b/mslib/mscolab/server.py index fa5b731f3..4b8ea704f 100644 --- a/mslib/mscolab/server.py +++ b/mslib/mscolab/server.py @@ -26,12 +26,13 @@ """ import logging import hashlib +import sys from flask import current_app, jsonify, request from flask_cors import CORS from flask_httpauth import HTTPBasicAuth -from mslib.mscolab.app import create_app, initialise_db +from mslib.mscolab.app import InsecureSecretError, create_app, initialise_db from mslib.mscolab.conf import mscolab_settings from mslib.mscolab.sockets_manager import _setup_managers @@ -118,7 +119,12 @@ def start_server(app, sockio, cm, fm, port=8083): def main(): - app = create_server_app() + try: + app = create_server_app() + except InsecureSecretError as ex: + # a configuration error, no traceback + print(f"mscolab: {ex}", file=sys.stderr) + sys.exit(1) start_server(app, app.extensions['sockio'], app.extensions['cm'], app.extensions['fm']) diff --git a/tests/_test_mscolab/test_server.py b/tests/_test_mscolab/test_server.py index e4c5d5bd3..e8b98cbf5 100644 --- a/tests/_test_mscolab/test_server.py +++ b/tests/_test_mscolab/test_server.py @@ -24,19 +24,27 @@ See the License for the specific language governing permissions and limitations under the License. """ +import copy import datetime import pytest import json import io import os +import runpy +import secrets +import sys import mock from PIL import Image from flask import current_app +import mslib.mscolab.mscolab +import mslib.mscolab.server +from mslib.mscolab.app import InsecureSecretError, create_app from mslib.mscolab.auth import register_user, check_login +from mslib.mscolab.conf import DefaultSettings, mscolab_settings from mslib.mscolab.models import User, Operation from mslib.mscolab.utils import ATTACHMENTS_URL_PREFIX @@ -648,3 +656,91 @@ def _upload_profile_image(self, test_client, token, email): } response = test_client.post('/upload_profile_image', data=data) return response + + +SAMPLE_SETTINGS = os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, + "docs", "samples", "config", "mscolab", "mscolab_settings.py.sample") + + +@pytest.fixture +def no_secret_key_in_environment(monkeypatch): + monkeypatch.delenv("MSCOLAB_SECRET_KEY", raising=False) + + +VALID_SECRET_KEY = "0123456789abcdefghijklmnopqrstuv" + + +@pytest.mark.parametrize("secret_key, message", [ + (None, "SECRET_KEY is not set"), + ("", "SECRET_KEY is not set"), + (10 ** 40, "SECRET_KEY must be a string"), + ("MySecretKey", "SECRET_KEY is the published sample value 'MySecretKey'"), + ("a" * 31, "SECRET_KEY must have at least 32 characters"), + (b"a" * 31, "SECRET_KEY must have at least 32 characters"), + ("secretkEyu", "SECRET_KEY must have at least 32 characters"), + ("a" * 32, "SECRET_KEY must have at least 10 different characters"), + (" " * 32, "SECRET_KEY must not start or end with whitespace"), + ("MySecretKey" * 3, "SECRET_KEY must have at least 10 different characters"), + (VALID_SECRET_KEY + "\n", "SECRET_KEY must not start or end with whitespace"), +]) +def test_create_app_refuses_insecure_secret_key(no_secret_key_in_environment, secret_key, message): + settings = copy.copy(mscolab_settings) + settings.SECRET_KEY = secret_key + with pytest.raises(InsecureSecretError, match=message): + create_app(settings) + + +@pytest.mark.parametrize("admin_token, message", [ + ("", "ADMIN_TOKEN is not set"), + ("0123456789abcde", "ADMIN_TOKEN must have at least 16 characters"), + ("a" * 22, "ADMIN_TOKEN must have at least 10 different characters"), +]) +def test_create_app_refuses_insecure_admin_token(no_secret_key_in_environment, admin_token, message): + settings = copy.copy(mscolab_settings) + settings.ADMIN_TOKEN = admin_token + with pytest.raises(InsecureSecretError, match=message): + create_app(settings) + + +# fixed ids: pytest-xdist needs the same test ids in every worker, the random keys would differ +@pytest.mark.parametrize("secret_key", [VALID_SECRET_KEY, secrets.token_urlsafe(24), secrets.token_urlsafe(32)], + ids=["fixed", "random-32-characters", "random-43-characters"]) +def test_create_app_accepts_secret_key(no_secret_key_in_environment, secret_key): + settings = copy.copy(mscolab_settings) + settings.SECRET_KEY = secret_key + assert create_app(settings).config["SECRET_KEY"] == secret_key + + +def test_environment_secret_key_wins_over_settings(monkeypatch): + # e.g. an old settings file with the published sample key + monkeypatch.setenv("MSCOLAB_SECRET_KEY", VALID_SECRET_KEY + "\n") + settings = copy.copy(mscolab_settings) + settings.SECRET_KEY = "MySecretKey" + assert create_app(settings).config["SECRET_KEY"] == VALID_SECRET_KEY + + +def test_no_default_secret_key(no_secret_key_in_environment): + # a random key per process would log users out at random with several workers and on every restart + assert DefaultSettings.SECRET_KEY is None + with pytest.raises(InsecureSecretError, match="SECRET_KEY is not set"): + create_app(DefaultSettings()) + + +def test_sample_settings_have_no_secret_key(no_secret_key_in_environment): + # the operator sets it in MSCOLAB_SECRET_KEY or in the file, a copied sample doesn't start + assert "SECRET_KEY" not in runpy.run_path(SAMPLE_SETTINGS) + + +@pytest.mark.parametrize("module, argv", [ + (mslib.mscolab.mscolab, ["mscolab", "db", "--seed", "-y"]), + (mslib.mscolab.server, ["server"]), +]) +def test_cli_reports_insecure_secret_without_traceback(monkeypatch, capsys, module, argv): + monkeypatch.setattr(sys, "argv", argv) + error = InsecureSecretError("SECRET_KEY is not set") + with mock.patch.object(module, "create_app", side_effect=error), \ + mock.patch.object(module, "create_server_app", side_effect=error, create=True): + with pytest.raises(SystemExit) as exit_info: + module.main() + assert exit_info.value.code == 1 + assert capsys.readouterr().err == "mscolab: SECRET_KEY is not set\n" diff --git a/tests/server_setup.py b/tests/server_setup.py index ebfbcc4d7..46cefc599 100644 --- a/tests/server_setup.py +++ b/tests/server_setup.py @@ -108,7 +108,7 @@ def ensure_mscolab_config(): ENGINEIO_LOGGER = True # used to generate and parse tokens -SECRET_KEY = secrets.token_urlsafe(16) +SECRET_KEY = secrets.token_urlsafe(32) # used to generate the password token SECURITY_PASSWORD_SALT = secrets.token_urlsafe(16)