Skip to content

Commit d41398d

Browse files
alexeygrigorevalexeygrigorev
committed
Merge issue-128-transport-staging: harden development content staging (#128)
2 parents 7cd611f + ee50cf3 commit d41398d

3 files changed

Lines changed: 438 additions & 20 deletions

File tree

‎_docs/runbooks/development-course-content-bootstrap.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ key. Do not put it in Git, the container image, CI artifacts, issue attachments,
3333
logs. Disable worker/scheduler execution and all outbound message/provider behavior on the one-off
3434
task.
3535

36+
The released non-root container stages the version-pinned download beneath the fixed writable
37+
ephemeral root `/tmp`, never beneath the read-only application tree and never from a caller-supplied
38+
path or `TMPDIR`. The command creates one atomically named mode-`0700` directory and one
39+
no-follow/exclusive mode-`0600` regular file, verifies their ownership and permissions, and removes
40+
both on success and on every fail-closed path. The task needs no root user, broad storage mount, or
41+
additional filesystem permission.
42+
3643
Run:
3744

3845
```text

‎courses/services/development_content_transport.py‎

Lines changed: 122 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,23 @@
33
from __future__ import annotations
44

55
import os
6+
import stat
67
import tempfile
78
from collections.abc import Iterator
89
from contextlib import contextmanager
910
from pathlib import Path
1011

11-
from django.conf import settings
12-
1312
from courses.services.development_content_import import (
1413
APPROVED_SOURCE_SIZE,
1514
DevelopmentContentImportError,
1615
)
1716

1817

18+
_EPHEMERAL_STAGING_ROOT = Path("/tmp")
19+
_STAGING_DIRECTORY_PREFIX = "dtc-course-content-"
20+
_STAGING_FILENAME = "artifact.sqlite3"
21+
22+
1923
def _assert_transport_metadata(response: dict, expected_kms_key_arn: str) -> None:
2024
if response.get("ServerSideEncryption") != "aws:kms":
2125
raise DevelopmentContentImportError("transport-not-kms-encrypted")
@@ -37,6 +41,99 @@ def _close_response_body(response: dict | None) -> None:
3741
pass
3842

3943

44+
def _validated_ephemeral_staging_root() -> Path:
45+
root = _EPHEMERAL_STAGING_ROOT
46+
try:
47+
metadata = root.lstat()
48+
except OSError:
49+
raise DevelopmentContentImportError("transport-local-storage-failed") from None
50+
permissions = stat.S_IMODE(metadata.st_mode)
51+
shared_writes = permissions & (stat.S_IWGRP | stat.S_IWOTH)
52+
if (
53+
not root.is_absolute()
54+
or stat.S_ISLNK(metadata.st_mode)
55+
or not stat.S_ISDIR(metadata.st_mode)
56+
or (shared_writes and not permissions & stat.S_ISVTX)
57+
):
58+
raise DevelopmentContentImportError("transport-local-storage-failed")
59+
try:
60+
writable = os.access(root, os.W_OK | os.X_OK, effective_ids=True)
61+
except (NotImplementedError, OSError):
62+
raise DevelopmentContentImportError("transport-local-storage-failed") from None
63+
if not writable:
64+
raise DevelopmentContentImportError("transport-local-storage-failed")
65+
return root
66+
67+
68+
def _remove_private_staging(directory: Path | None, path: Path | None) -> bool:
69+
cleaned = True
70+
if path is not None:
71+
try:
72+
path.unlink(missing_ok=True)
73+
except OSError:
74+
cleaned = False
75+
if directory is not None:
76+
try:
77+
directory.rmdir()
78+
except OSError:
79+
cleaned = False
80+
return cleaned
81+
82+
83+
def _create_private_staging_file() -> tuple[Path, Path, int]:
84+
root = _validated_ephemeral_staging_root()
85+
directory: Path | None = None
86+
path: Path | None = None
87+
descriptor = -1
88+
owned_directory = False
89+
try:
90+
directory = Path(tempfile.mkdtemp(prefix=_STAGING_DIRECTORY_PREFIX, dir=root))
91+
if directory.parent != root or not directory.name.startswith(_STAGING_DIRECTORY_PREFIX):
92+
raise OSError("unexpected staging directory")
93+
directory_metadata = directory.lstat()
94+
if (
95+
stat.S_ISLNK(directory_metadata.st_mode)
96+
or not stat.S_ISDIR(directory_metadata.st_mode)
97+
or directory_metadata.st_uid != os.geteuid()
98+
):
99+
raise OSError("unsafe staging directory")
100+
owned_directory = True
101+
os.chmod(directory, 0o700, follow_symlinks=False)
102+
directory_metadata = directory.lstat()
103+
if stat.S_IMODE(directory_metadata.st_mode) != 0o700:
104+
raise OSError("staging directory is not private")
105+
106+
path = directory / _STAGING_FILENAME
107+
nofollow = getattr(os, "O_NOFOLLOW", None)
108+
if nofollow is None:
109+
raise OSError("no-follow file creation is unavailable")
110+
flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | nofollow
111+
flags |= getattr(os, "O_CLOEXEC", 0)
112+
descriptor = os.open(path, flags, 0o600)
113+
os.fchmod(descriptor, 0o600)
114+
descriptor_metadata = os.fstat(descriptor)
115+
path_metadata = path.lstat()
116+
if (
117+
not stat.S_ISREG(descriptor_metadata.st_mode)
118+
or descriptor_metadata.st_uid != os.geteuid()
119+
or stat.S_IMODE(descriptor_metadata.st_mode) != 0o600
120+
or stat.S_ISLNK(path_metadata.st_mode)
121+
or path_metadata.st_dev != descriptor_metadata.st_dev
122+
or path_metadata.st_ino != descriptor_metadata.st_ino
123+
):
124+
raise OSError("unsafe staging file")
125+
return directory, path, descriptor
126+
except Exception:
127+
if descriptor >= 0:
128+
try:
129+
os.close(descriptor)
130+
except OSError:
131+
pass
132+
if owned_directory:
133+
_remove_private_staging(directory, path)
134+
raise DevelopmentContentImportError("transport-local-storage-failed") from None
135+
136+
40137
@contextmanager
41138
def downloaded_s3_artifact(
42139
*,
@@ -75,20 +172,13 @@ def downloaded_s3_artifact(
75172
_close_response_body(response)
76173
raise DevelopmentContentImportError("transport-download-failed") from None
77174

78-
scratch = Path(settings.BASE_DIR) / ".tmp"
175+
staging_directory = None
79176
path = None
80177
descriptor = -1
81178
total = 0
82179
try:
83180
try:
84-
scratch.mkdir(mode=0o700, parents=True, exist_ok=True)
85-
descriptor, raw_path = tempfile.mkstemp(
86-
prefix="course-content-",
87-
suffix=".sqlite3",
88-
dir=scratch,
89-
)
90-
path = Path(raw_path)
91-
os.chmod(path, 0o600)
181+
staging_directory, path, descriptor = _create_private_staging_file()
92182
destination = os.fdopen(descriptor, "wb")
93183
descriptor = -1
94184
with destination:
@@ -99,15 +189,21 @@ def downloaded_s3_artifact(
99189
total += len(chunk)
100190
if total > APPROVED_SOURCE_SIZE:
101191
raise DevelopmentContentImportError("transport-size-mismatch")
102-
destination.write(chunk)
192+
try:
193+
destination.write(chunk)
194+
except OSError:
195+
raise DevelopmentContentImportError(
196+
"transport-local-storage-failed"
197+
) from None
103198
except DevelopmentContentImportError:
104199
raise
105200
except Exception:
106-
raise DevelopmentContentImportError(
107-
"transport-download-failed"
108-
) from None
109-
destination.flush()
110-
os.fsync(destination.fileno())
201+
raise DevelopmentContentImportError("transport-download-failed") from None
202+
try:
203+
destination.flush()
204+
os.fsync(destination.fileno())
205+
except OSError:
206+
raise DevelopmentContentImportError("transport-local-storage-failed") from None
111207
except DevelopmentContentImportError:
112208
raise
113209
except Exception:
@@ -118,11 +214,17 @@ def downloaded_s3_artifact(
118214
raise DevelopmentContentImportError("transport-local-storage-failed")
119215
yield path
120216
finally:
217+
cleaned = True
121218
if descriptor >= 0:
122-
os.close(descriptor)
219+
try:
220+
os.close(descriptor)
221+
except OSError:
222+
cleaned = False
123223
_close_response_body(response)
124-
if path is not None:
125-
path.unlink(missing_ok=True)
224+
if not _remove_private_staging(staging_directory, path):
225+
cleaned = False
226+
if not cleaned:
227+
raise DevelopmentContentImportError("transport-local-cleanup-failed")
126228

127229

128230
def delete_s3_artifact_version(

0 commit comments

Comments
 (0)