track files in /boot in kexec_tree.txt

[3hhh]

Fixes #1248

[tlaurion]

@JonathonHall-Purism Seems to work fine in my test without regression. People upgrading firmware would get an error of hash files not being present even if it is, the user being asked to update them. I think it would not be bad. A default boot, which checks things tighter, would still fail in current code base with same error anyway.

Only thing is that directories names test and test\n would give info to user as being duplicate directory names, but directory tree is nice improvement, indeed:

One thing I would do on next step (since Qemu overhead [not kvm] really shows difference of cpu time) would be to compare dir tree file first instead of checking for hashes (for show options which doesn't check detached signature since force option is passed) where default boot checks kexec.sig and exits first for performance improvement at later time?

[JonathonHall-Purism]

Nice work @3hhh ! I think this is a great direction.

I tested this out a bit and have some notes:

Initial install - new file .auditing-0

I did a clean setup from scratch in qemu (with PureOS but I don't think the distro matters). Everything went smoothly until it was about to boot - somehow a file .auditing-0 was created between signing the contents /boot and actually booting, so the boot failed due to a new file. I'm not sure what this file is or what created it, but I re-ran the whole process and it was definitely created by something in Heads at that point (it was not there just after OS install, or even after the OEM reset and TOTP/HOTP reset).

@tlaurion Any idea what creates /boot/.auditing-0? I could not find an obvious reference and searching came up empty so far. Maybe GPG or something?

Escape sequences / control characters

@tlaurion - You probably knew this, but just to get on the same page, your test showed a directory containing literally \ and n, not a line break character. Both cause problems, and in different ways:

The literal line break causes oddities but I'm more concerned with control characters being passed through the whiptail prompt 🤔

As a crude example, an attacker can make text look like it is part of the prompt from Heads:

On regular terminal whiptail, I would think you could craft terminal escapes to overwrite the entire prompt if you wanted to, and the premise is that these files could be attacker-controlled.

If we had bash, I would recommend to just escape all the file names with ${VAR@Q} or something and be done with it, but I can't find a straightforward solution in busybox ash. Maybe we need to bite the bullet and write some "quoter" in a regex or C? There are definitely other places we should use it, like the boot menu (could solve the spaces-to-underscores in the boot menu choices).

[tlaurion]

Initial install - new file .auditing-0

I did a clean setup from scratch in qemu (with PureOS but I don't think the distro matters). Everything went smoothly until it was about to boot - somehow a file .auditing-0 was created between signing the contents /boot and actually booting, so the boot failed due to a new file. I'm not sure what this file is or what created it, but I re-ran the whole process and it was definitely created by something in Heads at that point (it was not there just after OS install, or even after the OEM reset and TOTP/HOTP reset).

@tlaurion Any idea what creates /boot/.auditing-0? I could not find an obvious reference and searching came up empty so far. Maybe GPG or something?

EDIT: It happens through libtpm usage https://github.com/osresearch/tpmtotp/blob/18b860fdcf5a55537c8395b891f2b2a5c24fc00a/libtpm/auditing.c#L391 and I thought it was created as part of TPM ownership (the file is written if non-existing at https://github.com/osresearch/tpmtotp/blob/18b860fdcf5a55537c8395b891f2b2a5c24fc00a/libtpm/auditing.c#L513-L514
Otherwise, it must be part of seal-totp operation through tpm calls. Will have to dig into that, since dir_tree and hashes should include that file prior of detach signing hashes.

This PR taking into consideration new files as opposed to previously only existing files being modified: this exposes this new behavior.

@JonathonHall-Purism I'm confused by your report stating that OEM reownership doesn't create the file, since TPM ownership should, so I will need to test this further. Creating a debian-xfce disk image now, will snapshot prior of using it with heads to check behavior myself and will report back.

The literal line break causes oddities but I'm more concerned with control characters being passed through the whiptail prompt thinking

As a crude example, an attacker can make text look like it is part of the prompt from Heads:

On regular terminal whiptail, I would think you could craft terminal escapes to overwrite the entire prompt if you wanted to, and the premise is that these files could be attacker-controlled.

This is really, really bad and an invitation to go back to simplicity. Will go deeper and test, but I would prefer to go into presenting the raw changes with minimal modification (print_tree does that here) where showing the result would require additional sanitization that might be unneeded. Having the output on screen is not enough to investigate. I think this output should be presented through command line only, just like it is, currently, when 10+ files are detected as modified.

What about the following to entice thought experiment: no more showing those into whiptail and using raw output to enforce auditing:

diff --git a/initrd/bin/gui-init b/initrd/bin/gui-init
index 0d2e45f1..9bb6cb0f 100755
--- a/initrd/bin/gui-init
+++ b/initrd/bin/gui-init
@@ -105,7 +105,7 @@ verify_global_hashes()
       fi
 
     else
-      if [ $CHANGED_FILES_COUNT -gt 10 ]; then
+      if [ $CHANGED_FILES_COUNT -gt 0 ]; then
         # drop to console to show full file list
         whiptail $ERROR_BG_COLOR --title 'ERROR: Boot Hash Mismatch' \
           --msgbox "${CHANGED_FILES_COUNT} files failed the verification process!\\n\nThis could indicate a compromise!\n\nHit OK to review the list of files.\n\nType \"q\" to exit the list and return." 16 60
@@ -114,6 +114,7 @@ verify_global_hashes()
         less /tmp/hash_output_mismatches
         #move outdated hash mismatch list
         mv /tmp/hash_output_mismatches /tmp/hash_output_mismatch_old
+       warn "List of modified files moved to /tmp/hash_output_mismatch_old for further review from recovery shell."
         TEXT="Would you like to update your checksums now?"
       else
         TEXT="The following files failed the verification process:\n\n${CHANGED_FILES}\n\nThis could indicate a compromise!\n\nWould you like to update your checksums now?"

This way the user would be presented the same list he would need to dig into prior of signing if he doesn't recognize the change, which we expect he would do from recovery shell, telling him where to find the new/modified file list for inspection.

We would dodge having to cleanup output altogether (which would be problematic: we want the list to be as it is so the user can inspect what happened to his system, not miss other bullets).

@3hhh @JonathonHall-Purism Thoughts?

If we had bash, I would recommend to just escape all the file names with ${VAR@Q} or something and be done with it, but I can't find a straightforward solution in busybox ash. Maybe we need to bite the bullet and write some "quoter" in a regex or C? There are definitely other places we should use it, like the boot menu (could solve the spaces-to-underscores in the boot menu choices).

@JonathonHall-Purism : unfortunately, from my attempts on integrating tpm2 from hardenedvault fork which included prior work from Trammel in old stalled PR, including bash would be really costly and doesn't seem an option for production. Not relevant for this ticket, but I would love to add it somehow into qemu builds, since it would be easily possible to track caller/callee/sourced functions under warn/die calls to ease troubleshooting if bash was included. That would help a bunch, but here again ash is failing us.

There are definitely other places we should use it, like the boot menu (could solve the spaces-to-underscores in the boot menu choices).

@JonathonHall-Purism This is another area where a little more thought should be invested. In case of Qubes (multiboot), we lost a lot of precious output on what the user is actually trusting to booting into (at setting new boot default, for example). Xen parameters and Kernel parameters would need to be outputted more in detail (not less), where they were removed prior of expending height of whiptail output dynamically. I will revisit that later on, but I think this should be a separate issue, differently problematic than the current case. Here again, thoughts welcome.
EDIT: we reduced what is taken at boot option selection here: 7769d13. To be reconsidered: adding more details when selecting a default boot option might be desired to compensate, where having real spaces instead of underscores would be nice but yet again, we do not want to modify input too much: we want the user to see what is really going to be booted, not obscure things up.

[tlaurion]

Initial install - new file .auditing-0

I did a clean setup from scratch in qemu (with PureOS but I don't think the distro matters). Everything went smoothly until it was about to boot - somehow a file .auditing-0 was created between signing the contents /boot and actually booting, so the boot failed due to a new file. I'm not sure what this file is or what created it, but I re-ran the whole process and it was definitely created by something in Heads at that point (it was not there just after OS install, or even after the OEM reset and TOTP/HOTP reset).
@tlaurion Any idea what creates /boot/.auditing-0? I could not find an obvious reference and searching came up empty so far. Maybe GPG or something?

EDIT: It happens through libtpm usage https://github.com/osresearch/tpmtotp/blob/18b860fdcf5a55537c8395b891f2b2a5c24fc00a/libtpm/auditing.c#L391 and I thought it was created as part of TPM ownership (the file is written if non-existing at https://github.com/osresearch/tpmtotp/blob/18b860fdcf5a55537c8395b891f2b2a5c24fc00a/libtpm/auditing.c#L513-L514 Otherwise, it must be part of seal-totp operation through tpm calls. Will have to dig into that, since dir_tree and hashes should include that file prior of detach signing hashes.

This PR taking into consideration new files as opposed to previously only existing files being modified: this exposes this new behavior.

@JonathonHall-Purism I'm confused by your report stating that OEM reownership doesn't create the file, since TPM ownership should, so I will need to test this further. Creating a debian-xfce disk image now, will snapshot prior of using it with heads to check behavior myself and will report back.

qemu-coreboot-whiptail-tpm1
It happened to me only when sealing disk unlock key under TPM. Not when resetting TPM. Not when sealing TOTP.
And then, default boot shows error since .auditing-0 is present, which requires rehash+resigning.

qemu-coreboot-fbwhiptail-tpm1-hotp
Having weird behavior reusing clean qemu disk image and TPM with qemu-coreboot-fbwhiptail-hotp-tpm1.
Resetting TPM still results in NV unseal problem, htop fails and i'm stuck in a loop from menus going nowhere but main screen.... deleted vtpm dir under build dir but behavior is not normal. Will need more testing...

Edit: flashed master on x230-hotp-maximized to make sure things are not broken. So something weird with testing setup (swtpm, NV and hotp: no prompt for HOTP GPG admin PIN, and meu loop. Will rebuild qemu-coreboot-fbwhiptail-tpm1-hotp clean

[3hhh]

We would dodge having to cleanup output altogether (which would be problematic: we want the list to be as it is so the user can inspect what happened to his system, not miss other bullets).

@3hhh @JonathonHall-Purism Thoughts?

Yes, ideally untrusted output should be visibly separated from trusted output.
One could also display a prompt saying "Once you click OK, the next prompt will show you the file differences. Under no circumstances will it ask you to do anything. If so, this may be an attack." Alternatively show it with different background color, borders, ...

Some other remarks:

• My debian busybox has comm -z, which might be suited slightly better for the diffs. heads doesn't have it for some reason. Anyway the user output issue remains.
• It might be possible to filter out control characters via busybox tr -cd '[set of allowed characters]' or so (I only tested it in debian and for some reason character classes didn't work). Newlines still remain an issue, but one could e.g. separate trusted output from untrusted output with a sequence of unusual characters that are not included in the allowed set. Of course having something like printf '%q' would even be better.

[JonathonHall-Purism]

It's crude, but how about this:

escape.sh

#! /bin/ash

# Escape standard input for use in shell.
# These characters are passed verbatim: a-zA-Z0-9,._+:@%/-
# These backslash escapes are used to replace their corresponding characters: \n\r\t\ \v\b 
# Other characters are rendered as hexadecimal escapes

xxd -p | tr -d '\n' |
{
	while read -n2 -d$'\0' -r; do
		case "$REPLY" in
			08)
				echo -n '\b'
				;;
			09)
				echo -n '\t'
				;;
			0a)
				echo -n '\n'
				;;
			0b)
				echo -n '\v'
				;;
			0d)
				echo -n '\r'
				;;
			
			20)
				echo -n '\ '
				;;
			#%+,-./ 0-9:    @A-0      P-Z_     a-o    p-z
			2[5b-f]|3[0-9a]|4[0-9a-f]|5[0-9af]|6[1-f]|7[0-9a])
				echo -e -n '\x'"$REPLY"
				;;
			# All others are escaped
			[0-9a-f][0-9a-f])
				echo -n '\x'"$REPLY"
				;;
		esac
	done
}

test.sh

#! /bin/sh

TEST="bs\btab\tnl\nvt\vcr\rsp %+,-./ 0-9:@ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz\x12\x00\x25%{}|"
echo "$TEST"
echo -n -e "$TEST" | xxd -p; echo
echo -n -e "$TEST" | ./escape.sh; echo

# Escape some uncontrolled text to build a message
MSG="Some new files:"$'\n'
MSG="$MSG* $(echo -n "File with spaces.txt" | ./escape.sh)"$'\n'
MSG="$MSG* $(echo -n -e "File\nwith\newlines.txt" | ./escape.sh)"$'\n'
MSG="$MSG* $(echo -n "File\nwith\nescapes.txt" | ./escape.sh)"$'\n'

echo "$MSG"

I'm sure performance is abysmal but it shouldn't make much of a difference here I think, and at least it works.

[tlaurion]

@3hhh:

Some other remarks:

* My debian busybox has `comm -z`, which might be suited slightly better for the diffs. heads doesn't have it for some reason. Anyway the user output issue remains.

Could be added under config/busybox if needed

* It might be possible to filter out control characters via `busybox tr -cd '[set of allowed characters]'` or so (I only tested it in debian and for some reason character classes didn't work). Newlines still remain an issue, but one could e.g. separate trusted output from untrusted output with a sequence of unusual characters that are not included in the allowed set. Of course having something like `printf '%q'` would even be better.

@JonathonHall-Purism @3hhh : Struggling with tr, which should be able to be up the task following changelogs of bysybox and some quick test (where -cd seems to misbehave though...)

~ # find /boot | tr '[:lower:]' '[:upper:]' | tail -n 5
/BOOT/CONFIG-5.10.0-20-AMD64
/BOOT/TEST2
/BOOT/TEST
/BOOT/LOST+FOUND
/BOOT/INITRD.IMG-5.10.0-20-AMD64

[JonathonHall-Purism]

A quick test of tr -cd did work for me (echo "abcdefghij" | tr -cd "bcd", etc.), but IMO I would lean toward the "escaping" strategy for a few reasons:

• It avoids any ambiguity (control chars aren't silently omitted, line breaks don't look like two files, etc.)
• It doesn't require any complex warnings / descriptions that users are likely to ignore or not fully understand
• It also solves ambiguity in the kexec_tree.txt listing itself (store the escaped paths instead of raw paths, and then newlines in paths don't cause any issues)

The implementation may not be stellar but at least it's isolated and easy to test, if it becomes limiting we could always replace it with a tiny escaper in C instead.

What do you all think though, do you prefer the alternatives?

[tlaurion]

@JonathonHall-Purism

A quick test of tr -cd did work for me (echo "abcdefghij" | tr -cd "bcd", etc.), but IMO I would lean toward the "escaping" strategy for a few reasons:

* It avoids any ambiguity (control chars aren't silently omitted, line breaks don't look like two files, etc.)

* It doesn't require any complex warnings / descriptions that users are likely to ignore or not fully understand

* It also solves ambiguity in the kexec_tree.txt listing itself (store the escaped paths instead of raw paths, and then newlines in paths don't cause any issues)

The implementation may not be stellar but at least it's isolated and easy to test, if it becomes limiting we could always replace it with a tiny escaper in C instead.

What do you all think though, do you prefer the alternatives?

I'm still struggling to make tr do the right thing, which is to be able to sanitize through character classes with optimal speed.

From my understanding of the problem (going to basics) find reports files with invalid characters on multiple lines:

/boot # ls dir\ with\ new\\nline/
file      new?file  new?line
/boot # ls dir\ with\ new\\nline
file      new?file  new?line

EDIT (forgot -print0 from past test, same output):
find ./ ! -path './kexec*' -print0| tr "\n" "\0"| xargs -0 -n1
(excerpt)

./dir with new\nline
./dir with new\nline/new
line
./dir with new\nline/file
./dir with new\nline/new
file

Unfortunately, I haven't found any tricks, as of now, to have tr specify a character class that would remove all control characters but null, so that we could, in my understanding, dodge the problem efficiently without recreating the wheel.

Trying to combine

• https://stackoverflow.com/questions/15976570/is-there-a-grep-equivalent-for-finds-print0-and-xargss-0-switches
• https://real-world-systems.com/docs/tr.1.html

In the goal of being able to remove cntrl characters from each line returned by find, so that the output that is to be saved in file is used for both comparison and guideline for user to investigate what is going on.

@3hhh @JonathonHall-Purism : if we could find hack to have tr used to do this or tools provided by busybox, that would be better.

To be honest @JonathonHall-Purism : I haven't tested your script to sanitize but from past experience, the performance efficiency

@JonathonHall-Purism Unfortunately there were issues opened in the past to limit /boot deepness for performance reasons in certain scenarios and going with something not efficient will bite us back quick.

[tlaurion]

Ohhhhh! Maybe https://stackoverflow.com/questions/14680100/removing-control-characters-from-a-file

Let's see...

[tlaurion]

@3hhh @JonathonHall-Purism

find ./ ! -path './kexec*' -print0| tr -d '\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037\177' | tr "\n" "\0"| xargs -0 -n1

exerpt:

./dir with new\nline
./dir with new\nline/newline
./dir with new\nline/file
./dir with new\nline/newfile

We have something better then this?
Basically removing everything but null with tr cntrl from https://real-world-systems.com/docs/tr.1.html

[tlaurion]

Initial install - new file .auditing-0

I did a clean setup from scratch in qemu (with PureOS but I don't think the distro matters). Everything went smoothly until it was about to boot - somehow a file .auditing-0 was created between signing the contents /boot and actually booting, so the boot failed due to a new file. I'm not sure what this file is or what created it, but I re-ran the whole process and it was definitely created by something in Heads at that point (it was not there just after OS install, or even after the OEM reset and TOTP/HOTP reset).

@tlaurion Any idea what creates /boot/.auditing-0? I could not find an obvious reference and searching came up empty so far. Maybe GPG or something?

@JonathonHall-Purism : actually, this is a bug.
Depending of when tpm operations are done, sometime currentdir is /boot and sometimes its /
So when tpm operations are done when currdir is /boot, that file is created.

That should be another issue which solution is to make sure that each time we do write operations is under a subshell.
As said previously, in my tests, the only time that file was created was when sealing a TPM disk unlock key. Will have to dig code deeper to understand why this case is creating .auditing-0 under /boot.

[tlaurion]

@JonathonHall-Purism @3hhh

With the following patch on top of 0006f58:

user@heads-tests:~/heads$ git diff  3hhh/add-files 
diff --git a/initrd/bin/gui-init b/initrd/bin/gui-init
index 1d1c35e1..e44ae931 100755
--- a/initrd/bin/gui-init
+++ b/initrd/bin/gui-init
@@ -115,6 +115,7 @@ verify_global_hashes()
         less /tmp/hash_output_mismatches
         #move outdated hash mismatch list
         mv /tmp/hash_output_mismatches /tmp/hash_output_mismatch_old
+        warn "List of modified files moved to /tmp/hash_output_mismatch_old for further review from recovery shell."
         TEXT="Would you like to update your checksums now?"
       else
         TEXT="The following files failed the verification process:\n\n${CHANGED_FILES}\n\nThis could indicate a compromise!\n\nWould you like to update your checksums now?"
diff --git a/initrd/etc/functions b/initrd/etc/functions
index 0f88f797..61c6f90a 100755
--- a/initrd/etc/functions
+++ b/initrd/etc/functions
@@ -338,8 +338,8 @@ update_checksums()
 }
 
 print_tree() {
-       #use \x0 as long as possible to avoid issues with newlines in file names
-       find ./ ! -path './kexec*' -print0 | sort -z | xargs -0 printf "%s\n"
+       # tr is used here with cntrl charset minus NULL character
+       find ./ ! -path './kexec*' -print0 | tr -d '\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037\177' | sort -z | xargs -0 printf "%s\n"
 }
 
 verify_checksums()

Whiptail output still problematic:

---

@JonathonHall-Purism : By bypassing whiptail output completely with quick hack (right now, if number of new/modified files lower then 10 we show file report under whiptail)

user@heads-tests:~/heads$ git diff  3hhh/add-files 
diff --git a/initrd/bin/gui-init b/initrd/bin/gui-init
index 1d1c35e1..fc814cd6 100755
--- a/initrd/bin/gui-init
+++ b/initrd/bin/gui-init
@@ -106,7 +106,7 @@ verify_global_hashes()
       fi
 
     else
-      if [ $CHANGED_FILES_COUNT -gt 10 ]; then
+      if [ $CHANGED_FILES_COUNT -gt 0 ]; then
         # drop to console to show full file list
         whiptail $ERROR_BG_COLOR --title 'ERROR: Boot Hash Mismatch' \
           --msgbox "${CHANGED_FILES_COUNT} files failed the verification process!\\n\nThis could indicate a compromise!\n\nHit OK to review the list of files.\n\nType \"q\" to exit the list and return." 16 60
@@ -115,6 +115,7 @@ verify_global_hashes()
         less /tmp/hash_output_mismatches
         #move outdated hash mismatch list
         mv /tmp/hash_output_mismatches /tmp/hash_output_mismatch_old
+        warn "List of modified files moved to /tmp/hash_output_mismatch_old for further review from recovery shell."
         TEXT="Would you like to update your checksums now?"
       else
         TEXT="The following files failed the verification process:\n\n${CHANGED_FILES}\n\nThis could indicate a compromise!\n\nWould you like to update your checksums now?"
diff --git a/initrd/etc/functions b/initrd/etc/functions
index 0f88f797..61c6f90a 100755
--- a/initrd/etc/functions
+++ b/initrd/etc/functions
@@ -338,8 +338,8 @@ update_checksums()
 }
 
 print_tree() {
-       #use \x0 as long as possible to avoid issues with newlines in file names
-       find ./ ! -path './kexec*' -print0 | sort -z | xargs -0 printf "%s\n"
+       # tr is used here with cntrl charset minus NULL character
+       find ./ ! -path './kexec*' -print0 | tr -d '\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037\177' | sort -z | xargs -0 printf "%s\n"
 }
 
 verify_checksums()

We make sure that the file reports would always be through less:

That would require a little additional gui-init work to make this hack more long term, but I think this is going where we want this to be.

@3hhh @JonathonHall-Purism ?

[tlaurion]

@JonathonHall-Purism @3hhh : another thing to keep in mind for UX.
As current PR, users might be confused when they will upgrade firmware, since the same generic message saying that hash files are missing is displayed to them even if what we mean is that dir tree is missing.
Currently, users do not expect their hashes to be invalidated per firmware upgrades.

Any suggestion how to deal with this better as a transitional measure to not scare users upon firmware upgrade?
This happens here https://github.com/osresearch/heads/pull/1262/files#diff-d0832bfa8bcbd1128aa957fad283dcdc4ae9c3c4bcac03791a4f5653d121c126R75

[tlaurion]

At the end of the day, finding back those files with invalid names will fail (will current patch proposed under #1262 (comment) please use and adapt as desired)

The user will probably want to delete those files from recovery shell/OS prior of generating new hashes/dir tree and sign.

I have no solution to propose for this nor have a use case in mind where such filenames are normal to be there.
Also note that most users do not know that they would have to mount -o remount,rw /boot to remove files and then mount -o remount,ro /boot to sync those changes under Heads as of today.

Anyway, trying to create digests for those awkward files fails:

[3hhh]

I haven't tested your script to sanitize but from past experience, the performance efficiency

I'm a bit in a hurry today, but let's please judge it based on some performance comparison with the alternative. I personally doubt that xxd | tr | read is significantly worse wrt performance than just tr.

And if it's somewhat OK performance wise, I agree with @JonathonHall-Purism that it's better to not remove the characters and just escape them.

[3hhh]

Hm OK the relative performance difference is quite significant. Whether the absolute difference of ~180ms on my debian box is relevant in practice I'll let for you guys to judge:

time busybox tr -d '\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037\177' < test.txt > out.txt

real    0m0.004s
user    0m0.001s
sys     0m0.003s

time ./escape.sh < test.txt > out.txt

real    0m0.184s
user    0m0.096s
sys     0m0.089s

(test.txt was a 13k /boot/ tree.)

Personally I don't care waiting ~200ms more.

@JonathonHall-Purism @3hhh : another thing to keep in mind for UX. As current PR, users might be confused when they will upgrade firmware, since the same generic message saying that hash files are missing is displayed to them even if what we mean is that dir tree is missing. Currently, users do not expect their hashes to be invalidated per firmware upgrades.

Any suggestion how to deal with this better as a transitional measure to not scare users upon firmware upgrade? This happens here https://github.com/osresearch/heads/pull/1262/files#diff-d0832bfa8bcbd1128aa957fad283dcdc4ae9c3c4bcac03791a4f5653d121c126R75

In general adjusting the error message and also mentioning the tree file should help. Moreover one could temporarily (for 1-2 versions) adjust the error message and say that the update request could also be caused by a firmware upgrade from a version prior to XYZ. But yes, at least mention it in the changelog.

[tlaurion]

OK, slept on it.

print_tree is used for both generation of dir tree and showing the information to user. I think the problem lies there.

We should warn the user that X files/directories contain non-printable characters and show them in less, while keeping dir tree saved with those invalid characters (minus newlines which interfere with output in on otherwise multiple lines).

The point discussed with @JonathonHall-Purism is that no file containing invalid characters should be present under boot, so user should know.

Will take another shot at this next week. I agree sanitization for output to screen is needed.

Will also review https://source.puri.sm/firmware/pureboot/-/commit/4e83fa9c3ee96768af5c47300495fac22659fccc and two prior commits

[tlaurion]

And the more I think about it, the more dir_tree might be irrelevant altogether.

@3hhh is there any reason why files only (not reporting on new/deleted empty directories outside of directories containing new/deleted files per sha256sum) would not be enough?

[3hhh]

We should warn the user that X files/directories contain non-printable characters and show them in less, while keeping dir tree saved with those invalid characters (minus newlines which interfere with output in on otherwise multiple lines).

Showing it in less without sanitization is also bad as your trust on the less implementation, see echo -e 'some evil stuff\b\b\b\b\b\b\b\b\b\bgood stuff' | busybox less (busybox vi gets it right btw).

So yes, sanitization is needed anyway.

@3hhh is there any reason why files only (not reporting on new/deleted empty directories outside of directories containing new/deleted files per sha256sum) would not be enough?

Any initrd developer could do something like if [ -d /boot/somepath ] ; then do_something ; else do_something_else ; fi. So an attacker could modify the boot code path by just creating a seemingly empty directory. Of course configuration files are more likely.

[3hhh]

The current version should fix most of the aforementioned issues.

[tlaurion]

Test upon properly detached signed digests (Options->Generate hashes + sign)

mount -o remount,rw /boot
cd /boot
mkdir "$(echo -e 'hidden dir\b\b\b\b\b\b\b\b\b\bshowed dir')"
mkdir "$(echo -e 'dir with newline\ncharacter')"
mkdir "directory with textual newl\nline"
cd *hidden*
touch "$(echo -e 'file with newline\ncharacter')"
touch "file with textual newl\nline"
mount -o remount,ro /boot
reboot

Options -> show OS boot options

Let's say we updating checksums + sign at that prompt

If we are ok with the idea that user should review files with weird characters and not sign, then I have nothing against merging this as this is to me a big improvement. @JonathonHall-Purism ?

[3hhh]

Interesting. Looks like sha256sum has issues with certain characters. Anyway that's probably not a new issue caused by this PR...

[tlaurion]

Upon generated hashes + sign of digests:

mount -o remount,rw /boot
cd /boot
mkdir "$(echo -e 'hidden dir\b\b\b\b\b\b\b\b\b\bshowed dir')"
cd *hidden*
touch "$(echo -e 'file with newline\ncharacter')"
touch "file with textual newl\nline"
mount -o remount,ro /boot
reboot

generate checkums, sign
recovery shell:

cd /boot
sha256sum -c kexec_hashes.txt

Exerpt

./showed dir/file with textual newl\nline: OK
sha256sum: can't open './showed dir/file with newline': No such file or directory
./showed dir/file with newline: FAILED

Even though checksums were created for invalid paths and files:

/boot # cat kexec_hashes.txt |grep hidden
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  ./showed dir/file with textual newl\nline
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  ./showed dir/file with newline

Even though kexec_dirtree.txt was created with escaped names:
./showed dir./hiddenshowed dir/file with newline

Again:

/boot # find ./ ! -path './kexec*' -print0 | tr -d '\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037\177' | sort -z | xargs -0 printf "%s\n"

Exerpt:

./hidden dirshowed dir
./hidden dirshowed dir/file with newlinecharacter
./hidden dirshowed dir/file with textual newl\nline

So my question here is: should we report on invalid folder names/filenames?

---

Solution space:
If we decide to seperate (per previous discussion points here) what we store (kexec_tree.txt) vs what we show on screen, here is a thing we should do to prepare the user for action in what we show to him:

/boot # find ./ ! -path './kexec*' -print0 | tr '\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037\177' "#"| sort -z | xargs -0 printf "%s\n" | grep "#" |wc -l

Outputs: 4
EDIT: 4 files telling us we should not go forward and drop to recovery shell at that point
tr call above is not deleting output (-d), but replacing charset found in string with #:

/boot # find ./ ! -path './kexec*' -print0 | tr '\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037\177' "#"| sort -z | xargs -0 printf "%s\n" | grep "#"
./dir with newline#character
./hidden dir##########showed dir
./hidden dir##########showed dir/file with newline#character
./hidden dir##########showed dir/file with textual newl\nline

We could squash multiple instances of the same replaced char with -s, but I think that would defeat the purpose:

/boot # find ./ ! -path './kexec*' -print0 | tr -s '\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037\177' "#"| sort -z | xargs -0 printf "%s\n" | grep "#"
./dir with newline#character
./hidden dir#showed dir
./hidden dir#showed dir/file with newline#character
./hidden dir#showed dir/file with textual newl\nline

Where to be able to resolve the issue with sha256sum -c/sha256sum and tree output to be kept under kexec_tree.txt, we seem to need to tr -d (delete) newlines for them to be wrapped/escaped correctly (correctly here means being parsable with other tools):

/boot # find ./ ! -path './kexec*' -print0 | tr -d '\012' | sort -z | xargs -0 p
rintf "%s\n"| grep showed
./showed dir
./showed dir/file with newlinecharacter
./showed dir/file with textual newl\nline

---

But as you can see above there is not such a thing as saving "raw" directory tree with bad dir/filenames unless we sanitize them, and if we do, the paths saved into kexec_tree.txt/kexec_hashes.txt won't exist and will fail later on at verification.

IE:
find ./ -type f ! -path './kexec*' -print0 | xargs -0 sha256sum > /tmp/test
does the right thing

where:

sha256sum -c /tmp/test | grep -v 'OK' 
sha256sum: can't open './showed dir/file with newline': No such file or directory
./showed dir/file with newline: FAILED
sha256sum: WARNING: 2 of 340 computed checksums did NOT match

So we either create an issue out of this reply to deal later or try to come out with a solution here prior of merge?
@3hhh @JonathonHall-Purism

[3hhh]

It's a bug in busybox sha256sum that should be reported upstream IMHO:

user@disp2426:~$ l file*
-rw-r--r-- 1 user user 0 Jan 11 07:55 'file with \nbackslash'
-rw-r--r-- 1 user user 0 Jan 11 07:50 'file with'$'\n''newline'

user@disp2426:~$ busybox sha256sum file*
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  file with \nbackslash
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  file with
newline

user@disp2426:~$ sha256sum file*
\e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  file with \\nbackslash
\e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  file with\nnewline

[3hhh]

@tlaurion Btw with the last version, no escaped output is stored in kexec_tree.txt, just zero-delimited "raw" find output. The zero character is the only one not allowed in file names IIRC. So that should be safe. However kexec_tree.txt has nothing to do with sha256sum anyway.

[JonathonHall-Purism]

@tlaurion @3hhh Thanks for letting me catch up with this. The improvements are great and I think it is close.

I did a fair amount of testing this morning and we still have a few things that I think were discussed partially but lost in the shuffle.

I think the upgrade path is fine - the message presented is pretty good (specifically says "...if you are upgrading..."). While we could detect an old signature lacking kexec_tree.txt I don't think it's necessary, and I think we should have the general expectation that updating Heads/PB may require re-signing this way, in general adding compatibilities has a significant risk of reopening holes IMO.

.auditing-0 still causes a problem in clean setup

First sign + boot on a clean setup still fails to boot due to creating /boot/.auditing-0. Though this PR didn't introduce .auditing-0 I still think it blocks because this is a regression, first boot no longer works, have to fall back and sign a second time.

• Maybe we should fix this in the tool? I'm not sure why the file is created at all, maybe it should be created somewhere else or not at all. Fixing everywhere we invoke it to avoid working in /boot seems very error-prone.
• Maybe we have to hack around this file in kexec_tree for now, just exclude it? I don't love it but it is likely reasonable to assume that initrds aren't looking for /boot/.auditing-0.

Hash-escaping should be a bit cleaner

I think we can simplify the hash-escaping a bit to make the display more readable. Commented inline on the file

Future - is there unneeded redundancy between kexec_tree and kexec_hashes?

The only reason for kexec_tree.txt to exist at this point is to control empty directories (which I agree with) - we could control extra files by using kexec_hashes. But given that, why duplicate the list of files from kexec_hashes?

I think the current approach is fine for the first version, this is a thought about future iterations.

[JonathonHall-Purism]

Since this is no longer trying to create shell-safe output (which is fine, makes sense for whiptail), I think we can allow more characters through to make the output more readable.

I think we'd be fine to escape 00-1f (controls), # (escape char itself), \ (interpreted by whiptail), 7f (del). Continuing to use #n#r#t#v#b## for those chars is good IMO, hex for everything else.

In particular leaving space alone would make files with spaces more readable. I don't think escaping other shell chars like "'(){}[], etc. has any benefit.

I think # was a good choice for the escape char. It's unfortunate that we have to invent another kind of escaping and hope that the user understands it, but # is odd enough not just in file names but any sort of terminal output that I think it's OK. I liked what less did with other control chars (inverted colors, some unusual character) but your point about \b is good, I hadn't checked that one specifically, better safe than sorry.

[tlaurion]

Updated upstream related ticket at https://bugs.busybox.net/show_bug.cgi?id=14226 for sha256sum (and other busybox applets) bad behavior

[tlaurion]

Per reply at #1262 (comment)

Solution space: If we decide to seperate (per previous discussion points here) what we store (kexec_tree.txt) vs what we show on screen, here is a thing we should do to prepare the user for action in what we show to him:

/boot # find ./ ! -path './kexec*' -print0 | tr '\001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037\177' "#"| sort -z | xargs -0 printf "%s\n" | grep "#" |wc -l

Outputs: 4 EDIT: 4 files telling us we should not go forward and drop to recovery shell at that point

@3hhh @JonathonHall-Purism this is on what I want to direct your attention.
Agreed that on master as of now, we do not prevent this. But adding the code in current PR, some people will complain that generation/verification will be longer (some already suggested to limit depth of find :/)

But by introducing new code that introduce creation of a dir tree, we should, I think, at the same time, prevent the user of creating invalid checksums/signing of files that would fail verification on next boot.

Thoughts on that point?

---

Otherwise @JonathonHall-Purism #1272 fixes your concerns about /boot/.auditing-0 being created under /boot.

[JonathonHall-Purism]

@3hhh - @tlaurion and I discussed this since we were going in circles:

Since signing a /boot containing control chars doesn't work (and is unlikely to work in any near future), it doesn't make sense to try to handle things like this in kexec_tree, the prompts, etc. Instead we should prevent the user from signing, because it won't work.

So we suggest:

1. when validating /boot checksums/tree, first check if there are any control chars rendering /boot unsignable
   • something like this: find ./ -print0 | tr '\001-\037\134\177\200-\377' '#' | sort -z | xargs -0 printf "%s\n" | grep '#' | wc -l
   • note I removed the kexec_ filter because kexec_ files can't have control chars either, and I added \200-\377 because UTF-8 has all sorts of chars, maybe that should be discussed?
   • also added backslash (\134) due to whiptail interpreting it
2. If /boot is unsignable, prompt user:
   • X files in contain undisplayable characters and can't be signed
   • offer to drop to recovery to examine (tell user where the list is with #-substituted names for review) or to override and boot
3. Then kexec_tree doesn't have to do anything regarding control chars, backslashes, etc.

While Heads already cannot sign files like this, we would like these changes in order to merge this PR because the alternative escaping strategy leaves the user stuck in a flow where they attempt to sign, then the signatures are invalid and they're prompted to sign again, etc. I think the performance is a problem, 0.2 s doesn't seem like much but by my math it could take 25 min if an attacker created 100 MB of file names.

Thanks again for all the work on this and hashing this out with us. @tlaurion Please comment if I overlooked any detail from our discussion.

[tlaurion]

Found errors. Fixed in master

[tlaurion]

@3hhh : sorry for past noise.

If you rebase this PR on master, I changed 2 things in the following patch provided on top of master (or tlaurion@09ae3d9 directly after rebasing this PR on master)

• assert_signable is called prior of confirm_gpg_card under kexec-sign-config (no point losing time loading drivers and asking user to confirm GPG card is plugged in if we might not need it because digests would not be signable)
• your call to die is replaced by a call to recovery, proposing investigation trail directly on console prior of jumping under heads recovery shell under assert_signable. This would be useful for any investagation where users might want to reinstall and restore backups if that happens, IMOH.

With a rebase on master and the following changes, I think this PR is ready to merge.
Please ping JonathonHall-Purism when done for final review!

diff --git a/initrd/bin/gui-init b/initrd/bin/gui-init
index e3555cb6..74e30dba 100755
--- a/initrd/bin/gui-init
+++ b/initrd/bin/gui-init
@@ -66,14 +66,15 @@ verify_global_hashes()
   # Check the hashes of all the files, ignoring signatures for now
   check_config /boot force
   TMP_HASH_FILE="/tmp/kexec/kexec_hashes.txt"
+  TMP_TREE_FILE="/tmp/kexec/kexec_tree.txt"
   TMP_PACKAGE_TRIGGER_PRE="/tmp/kexec/kexec_package_trigger_pre.txt"
   TMP_PACKAGE_TRIGGER_POST="/tmp/kexec/kexec_package_trigger_post.txt"
 
-  if ( cd /boot && sha256sum -c "$TMP_HASH_FILE" > /tmp/hash_output ) then
+  if verify_checksums /boot ; then
     return 0
-  elif [ ! -f $TMP_HASH_FILE ]; then
-    if (whiptail $BG_COLOR_ERROR --title 'ERROR: Missing Hash File!' \
-        --yesno "The file containing hashes for /boot is missing!\n\nIf you are setting this system up for the first time, select Yes to update\nyour list of checksums.\n\nOtherwise this could indicate a compromise and you should select No to\nreturn to the main menu.\n\nWould you like to update your checksums now?" 0 80) then
+  elif [[ ! -f "$TMP_HASH_FILE" || ! -f "$TMP_TREE_FILE" ]] ; then
+    if (whiptail $BG_COLOR_ERROR --title 'ERROR: Missing File!' \
+        --yesno "One of the files containing integrity information for /boot is missing!\n\nIf you are setting up heads for the first time or upgrading from an\nolder version, select Yes to create the missing files.\n\nOtherwise this could indicate a compromise and you should select No to\nreturn to the main menu.\n\nWould you like to create the missing files now?" 0 80) then
       if update_checksums ; then
         BG_COLOR_MAIN_MENU=""
         return 0;
diff --git a/initrd/bin/kexec-select-boot b/initrd/bin/kexec-select-boot
index ceb0c5e9..44fee308 100755
--- a/initrd/bin/kexec-select-boot
+++ b/initrd/bin/kexec-select-boot
@@ -52,7 +52,7 @@ verify_global_hashes()
 {
 	echo "+++ Checking verified boot hash file "
 	# Check the hashes of all the files
-	if ( cd $bootdir && sha256sum -c "$TMP_HASH_FILE" > /tmp/hash_output ); then
+	if verify_checksums "$bootdir" "$gui_menu" ; then
 		echo "+++ Verified boot hashes "
 		valid_hash='y'
 		valid_global_hash='y'
@@ -326,6 +326,7 @@ while true; do
 	TMP_DEFAULT_FILE=`find /tmp/kexec/kexec_default.*.txt 2>/dev/null | head -1` || true
 	TMP_MENU_FILE="/tmp/kexec/kexec_menu.txt"
 	TMP_HASH_FILE="/tmp/kexec/kexec_hashes.txt"
+	TMP_TREE_FILE="/tmp/kexec/kexec_tree.txt"
 	TMP_DEFAULT_HASH_FILE="/tmp/kexec/kexec_default_hashes.txt"
 	TMP_ROLLBACK_FILE="/tmp/kexec/kexec_rollback.txt"
 	TMP_KEY_DEVICES="/tmp/kexec/kexec_key_devices.txt"
@@ -385,4 +386,4 @@ while true; do
 	fi
 done
 
-die "!!! Shouldn't get here""
+die "!!! Shouldn't get here"
diff --git a/initrd/bin/kexec-sign-config b/initrd/bin/kexec-sign-config
index cb69ef52..88a74c14 100755
--- a/initrd/bin/kexec-sign-config
+++ b/initrd/bin/kexec-sign-config
@@ -21,18 +21,24 @@ fi
 
 paramsdir="${paramsdir%%/}"
 
+assert_signable
+
 confirm_gpg_card
 
 # update hashes in /boot before signing
 if [ "$update" = "y" ]; then
 	(
 		cd /boot
-		find ./ -type f ! -name '*kexec*' -print0 | xargs -0 sha256sum > /boot/kexec_hashes.txt
+		find ./ -type f ! -path './kexec*' -print0 | xargs -0 sha256sum > /boot/kexec_hashes.txt
 		if [ -e /boot/kexec_default_hashes.txt ]; then
 			DEFAULT_FILES=$(cat /boot/kexec_default_hashes.txt | cut -f3 -d ' ')
 			echo $DEFAULT_FILES | xargs sha256sum > /boot/kexec_default_hashes.txt
 		fi
+
+		#also save the file & directory structure to detect added files
+		print_tree > /boot/kexec_tree.txt
 	)
+	[ $? -eq 0 ] || die "$paramsdir: Failed to update hashes."
 
 	# Remove any package trigger log files
 	# We don't need them after the user decides to sign
diff --git a/initrd/bin/oem-factory-reset b/initrd/bin/oem-factory-reset
index 89459f6d..5d18b207 100755
--- a/initrd/bin/oem-factory-reset
+++ b/initrd/bin/oem-factory-reset
@@ -195,9 +195,14 @@ generate_checksums()
     fi
 
     # generate hashes
-    find /boot -type f ! -name '*kexec*' -print0 \
-        | xargs -0 sha256sum > /boot/kexec_hashes.txt 2>/dev/null \
-        || whiptail_error_die "Error generating kexec hashes"
+    (
+        set -e -o pipefail
+        cd /boot
+        find ./ -type f ! -path './kexec*' -print0 \
+            | xargs -0 sha256sum > /boot/kexec_hashes.txt 2>/dev/null
+        print_tree > /boot/kexec_tree.txt
+    )
+    [ $? -eq 0 ] || whiptail_error_die "Error generating kexec hashes"
 
     param_files=`find /boot/kexec*.txt`
     [ -z "$param_files" ] \
@@ -553,6 +558,8 @@ if ! gpg --card-status >/dev/null 2>&1 ; then
     fi
 fi
 
+assert_signable
+
 # Action time...
 
 # detect and set /boot device
diff --git a/initrd/etc/functions b/initrd/etc/functions
index af787213..50fc3fa7 100755
--- a/initrd/etc/functions
+++ b/initrd/etc/functions
@@ -335,6 +335,118 @@ update_checksums()
 	return $rv
 }
 
+print_tree() {
+	find ./ ! -path './kexec*' -print0 | sort -z
+}
+
+# Escape zero-delimited standard input for use in shell.
+# These characters are passed verbatim: a-zA-Z0-9,._+:@%/-
+# These escapes are used to replace their corresponding characters: #n#r#t# #v#b
+# Other characters are rendered as hexadecimal escapes
+# escape_zero [prefix] [escape character]
+# prefix: \0 in the input will result in \n[prefix]
+# escape character: character to use for escapes (default: #); \ may be interpreted by `whiptail`
+escape_zero() {
+	local prefix="$1"
+	local echar="${2:-#}"
+	local todo=""
+
+	echo -e -n "$prefix"
+	xxd -p | tr -d '\n' |
+	{
+		while IFS= read -r -n2 -d '' ; do
+			if [ -n "$todo" ] ; then
+				#REPLY == "  " is EOF
+				[[ "$REPLY" == "  " ]] && echo '' || echo -e -n "$todo"
+				todo=""
+			fi
+
+			case "$REPLY" in
+				00)
+					todo="\n$prefix"
+					;;
+				08)
+					echo -n "${echar}b"
+					;;
+				09)
+					echo -n "${echar}t"
+					;;
+				0a)
+					echo -n "${echar}n"
+					;;
+				0b)
+					echo -n "${echar}v"
+					;;
+				0d)
+					echo -n "${echar}r"
+					;;
+
+				20)
+					echo -n "${echar} "
+					;;
+				#%+,-./ 0-9:    @A-0      P-Z_     a-o    p-z
+				2[5b-f]|3[0-9a]|4[0-9a-f]|5[0-9af]|6[1-f]|7[0-9a])
+					echo -e -n '\x'"$REPLY"
+					;;
+				# All others are escaped
+				[0-9a-f][0-9a-f])
+					echo -n "${echar}x$REPLY"
+					;;
+			esac
+		done
+	}
+}
+
+# Currently heads doesn't support signing file names with certain characters
+# due to https://bugs.busybox.net/show_bug.cgi?id=14226. Also, certain characters
+# may be intepreted by `whiptail`, `less` et al (e.g. \n, \b, ...).
+assert_signable() {
+	# ensure /boot mounted
+	if ! grep -q /boot /proc/mounts ; then
+		mount -o ro /boot || die "Unable to mount /boot"
+	fi
+
+	find /boot -print0 > /tmp/signable.ref
+	local del='\001-\037\134\177-\377'
+	LC_ALL=C tr -d "$del" < /tmp/signable.ref > /tmp/signable.del || die "Failed to execute tr."
+	if ! cmp -s "/tmp/signable.ref" "/tmp/signable.del" &> /dev/null ; then
+		cd /boot
+		recovery "Some /boot file names contain characters that are currently not supported by heads: $del"$'\n'"Please investigate the following relative paths to /boot (where # are sanitized invalid characters):"$'\n'"$(cat /tmp/hash_output_mismatches)"
+	fi
+	rm -f /tmp/signable.*
+}
+
+verify_checksums()
+{
+	local boot_dir="$1"
+	local gui="${2:-y}"
+
+	(
+		set +e -o pipefail
+		local ret=0
+		cd "$boot_dir" || ret=1
+		sha256sum -c "$TMP_HASH_FILE" > /tmp/hash_output || ret=1
+
+		# also make sure that the file & directory structure didn't change
+		# (sha256sum won't detect added files)
+		print_tree > /tmp/tree_output || ret=1
+		if ! cmp -s "$TMP_TREE_FILE" /tmp/tree_output &> /dev/null ; then
+			ret=1
+			[[ "$gui" != "y" ]] && exit "$ret"
+			# produce a diff that can safely be presented to the user
+			# this is relatively hard as file names may e.g. contain backslashes etc.,
+			# which are interpreted by whiptail, less, ...
+			escape_zero "(new) " < "$TMP_TREE_FILE" > "${TMP_TREE_FILE}.user"
+			escape_zero "(new) " < /tmp/tree_output > /tmp/tree_output.user
+			diff "${TMP_TREE_FILE}.user" /tmp/tree_output.user | grep -E '^\+\(new\).*$' | sed -r 's/^\+\(new\)/(new)/g' >> /tmp/hash_output
+			rm -f "${TMP_TREE_FILE}.user"
+			rm -f /tmp/tree_output.user
+		fi
+		exit $ret
+	)
+	return $?
+}
+
 # detect and set /boot device
 # mount /boot if successful
 detect_boot_device() 

(this is also under https://github.com/tlaurion/heads/tree/3hhh-add-files_WiP: tlaurion@09ae3d9)

Edit: This would give the following hints to the user to be acted upon when landing on the recovery shell, already under /boot dir in case of undesirable files/directories being present under /boot prior of signing:

[3hhh]

It should be ready to go then. ping @JonathonHall-Purism

[tlaurion]

@JonathonHall-Purism : I retested and LGTM!

[tlaurion]

The above happening on recovery console when a user decides to proceed signing invalid files/dir only (with now extended charset escaped under d07df1e).

[JonathonHall-Purism]

Works perfectly! Awesome work @3hhh and @tlaurion ; let's merge it 👍