Polish the new memory wiping implementation and design doc

Originally created by @intrigeri on #12560 (Redmine)

… according to anonym’s review (#12354 (comment 49611)) i.e.

In wiki/src/contribute/design/memory_erasure.mdwn:

In order to protect against memory recovery such as cold boot attack,
most of the system RAM is overwritten when Tails is being shutdown or when the
boot medium is physically removed.

I guess we could add “and immediately after applications exit” or something to that effect? That is an improvement over the old kexec-based approach when treated in isolation.

#### The big picture

Can we put the story about the old design in another sub section, like “##### Obsolete kexec-based approach”, and the current one in “##### Current memory poisoning-based approach”? I.e. just insert these two headers ( and possibly adapt the edges of the affected paragraphs so the text still makes sense). I’d just like to make the history lesson more optional. :) Is it really needed for understanding the current design?

Different kinds of events trigger the memory erasure process. All lead
to run the `tails-kexec` shutdown script.

There’s no tails-kexec any more.

[[!tails_gitweb features/erase_memory.feature desc=“Automated tests”]]
ensure that the most important parts of memory are erased this way.

Can we describe “the most important parts of memory” a bit better than this (not necessarily at this place in the design page, btw)? Or do we want the .feature file to enumerate what we think is “important”? I really think this design page should list any limitations of the memory poisoning approach.

In features/step_definitions/common_steps.rb:

-def mount_USB_drive(disk, fs)
+def mount_USB_drive(disk, fs_options = {})
+ fs_options[:encrypted] ||= false

Just a FYI: nil is treated as false when interpreted as a boolean, so this assignment is unnecessary. That said, I don’t really mind making the default more explicit, for readability.

In features/step_definitions/erase_memory.rb:

kernel_mem_reserved_k = 64*1024 # Duplicated in /usr/share/initramfs-tools/scripts/init-premount/memory_wipe
[…]
admin_mem_reserved_k = 128*1024 # Duplicated in /usr/share/initramfs-tools/scripts/init-premount/memory_wipe

These don’t duplicate anything any longer.

In config/chroot_local-includes/usr/share/initramfs-tools/hooks/shutdown:

# systemd-shutdown itself
mkdir -p $DESTDIR/lib/systemd
copy_exec /lib/systemd/systemd-shutdown /shutdown

# Our shutdown hook (run from inside the initramfs)
mkdir -p $DESTDIR/lib/systemd/system-shutdown
copy_file "regular file" \
@ /usr/local/lib/initramfs-pre-shutdown-hook \@
@ /lib/systemd/system-shutdown/tails@

To me it looks like you are following the common pattern of mkdir -p:ing the target directories for the subsequent copy actions, but these directories are not used as the target (in fact they are not used at all). I’m just double-checking that these mkdir calls still are relevant, and no residues from some Git history rewrite.

In features/emergency_shutdown.feature:

Then I find very few patterns in the guest’s memory
And Tails eventually shuts down

This means that we will always wait the full two minutes (the “Happy dumping!” timeout) for each of these scenarios: there’s currently four, so that’s almost 2*4 = 8 minutes of needless waiting. I’m tempted to suggest that we add a scenario dedicated to make sure that the shutdown happens without any of the memory erasure checks, and that we simply remove the Tails eventually shuts down step from these four scenarios. In fact, I think you should move these four scenario to erase_memory.feature as well — I know it can be argued both ways, but I slightly prefer what I suggest (so no strong opinion here, just ignore this move if you disagree).

Feature Branch: bugfix/12560-polish-memory-wiping

Related issues

• Related to #12354 (closed)