Normalize exporter failures, durable mkdir, reject swapped FIFO
Review found three robustness gaps:
- set -e + command substitution let a mailman-web that exits 3 abort the
script with exit 3, colliding with the reserved "nothing archived" code
so a caller could treat a failed export as empty. Wrap the invocation in
`... ) || { ...; exit 1; }` so every exporter failure normalizes to 1
and can never be read as 3.
- Crash durability missed a newly-created output directory: only the leaf
dir was fsynced after the rename, not the parent holding that new dir
entry. Create the output tree in-process and fsync each new level into
its parent before reporting success.
- A same-uid process could swap the /var/tmp temp for a FIFO; an
O_RDONLY open without O_NONBLOCK blocks forever before the regular-file
check, hanging the job instead of failing closed. Open with
O_NOFOLLOW|O_NONBLOCK, then fstat and reject anything non-regular.
Verified: mailman-web exit 3 -> script exit 1 (NONE still 3, garbage 1);
FIFO source -> rejected in <5s, no hang; publish into a missing nested
dir creates it durably and writes 0600; existing-dir path unregressed.
This commit is contained in:
parent
ddb3bc6908
commit
4069f26769
1 changed files with 34 additions and 14 deletions
|
|
@ -115,7 +115,9 @@ for chunk in iter(lambda: raw.read(1 << 16), b""):
|
|||
raw.close()
|
||||
print("MM3HK|FILE|%d|%s|%s" % (n, path, h.hexdigest()))
|
||||
EOF
|
||||
)
|
||||
) || { echo "mm3-hk-export: mailman-web shell failed; aborting, nothing exported" >&2; exit 1; }
|
||||
# Any non-zero from mailman-web is normalized to exit 1 above so a wrapper that
|
||||
# happens to exit 3 can never be misread as our reserved "nothing archived" code.
|
||||
|
||||
line=$(printf '%s\n' "$result" | grep '^MM3HK|' | tail -n1 || true)
|
||||
if [ -z "$line" ]; then
|
||||
|
|
@ -153,31 +155,53 @@ case "$status" in
|
|||
FILE)
|
||||
path=$(printf '%s' "$line" | cut -d'|' -f4)
|
||||
sha=$(printf '%s' "$line" | cut -d'|' -f5)
|
||||
mkdir -p "$(dirname "$out")"
|
||||
# Publish the web-user temp file as root without re-resolving its path.
|
||||
# The export lives in /var/tmp owned by the web user, so any process
|
||||
# with that uid could swap the name for a symlink between a name-based
|
||||
# check and a name-based copy (TOCTOU) and make root copy an arbitrary
|
||||
# root-readable file. So: open the source ONCE with O_NOFOLLOW (a swapped
|
||||
# symlink makes the open fail outright), confirm it's a regular file via
|
||||
# root-readable file. So: open the source once with O_NOFOLLOW (a swapped
|
||||
# symlink makes the open fail) plus O_NONBLOCK (a swapped FIFO returns
|
||||
# instead of blocking the open forever), confirm it's a regular file via
|
||||
# the open fd, copy from that fd into a fresh root-owned temp in the
|
||||
# destination dir while hashing, verify the SHA-256 matches what the
|
||||
# exporter reported (rejects a regular-file swap, truncation, or partial
|
||||
# write by another same-uid process), fsync, and atomically rename into
|
||||
# place. Paths + digest are passed as argv (heredoc is single-quoted) so
|
||||
# nothing is interpolated.
|
||||
# place. The output directory is created in-process so each newly-made
|
||||
# level is fsynced into its parent before success is reported. Paths and
|
||||
# digest are passed as argv (heredoc is single-quoted) so nothing is
|
||||
# interpolated.
|
||||
if ! python3 - "$path" "$out" "$sha" <<'PY'
|
||||
import os, sys, stat, tempfile, hashlib
|
||||
src, out, expected = sys.argv[1], sys.argv[2], sys.argv[3]
|
||||
dest_dir = os.path.dirname(out) or "."
|
||||
|
||||
def fsync_dir(d):
|
||||
dfd = os.open(d, os.O_RDONLY | os.O_DIRECTORY)
|
||||
try: os.fsync(dfd)
|
||||
finally: os.close(dfd)
|
||||
|
||||
# Create any missing output directories, fsyncing each new level into its parent
|
||||
# so a freshly-created backup directory is itself crash-durable, not just the
|
||||
# file later renamed into it.
|
||||
missing, d = [], os.path.abspath(dest_dir)
|
||||
while not os.path.isdir(d):
|
||||
missing.append(d)
|
||||
parent = os.path.dirname(d)
|
||||
if parent == d: break
|
||||
d = parent
|
||||
for made in reversed(missing):
|
||||
os.mkdir(made)
|
||||
fsync_dir(os.path.dirname(made))
|
||||
|
||||
try:
|
||||
fd = os.open(src, os.O_RDONLY | os.O_NOFOLLOW)
|
||||
fd = os.open(src, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)
|
||||
except OSError as e:
|
||||
sys.exit("mm3-hk-export: cannot open export temp (symlink swapped in?): %s" % e)
|
||||
sys.exit("mm3-hk-export: cannot open export temp (symlink/FIFO swapped in?): %s" % e)
|
||||
dtmp = None
|
||||
try:
|
||||
if not stat.S_ISREG(os.fstat(fd).st_mode):
|
||||
sys.exit("mm3-hk-export: export temp is not a regular file")
|
||||
dfd, dtmp = tempfile.mkstemp(prefix=".mm3hk.", dir=os.path.dirname(out) or ".")
|
||||
dfd, dtmp = tempfile.mkstemp(prefix=".mm3hk.", dir=dest_dir)
|
||||
h = hashlib.sha256()
|
||||
r = os.fdopen(fd, "rb"); fd = -1 # fd now owned by r
|
||||
with r, os.fdopen(dfd, "wb") as w: # dfd now owned by w
|
||||
|
|
@ -194,11 +218,7 @@ try:
|
|||
# fsync the containing directory so the new directory entry is crash-durable
|
||||
# before we report success: a caller that then destroys the list must not be
|
||||
# able to lose the backup to a crash that drops the unsynced rename.
|
||||
dirfd = os.open(os.path.dirname(out) or ".", os.O_RDONLY | os.O_DIRECTORY)
|
||||
try:
|
||||
os.fsync(dirfd)
|
||||
finally:
|
||||
os.close(dirfd)
|
||||
fsync_dir(dest_dir)
|
||||
except BaseException:
|
||||
if dtmp is not None:
|
||||
try: os.unlink(dtmp)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue