Skip to content

Commit 6ddbdeb

Browse files
authored
PMM-15316 Stage SEP secret files before renaming
publish() interleaved the write and the rename per file, so a write that died part-way through the set - a full volume being the realistic cause - left SEP some files rotated and the rest stale. Verified against the previous revision: a failure on the third password file rotated two of them and left the third behind. Split it into stage() and commit_staged(). Every fallible step now runs before the first rename, so the same failure leaves the previous complete set in place and the script still exits nonzero. The temporary is recorded for cleanup immediately after mktemp rather than after the write, since errexit abandons the rest of stage() on a failed write and an unrecorded temporary is one the EXIT trap cannot find. This narrows rather than closes the gap: the set is still four renames, so a failure between them leaves the earlier files updated. Closing that needs a directory swap the mountpoint contract rules out. Signed-off-by: Yan Orestes <yan.orestes@percona.com>
1 parent bde170b commit 6ddbdeb

1 file changed

Lines changed: 42 additions & 11 deletions

File tree

build/ansible/roles/sep/files/sep-secrets

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,26 +29,55 @@ declare -a DB_PASSWORD_FILES=(
2929
TASKS__DATABASE__PASSWORD
3030
)
3131
declare -a MANAGED_FILES=(SECRET_KEY "${DB_PASSWORD_FILES[@]}")
32+
declare -a STAGED_TMP=() STAGED_DEST=()
3233
declare TMP_FILE=""
3334

3435
cleanup() {
36+
local tmp
37+
3538
[ -n "$TMP_FILE" ] && rm -f "$TMP_FILE"
39+
for tmp in "${STAGED_TMP[@]}"; do
40+
rm -f "$tmp"
41+
done
3642
return 0
3743
}
3844
trap cleanup EXIT
3945

4046
is_enabled() { [ "$1" = "1" ] || [ "$1" = "true" ]; }
4147

42-
# Written through a temporary file so SEP never reads a half-written secret, and with
48+
# Every fallible step - mktemp, the write, the chmod - runs here, before commit_staged
49+
# renames anything, so the failure that actually happens to this operation (a write dying
50+
# on a full volume) leaves SEP the previous complete set rather than a mix of new and
51+
# stale credentials. Interleaving write and rename per file is what makes that mix
52+
# reachable; the rename itself, within one filesystem and after a complete write, is not
53+
# the fallible part.
54+
#
4355
# printf '%s' so no trailing newline reaches a reader that strips rather than chomps.
44-
publish() {
45-
local name="$1" value="$2"
46-
47-
TMP_FILE=$(mktemp "$SECRETS_DIR/.${name}.XXXXXX")
48-
printf '%s' "$value" > "$TMP_FILE"
49-
chmod 0640 "$TMP_FILE"
50-
mv "$TMP_FILE" "$SECRETS_DIR/$name"
51-
TMP_FILE=""
56+
stage() {
57+
local name="$1" value="$2" tmp
58+
59+
tmp=$(mktemp "$SECRETS_DIR/.${name}.XXXXXX")
60+
# Recorded before the write, not after: errexit abandons the rest of this function on
61+
# a failed write, and a temporary created but not yet recorded is one cleanup cannot
62+
# find.
63+
STAGED_TMP+=("$tmp")
64+
STAGED_DEST+=("$SECRETS_DIR/$name")
65+
printf '%s' "$value" > "$tmp"
66+
chmod 0640 "$tmp"
67+
}
68+
69+
# Each rename replaces a whole file with one already written in full, so SEP never reads a
70+
# half-written secret. The set is still four renames rather than one operation: a failure
71+
# between them leaves the earlier files updated, which is the residue this cannot remove
72+
# without a directory swap the mountpoint contract rules out.
73+
commit_staged() {
74+
local i
75+
76+
for i in "${!STAGED_TMP[@]}"; do
77+
mv "${STAGED_TMP[$i]}" "${STAGED_DEST[$i]}"
78+
done
79+
STAGED_TMP=()
80+
STAGED_DEST=()
5281
}
5382

5483
# Never fatal, so the disabled path cannot fail a start over a directory this uid cannot
@@ -143,10 +172,12 @@ fi
143172

144173
# Republished on every start, not only on generation, so a wiped or newly attached secrets
145174
# volume is refilled from the persisted key rather than left short a file.
146-
publish SECRET_KEY "$SECRET_KEY_VALUE"
175+
stage SECRET_KEY "$SECRET_KEY_VALUE"
147176

148177
for name in "${DB_PASSWORD_FILES[@]}"; do
149-
publish "$name" "$PMM_SEP_POSTGRES_PASSWORD"
178+
stage "$name" "$PMM_SEP_POSTGRES_PASSWORD"
150179
done
151180

181+
commit_staged
182+
152183
echo "Published SEP's SECRET_KEY and database credentials to $SECRETS_DIR."

0 commit comments

Comments
 (0)