Add secure thumb drive creation premisses #1446
Conversation
|
sizes.txt of x230-hotp-maximized artifacts on this PR at 9332883 #sizes.txt of x230-hotp-maximized artifacts master at d7b4a47 Let's compare them So increase of size without tuning is observable from compressed initrd.cpio.xz mainly from changes from version bump of busybox and includion of mke2fs and exfat tools for and increase Question: do we really need journalizing on the LUKS encompassed ext4 partition? We otherwise have ext2 out of the box with busybox... Might be enough. Wil ltry to tune mke2fs further, while it is to note that no legacy board build broke by lack of space!. |
|
@JonathonHall-Purism Are we willing to increase consumed space by 189440bytes to add native support for exfat and ext4? Thoughts? |
|
@tlaurion Yes, I'm willing to trade off ~189KB for exFAT and ext4. I just had two users run into flash drive mounting problems last week, I think both were exFAT-formatted. I haven't reviewed the changes yet, but have you enabled the option to use ext4 to mount ext2? It may reclaim some of the space cost if you haven't, since we would not need ext2 modules any more. |
|
@JonathonHall-Purism yes, ext4 is used to mount ext2. Will review kernel modules once more, thanks for the pointer. |
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
from
9332883
to
41ca197
Compare
@JonathonHall-Purism it does! Review when you can please! test.sh will be your friend under QEMU (which passes 128mb raw file convenient to test risk free here.) Todo
|
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
2 times, most recently
from
d447cab
to
054700e
Compare
|
@JonathonHall-Purism ready for review! Testable through test.sh that needs to be removed prior of merge. |
|
Oups. Some boards were left behind. Readding them. |
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
from
054700e
to
9e19c15
Compare
|
Unfortunately, Talos II is linux 5.5 based, which doesn't offer EXFATFS. |
There was a problem hiding this comment.
@tlaurion Thanks for letting me know this was ready for review! The structure looks pretty good, I have suggested a handful of improvements for robustness/simplicity/etc.
I will try it out later too 👍
initrd/bin/mount-usb
Outdated
|
|
||
| TRACE "Under /bin/mount-usb" | ||
|
|
||
| function usage() { | ||
| cat <<USAGE_END | ||
| usage: $0 [options...] <-mode [ro|rw]> <-device> <-mountpoint mountpoint> <-pass passhphrase> |
There was a problem hiding this comment.
-device should have an argument
passphrase has an extra h
initrd/bin/mount-usb
Outdated
| mkdir /media | ||
| else | ||
| DEBUG "Cleaning /media directory" | ||
| umount /media > /dev/null 2>&1 || true |
There was a problem hiding this comment.
Use "$MOUNTPOINT" instead of /media
initrd/bin/test.sh
Outdated
|
|
||
| enable_usb | ||
| enable_usb_storage | ||
| prepare_thumb_drive /dev/sda 25 "test" |
There was a problem hiding this comment.
Is this going to appear in menus anywhere or just something we suggest to do from the recovery shell for now?
There was a problem hiding this comment.
@JonathonHall-Purism will be used under non-default oem-factory-reset/Re-Ownership to create GPG backup thumb drive of in-memory created gpg key material under LUKS encrypted partition, where public partition used to store public key. And then Thumb drive is useable in case USB Security dongle is lost if that option was selected on Re-Ownership, until USB Security dongle replacement is received. And then another PR will enable USB thumb drive subkeys restoration on USB Security dongle received replacement. And finally, OEM Factory Reset will permit to not depend on a USB Security dongle altogether altogether. And then another PR will enable authentication in case USB thumb drive backup of GPG key material exist, which will be useable to test detach sign/verify, which will permit authentication prior of recovery shell access, flashing and USB boot.
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
from
9e19c15
to
57f59c4
Compare
|
Annnnnd.... we lack space under legacy boards again. |
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
from
57f59c4
to
eb3ac0b
Compare
|
@JonathonHall-Purism I addressed your review! Thanks a bunch! Tested manually from whiptail-tpm1 (removed test.sh so that its ready to merge) |
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
3 times, most recently
from
571bf42
to
1806c95
Compare
initrd/bin/mount-usb
Outdated
| DEBUG "Parameters: --mode="$MODE", --device=${DEVICE:-empty}, --mountpoint="$MOUNTPOINT", --pass=${PASS:+provided}" | ||
|
|
||
| enable_usb | ||
| enable_usb_storage | ||
|
|
||
| if [ ! -d /media ]; then | ||
| mkdir /media | ||
| if [ ! -d "$MOUNTPOINT" ]; then | ||
| DEBUG "Creating "$MOUNTPOINT" directory" | ||
| mkdir -p "$MOUNTPOINT" > /dev/null 2>&1 | ||
| else | ||
| DEBUG "Cleaning "$MOUNTPOINT" directory" |
There was a problem hiding this comment.
This might be an over-application of my suggestion to "put quotes around expansions" - these expansions were already quoted since they were inside the quoted string.
The extra quotes now cause $MODE, $MOUNTPOINT, etc. to be word-split (and the quotes won't be printed around them).
If you don't want to print any quotes, just remove them: "Parameters: --mode=$MODE ..."
If you do want to print quotes, you have to escape them (or do weirder stuff mixing in single quotes): "Parameters: --mode=\"$MODE\" ..."
(The sort-of-exception to this is if you have a command expansion, then arguments to the command still need to be quoted, i.e. BLKID="$(blkid "$DEV")".)
There was a problem hiding this comment.
@JonathonHall-Purism I think I fixed them all
initrd/bin/mount-usb
Outdated
| # Check if the user has specified a USB device | ||
| if [ -n "$DEVICE" ]; then | ||
| DEBUG "Checking if $DEVICE is a USB device" | ||
| if grep -q $DEVICE /tmp/usb_block_devices; then |
There was a problem hiding this comment.
Not sure why this one got flagged as "outdated" since this code has not changed - I still suggest reworking list_usb_devices and testing just the device you care about rather than the roundabout "enumerate everything and test, then check if my device was in the results" with the potential problems relating to the naïve grep
initrd/etc/luks-functions
Outdated
| { | ||
| #generate a list of devices to choose from that contain a LUKS header | ||
| lvm vgscan||true | ||
| blkid | cut -d ':' -f 1 | while read device;do cryptsetup isLuks $device;if [ $(echo $?) == 0 ]; then echo $device;fi; done | sort |
There was a problem hiding this comment.
This doesn't appear to have changed in the current version
| # Check if the user has specified a USB device | ||
| if [ -n "$DEVICE" ]; then | ||
| DEBUG "Checking if "$DEVICE" is a USB device" | ||
| if grep -q "$DEVICE" /tmp/usb_block_devices; then |
There was a problem hiding this comment.
Visibility comment for #1446 (comment) since I am tired of wrestling with Github's idea that it is outdated 😝
|
#1403 now merged. Rebasing so that no board fails packing linux suport for exfat + exfatprogs/e2fsprogs. |
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
from
1806c95
to
6cca444
Compare
|
Will address review tomorrow, not sure what happened here but thanks for holding your point! Will be cleaner and will poke you back. |
|
Ok, legacy boards still have manageable amount of free space but that will most probaly be the final blow to them. I don't even know for whom i'm maintaining those 7.5mb images. |
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
2 times, most recently
from
00a5fd9
to
e032852
Compare
@JonathonHall-Purism updated tests |
|
@JonathonHall-Purism I should have addressed all your previous comments. I will let you set them as resolved this time instead of making Github confused, I guess. |
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
from
e032852
to
50aee3e
Compare
|
@JonathonHall-Purism Corrected everything previously reviewed. Note that under qemu, usb fb needs to be wiped to be recreated with 256mb otherwise 10% of it is not sufficient to create a LUKS container on 12mb. Testing: |
|
Oups. Pushed fixes for qemu boards but forgot to pass default back to 10% |
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
from
50aee3e
to
b016baf
Compare
initrd/etc/functions
Outdated
| # List all USB storage devices, including partitions unless we received argument stating we want drives only | ||
| # The output is a list of device names, one per line. | ||
|
|
||
| #Ouput debug info if we received argument otherwise show "empty" instead |
There was a problem hiding this comment.
Nit: spelling of "Output"
initrd/etc/luks-functions
Outdated
|
|
||
| DEBUG "DEVICE to partition: $DEVICE" | ||
| DEBUG "PERCENTAGE of device that will be used for LUKS container: $PERCENTAGE" | ||
| #Ouput provided" if passphrase is provided as parameter |
There was a problem hiding this comment.
Nit: spelling of Output and spurious quotation mark
initrd/etc/luks-functions
Outdated
| #Prompt for passphrase if not provided as parameter | ||
| if [ -z "$PASSPHRASE" ]; then | ||
| #If no passphrase was provided, ask user to select passphrase for LUKS container | ||
| whiptail --title 'Enter passphrase for LUKS container' --passwordbox \ | ||
| "Enter passphrase for LUKS container:" 0 80 2> /tmp/passphrase \ | ||
| || die "Error: No passphrase provided" | ||
| #remove trailing newline and/or carriage return | ||
| PASSPHRASE=$(cat /tmp/passphrase | tr -d '\n' | tr -d '\r') | ||
| fi |
There was a problem hiding this comment.
- I don't think fbwhiptail supports
--passwordbox - If we have to support devices lacking whiptail/fbwhiptail, you probably need to support it here (or otherwise remove the non-whiptail fallback above)
- Move this out of
if [ -z "$DEVICE" ];, seems odd that password is only optional if device was not provided (right?) - The passphrase file should go in /tmp/secret, and
at_exit shred "/tmp/secret/passphrase"to remove it sooner would probably be a good idea
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
2 times, most recently
from
876136a
to
4a2e5f4
Compare
|
@JonathonHall-Purism review addressed! |
prepare_thumb_drive: default to creating 10% LUKS container on usb drive, prompts for passphrase is not provided and scan drives if no --device specified NOTE: qemu usb_thumb drive of 128 mb are not big enough so that 10% of it (12mb) can be used to create thumb drive. Adds: - e2fsprogs to support ext4 filesystem creation through mke2fs - add /etc/mke2fs.conf so that mke2fs knows how to handle ext2/ext3/ext4 - removes mke2fs support from busybox - bump busybox to latest version which adds cpu accelerated hash functions (not needed per se here) - Adds exfatprogs to have mkfs.exfat and fsck.exfat - Adds prepare_thumb_drive /etc/luks-functions to be able to prepare a thumb drive with percentage of drive assigned to LUKS, rest to exfat - Modify most board configs to test space requirements failing - Talos2 linux config: add staging Exfat support - Make e2fsprogs and exfatprogs included by default unless explicitely deactivate in board configs - Change cryptsetup calls : luksOpen to open and luksClose to close to addresss review - etc/luks_functions: cleanup GOAL here is to have secure thumb drive creation which Heads will be able to use to backup/restore/use generated GPG key material in the future (next PR)
… 256MB Otherwise 10% of 128mb (12mb) is not enough to create a LUKS container
tlaurion
force-pushed
the
Add_secure_thumb_drive_premisses
branch
from
4a2e5f4
to
d5aa0c8
Compare
|
Thanks @tlaurion , looks good to me, let's merge! 🚢 |
The idea of this PR is provide helpers needed to plug a brand new usb thumb drive into Heads and have Heads being able to create two partitions: one he will use to store key material and secrets (LUKS contained ext4 partition) and one public (exfat).
To do so this PR:
The logic is based on prompting the user to enter a device, a percentage of the drive to the LUKS container and a passphrase for that container through
etc/luks_functions:prepare_thumb_drive(). Then mount-usb was refactored to facilitate external calling with parameters, including passing down--device--mountpoint--passand--modewhich if not specified, use old defaults:--mode ro--mountpoint /mediawithout password, which prompts the user if many devices were found for him to select, and if the selected partition is a LUKS device, asks passphrase to unlock it and mount it. If a user tries to remount a device, it unmounts first and close the luks volume. It is also possible to mount multiple devices with different mountpoints.Those can be parametrized to ease other callers to cache/reuse eg. passphrases and only prompt once and pass info to the mount-usb helper. The caller is responsible to cache and pass down the information, otherwise the calle will fail down and be explicit on its failing cause.
TODO: