Merge lp:~smoser/cloud-initramfs-tools/trunk.lp1641678 into lp:cloud-initramfs-tools

Proposed by Scott Moser
Status: Merged
Merged at revision: 130
Proposed branch: lp:~smoser/cloud-initramfs-tools/trunk.lp1641678
Merge into: lp:cloud-initramfs-tools
Diff against target: 44 lines (+12/-6)
2 files modified
overlayroot/etc/overlayroot.conf (+2/-0)
overlayroot/scripts/init-bottom/overlayroot (+10/-6)
To merge this branch: bzr merge lp:~smoser/cloud-initramfs-tools/trunk.lp1641678
Reviewer Review Type Date Requested Status
Seth Arnold (community) Approve
Dustin Kirkland  Needs Information
cloud-initramfs-tools Pending
Review via email: mp+310796@code.launchpad.net

Commit message

overlayroot: write the password to consistent filename

Previously, when overlayroot=crypt was used, and no password was
provided, the password was stored to a filename in /run/initramfs/
named overlayroot.XXXXXX. The XXXXXX template was random.

This just made it more difficult to read that password file.

Now, we publish the passfile as /run/initramfs/overlayroot.passwd.
Note, that by design the password file name fits into the template's
possible filenames. This means that if a tool was looking for
 /run/initramfs/overlayroot.??????
then it would find
 /run/initramfs/overlayroot.passwd

To post a comment you must log in.
131. By Scott Moser

name file to overlayroot.passwd to match previous template.

overlayroot.passwd is a (unlikely but) possible name that could
have been produced by mktemp. So, anyone looking defensively for
a file matching 'overlayroot.??????' will also now match
overlayroot.passwd.

The new name thus seems more suitable for stable release update.

132. By Scott Moser

update doc to reference password file

Revision history for this message
Dustin Kirkland  (kirkland) wrote :

It's okay with me, but you might want to ask the security team (Tyler, Marc, Jamie?) to review. There are attacks against insecurely created temporary files: https://www.owasp.org/index.php/Insecure_Temporary_File I think what you're doing here is okay, since you're creating the temporary file, and then renaming it to the common name. But I do recommend you check with them.

review: Needs Information
Revision history for this message
Seth Arnold (seth-arnold) wrote :

Looks good to me:

- $entropy_sources includes several kernel-provided randomness sources, and reasonable efforts to protect against weak kernel rng
- ${PERSIST_DIR} is created early in the script so the mktemp(1) call shouldn't fail
- the temporary file and the final file are on the same device so mv(1) should be atomic

Thanks

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'overlayroot/etc/overlayroot.conf'
2--- overlayroot/etc/overlayroot.conf 2016-09-23 19:11:19 +0000
3+++ overlayroot/etc/overlayroot.conf 2016-11-14 17:54:50 +0000
4@@ -48,6 +48,8 @@
5 # the name of the map device to be created in /dev/mapper
6 # * pass: default: ""
7 # if not provided or empty, password is randomly generated
8+# the generated password will be stored for recovery in
9+# /run/initramfs/overlayroot.passwd
10 # * fstype: default: "ext4"
11 # mapname=mapper,pass=foo,fstype=ext4,mkfs=1
12 # * mkfs: default: 1
13
14=== modified file 'overlayroot/scripts/init-bottom/overlayroot'
15--- overlayroot/scripts/init-bottom/overlayroot 2016-11-14 17:23:53 +0000
16+++ overlayroot/scripts/init-bottom/overlayroot 2016-11-14 17:54:50 +0000
17@@ -222,17 +222,21 @@
18 [ "$mkfs" = "0" ] &&
19 { log_fail "mkfs=0, but no password provided"; return 1; }
20 entropy_sources="$entropy_sources $dev"
21- local pass_file=""
22- pass_file=$(mktemp "${PERSIST_DIR}/${MYTAG}.XXXXXX") ||
23+ local tmpf="" pass_file="${PERSIST_DIR}/${MYTAG}.passwd"
24+ tmpf=$(mktemp "${PERSIST_DIR}/${MYTAG}.XXXXXX") ||
25 { log_fail "failed creation of password file"; return 1; }
26- stat -L /dev/* /proc/* /sys/* >"${pass_file}" 2>&1 ||
27+ stat -L /dev/* /proc/* /sys/* >"$tmpf" 2>&1 ||
28 { log_warn "could not seed with stat entropy [$entropy_sources]"; }
29- head -c 4096 $entropy_sources >> "${pass_file}" ||
30+ head -c 4096 $entropy_sources >> "$tmpf" ||
31 { log_fail "failed reading entropy [$entropy_sources]"; return 1; }
32- pass=$(sha512sum "${pass_file}") ||
33+ pass=$(sha512sum "$tmpf") ||
34 { log_fail "failed generation of password"; return 1; }
35 pass=${pass%% *}
36- printf "%s" "${pass}" > "${pass_file}"
37+ printf "%s" "${pass}" > "$tmpf" ||
38+ { log_fail "failed to record password to '$tmpf'"; return 1; }
39+ mv "$tmpf" "${pass_file}" ||
40+ { log_fail "failed rename '$tmpf' to '${pass_file}'"; return 1; }
41+ log_debug "stored password in ${pass_file}"
42 fi
43
44 log_warn "setting up new luks device at $dev"

Subscribers

People subscribed via source and target branches