From 2f13cf16a6c8a78adb80dca18cfada3110e5cd1f Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Mon, 6 Feb 2023 13:41:44 -0500 Subject: [PATCH 1/3] Fix app creation in source.wsgi, add regression test In journalist.wsgi and source.wsgi, we import the relvant `app` variable so it can be executed by mod_wsgi. In bea3a0a9a this was accidentally refactored to be under the `if __name__ == "__main__"` block, so it wasn't available to be imported. To prevent this from recurring, add a documentation comment making it clear the variable needs to be globally accessible. A regression test mostly verifies that the wsgi files can be executed without errors. This required a slight modification to `source_app.create_app()` which tries to scan a directory that doesn't exist when tests are run. If the folder doesn't exist, we can skip that entire routine since there's no data to obscure. Fixes #6741. --- securedrop/journalist.py | 1 + securedrop/source.py | 6 ++++-- securedrop/source_app/__init__.py | 17 ++++++++++------- securedrop/tests/test_wsgi.py | 22 ++++++++++++++++++++++ 4 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 securedrop/tests/test_wsgi.py diff --git a/securedrop/journalist.py b/securedrop/journalist.py index 94f67c0041..07757a2976 100644 --- a/securedrop/journalist.py +++ b/securedrop/journalist.py @@ -5,6 +5,7 @@ from sdconfig import SecureDropConfig config = SecureDropConfig.get_current() +# app is imported by journalist.wsgi app = create_app(config) diff --git a/securedrop/source.py b/securedrop/source.py index ce5f495e48..176f416b8a 100644 --- a/securedrop/source.py +++ b/securedrop/source.py @@ -1,9 +1,11 @@ from sdconfig import SecureDropConfig from source_app import create_app +config = SecureDropConfig.get_current() +# app is imported by source.wsgi +app = create_app(config) + if __name__ == "__main__": # pragma: no cover - config = SecureDropConfig.get_current() - app = create_app(config) debug = getattr(config, "env", "prod") != "prod" # nosemgrep: python.flask.security.audit.app-run-param-config.avoid_app_run_with_bad_host app.run(debug=debug, host="0.0.0.0", port=8080) # nosec diff --git a/securedrop/source_app/__init__.py b/securedrop/source_app/__init__.py index a7301d71bf..c646f10e98 100644 --- a/securedrop/source_app/__init__.py +++ b/securedrop/source_app/__init__.py @@ -118,12 +118,15 @@ def internal_error(error: werkzeug.exceptions.HTTPException) -> Tuple[str, int]: # Obscure the creation time of source private keys by touching them all # on startup. private_keys = config.GPG_KEY_DIR / "private-keys-v1.d" - now = time.time() - for entry in os.scandir(private_keys): - if not entry.is_file() or not entry.name.endswith(".key"): - continue - os.utime(entry.path, times=(now, now)) - # So the ctime is also updated - os.chmod(entry.path, entry.stat().st_mode) + # The folder may not exist yet in some dev/testing setups, + # and if it doesn't exist there's no mtime to obscure. + if private_keys.is_dir(): + now = time.time() + for entry in os.scandir(private_keys): + if not entry.is_file() or not entry.name.endswith(".key"): + continue + os.utime(entry.path, times=(now, now)) + # So the ctime is also updated + os.chmod(entry.path, entry.stat().st_mode) return app diff --git a/securedrop/tests/test_wsgi.py b/securedrop/tests/test_wsgi.py new file mode 100644 index 0000000000..dc544f6c30 --- /dev/null +++ b/securedrop/tests/test_wsgi.py @@ -0,0 +1,22 @@ +import os +import subprocess +import sys +from pathlib import Path + +import pytest + + +@pytest.mark.parametrize("filename", ("journalist.wsgi", "source.wsgi")) +def test_wsgi(filename): + """ + Verify that all setup code and imports in the wsgi files work + + This is slightly hacky because it executes the wsgi files using + the current virtualenv, and we hack the paths so it works out. + """ + sd_dir = Path(__file__).parent.parent + wsgi = sd_dir / "debian/app-code/var/www" / filename + python_path = os.getenv("PYTHONPATH", "") + subprocess.check_call( + [sys.executable, str(wsgi)], env={"PYTHONPATH": f"{python_path}:{sd_dir}"} + ) From a0751e60d870a90d02e1a716d7cfbc85e05449c8 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Tue, 7 Feb 2023 15:18:36 -0500 Subject: [PATCH 2/3] Use correct permissions when installing source.wsgi.logging in staging The file needs to be readable by www-data, so the "world" spot should be set to 4 instead of 0. --- .../ansible-base/roles/app-test/tasks/staging_wsgi_files.yml | 2 +- molecule/testinfra/app/test_appenv.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/install_files/ansible-base/roles/app-test/tasks/staging_wsgi_files.yml b/install_files/ansible-base/roles/app-test/tasks/staging_wsgi_files.yml index 9f40a08c7e..4f005e6017 100644 --- a/install_files/ansible-base/roles/app-test/tasks/staging_wsgi_files.yml +++ b/install_files/ansible-base/roles/app-test/tasks/staging_wsgi_files.yml @@ -15,6 +15,6 @@ src: source.wsgi.logging dest: /var/www/source.wsgi owner: "root" - mode: "0640" + mode: "0644" tags: - apache diff --git a/molecule/testinfra/app/test_appenv.py b/molecule/testinfra/app/test_appenv.py index 88e25dec38..a3cce8ec53 100644 --- a/molecule/testinfra/app/test_appenv.py +++ b/molecule/testinfra/app/test_appenv.py @@ -21,7 +21,7 @@ def test_app_wsgi(host): f = host.file("/var/www/source.wsgi") with host.sudo(): assert f.is_file - assert f.mode == 0o640 + assert f.mode == 0o644 assert f.user == "root" assert f.group == "root" assert f.contains("^import logging$") From 193b8d1d94059031e549e43da5bdcb6b850e5cda Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Mon, 6 Feb 2023 15:41:31 -0500 Subject: [PATCH 3/3] Add basic testinfra smoke tests to verify SI and JI are up Use curl on the host itself to verify that both the JI and SI reply with HTTP 200 OK and have "Powered by..." in the HTML. Notably this would've caught #6741. --- molecule/testinfra/app/test_smoke.py | 33 ++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 molecule/testinfra/app/test_smoke.py diff --git a/molecule/testinfra/app/test_smoke.py b/molecule/testinfra/app/test_smoke.py new file mode 100644 index 0000000000..6145745c1e --- /dev/null +++ b/molecule/testinfra/app/test_smoke.py @@ -0,0 +1,33 @@ +""" +Basic smoke tests that verify the apps are functioning as expected +""" +import pytest +import testutils + +sdvars = testutils.securedrop_test_vars +testinfra_hosts = [sdvars.app_hostname] + + +@pytest.mark.parametrize( + "name,url,curl_flags", + ( + # We pass -L to follow the redirect from / to /login + ("journalist", "http://localhost:8080/", "L"), + ("source", "http://localhost:80/", ""), + ), +) +def test_interface_up(host, name, url, curl_flags): + """ + Ensure the respective interface is up with HTTP 200 if not, we try our + best to grab the error log and print it via an intentionally failed + assertion. + """ + response = host.run(f"curl -{curl_flags}i {url}").stdout + if "200 OK" not in response: + # Try to grab the log and print it via a failed assertion + with host.sudo(): + f = host.file(f"/var/log/apache2/{name}-error.log") + if f.exists: + assert "nopenopenope" in f.content_string + assert "200 OK" in response + assert "Powered by" in response