Fix private vol handling during updates (QSB #50)

[tasket]

Qubes OS version
R4.0

Affected component(s) or functionality
dom0 updates i.e. qubes-dom0-update

Brief summary
The security flaw described in QSB #50 is that a template reinstall process won't erase the contents of the private volume. Assuming the template was compromised before reinstallation, this might result in compromise after reinstallation via contents in /rw.

To Reproduce
qubes-dom0-update --action=reinstall qubes-template-something-os

Expected behavior
Template's *-private volume is blanked-out, causing the template OS to reinitialize it via /usr/lib/qubes/init/setup-rw.sh on the next template startup.

Solutions you've tried
QSB 50 describes a workaround using the more convoluted instructions in the docs (see below).

As for the right way to patch: I don't know of a simple command that can be inserted into qubes-dom0-update to achieve a zeroed-out private volume. The qvm-volume command doesn't seem to support such a function (and its resize --force option appears broken), so the Linux tools blkdiscard and truncate could be used directly for their respective storage types.

A way to determine storage type would be to parse the output of qvm-volume ls to get the storage pool for the target volume, then look up the storage type using qvm-pool -l. This provides enough information to use the correct zeroing method (i.e. a volume residing in an 'lvm-thin' pool would be zeroed using blkdiscard).

Another way to resolve the issue would be to add a high-level zeroing function in qvm-volume. Unfortunately, the storage API doesn't seem to support either zeroing or independent volume removal or a resize that shrinks (in this case, would be shrink to zero size) – please correct this assertion if its not accurate. Adding a zeroing function to the storage layer, however, could be a reasonable approach.

Relevant documentation you've consulted
https://www.qubes-os.org/doc/reinstall-template/#manual-method

Related, non-duplicate issues

[rustybird]

I don't know of a simple command that can be inserted into qubes-dom0-update to achieve a zeroed-out private volume. The qvm-volume command doesn't seem to support such a function (and its resize --force option appears broken) [...]

Unfortunately, the storage API doesn't seem to support either zeroing or independent volume removal or a resize that shrinks (in this case, would be shrink to zero size)

qvm-volume resize --force vm:volume 0 generally works fine, but there's a redundant safety check in lvm_thin and in file, so in practice only file-reflink supports downsizing.

We could specify a storage API method volume.reset() that preserves the current volume data as a revision (if applicable) and then resets it. This would also fix #5177.

[andrewdavidwong]

CC: @marmarek, @HW42

[tasket]

@rustybird Currently, qvm-volume resize --force vm:private 0 says it won't comply:

For your own safety, shrinking of private is disabled (0 < 8799649792). If you really know what you are doing, use `lvresize`

Oddly enough, if you omit --force it will say the operation could be performed if you include the --force option. Repairing and using this may be problematic because:

Also, blkdiscard has recently been added to the lvm.py module to compensate for lvremove not generating discards. So the best practice in fixing this issue would be with a patch that's consistent with the blkdiscard behavior. I don't think that using a resize-shrink operation by itself would generate discards for freed blocks. (Another way to look at this is that adding discard generation to lvm.py was simple because we didn't have to take shrinking into account as its unsupported.) @brendanhoar

[hexagonrecursion]

Unfortunately, the storage API doesn't seem to support [...] zeroing

I'm not an expert is Qubes OS internals but I think it technically does:

f = tempfile.TemporaryFile()
f.truncate(volume.size())
volume.import_data(stream=f)

Note: technically whether truncate zero-fills the file contents is platform-dependent.

P.S. I think having some way to shrink a volume to 0 size or completely remove it has some advantages:

1. less IO compared to reading and writing out 2GiB or zeroes.
2. If the user extended the private volume, it would be back to the default size after reinstall (consistent with how the root volume appears to be currently handled).
3. I don't know whether lvm or btrfs are smart enough not to store entire blocks of zeroes (they probably are).

P.P.S. Could we just make the size of 0 special (ie bypass the test for shrinking)? If the user explicitly asks a volume to be shrunk to 0 size, it's pretty clear they don't care about preserving the data inside.

[rustybird]

Currently, qvm-volume resize --force vm:private 0 says it won't comply:

For your own safety, shrinking of private is disabled (0 < 8799649792). If you really know what you are doing, use `lvresize`

That's what I meant by lvm_thin having a redundant safety check. This message doesn't come from the general "storage driver agnostic" qvm-volume code, but from the specific driver's implementation of resize().

I haven't tested it, but it looks possible to just remove the LVM specific check and change lvextend to lvresize.

Oddly enough, if you omit --force it will say the operation could be performed if you include the --force option.

That safety check and its error message comes from general qvm-volume code, before any particular driver's resize() method is invoked.

Also, blkdiscard has recently been added to the lvm.py module to compensate for lvremove not generating discards. So the best practice in fixing this issue would be with a patch that's consistent with the blkdiscard behavior. I don't think that using a resize-shrink operation by itself would generate discards for freed blocks.

Oh right, it probably wouldn't.

[brendanhoar]

For the case of template reinstall, why not a post-install script that blkdiscards the template private volume if it already exists, lvremoves it then lvcreates it?

Also, blkdiscard has recently been added to the lvm.py module to compensate for lvremove not generating discards. So the best practice in fixing this issue would be with a patch that's consistent with the blkdiscard behavior. I don't think that using a resize-shrink operation by itself would generate discards for freed blocks. (Another way to look at this is that adding discard generation to lvm.py was simple because we didn't have to take shrinking into account as its unsupported.) @brendanhoar

Oh yeah. Hmm.

I'll just note that a "resize" function for lvm-thin LVs isn't as particularly critical as it is with file-based volumes in general.* That being said, blkdiscard can take lists or ranges of logical block numbers to issue discards against. So I suspect the solution (at the volume level) is pretty straightforward, other than being really really sure that the math is correct: blkdiscard [upper-range-of-blocks] then lvresize just under that upper range of blocks. Though I may be assuming how lvresize does things (so perhaps I should look).

Also, when LVM is in use, Qubes should enforce qubes-created LVs that are multiples of 1MB storage space (to ensure alignment with all possible LVM cluster sizes) and, if partitions are used in the LV, have the partitions start at 1MB and be 1MB aligned.

OTOH, the filesystem stuff is generally a nightmare, PLUS we have the Qubes context of avoiding the mounting of VM filesystems or even the parsing of VM LV partitions for security model reasons. If one wants something turnkey in dom0, one would probably need to design a process that utilizes disposable "partition and filesystem adjustment" VMs to compact the filesystems/partitions (or even just to verify they are compacted). Alternately...have the user type "ireallyknowwhatiamdoingpleasedestroymyvolume" every time they shrink a LV from dom0. :)

B

* Caveat: I do think that the linux templates and template-based AppVMs should be performing fstrim automatically on shutdown (on only the default r/w volumes for that VM), primarily for maximum storage space recovery; the currently utilized discard mount option doesn't recover as much space due to the LVM cluster-size vs. file-system block-size disparity. I look at the LV volume size primarily as a denial-of-service protection against storage resource exhaustion due to bad planning (and possibly attackers). Disposable VMs would not need the fstrim, of course.

[marmarek]

We could specify a storage API method volume.reset() that preserves the current volume data as a revision (if applicable) and then resets it. This would also fix #5177.

On Admin API level it's already possible - call admin.vm.volume.Import+private with empty or all zeroes input. So, it's just a matter of adding relevant qvm-volume sub-command. Also, you can control whether revision should be saved or not with revisions_to_keep property.
If for whatever reason (like @noseshimself case) one would like to restore old version, there is qvm-volume revert.

[qubesos-bot]

Automated announcement from builder-github

The package qubes-core-admin-client_4.0.26-1 has been pushed to the r4.1 testing repository for the Debian template.
To test this update, first enable the testing repository in /etc/apt/sources.list.d/qubes-*.list by uncommenting the line containing stretch-testing (or appropriate equivalent for your template version), then use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

[qubesos-bot]

Automated announcement from builder-github

The package core-admin-client has been pushed to the r4.1 testing repository for the CentOS centos7 template.
To test this update, please install it with the following command:

sudo yum update --enablerepo=qubes-vm-r4.1-current-testing

Changes included in this update

[qubesos-bot]

Automated announcement from builder-github

The package python2-qubesadmin-4.0.26-0.1.fc25 has been pushed to the r4.0 testing repository for dom0.
To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update

[qubesos-bot]

Automated announcement from builder-github

The package core-admin-client has been pushed to the r4.0 stable repository for the CentOS centos7 template.
To install this update, please use the standard update command:

sudo yum update

Changes included in this update

[qubesos-bot]

Automated announcement from builder-github

The package python2-qubesadmin-4.0.26-0.1.fc25 has been pushed to the r4.0 stable repository for dom0.
To install this update, please use the standard update command:

sudo qubes-dom0-update

Or update dom0 via Qubes Manager.

Changes included in this update

[qubesos-bot]

Automated announcement from builder-github

The package qubes-core-admin-client_4.0.26-1+deb9u1 has been pushed to the r4.0 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update