Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function "reset_private_image" in qvm_template_postprocess fails with "not enough data" #5946

Closed
WillyPillow opened this issue Jul 11, 2020 · 8 comments · Fixed by QubesOS/qubes-core-admin-client#149 or QubesOS/qubes-core-admin#359
Labels
C: core diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. pr submitted A pull request has been submitted for this issue. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Milestone

Comments

@WillyPillow
Copy link

Qubes OS version

R4.1

Affected component(s) or functionality

qvm-template-postprocess

Brief summary

reset_private_img raises the following error:

qubesadmin.exc.QubesException: Data import failed: not enough data (copied 0 bytes, expected 2147483648 bytes)

To Reproduce

Reinstall an already-installed template RPM.

Expected behavior

Private volume is cleared.

Actual behavior

The aforementioned error occurs.

Additional context

Ran into this issue while testing the the {reinstall,{down,up}grade} operations of qvm-template (QubesOS/qubes-core-admin-client#145).

Upon further inspection, it seems that the admin.vm.volume.Import call requires the payload stream to be of the same size as the volume, causing the error. (Commit that introduced the check: QubesOS/qubes-core-admin@e9b97e4)

Solutions you've tried

I tried replacing the import call with resizing the volume to zero then back to the old size. However, the relevant qrexec call does not seem to support the shrinking of volumes.

It seems sufficient to simply treat an input length of zero as a special case in admin.vm.volume.Import, though I am not sure if this is an ideal solution.

Relevant documentation you've consulted

Related, non-duplicate issues

None.

@WillyPillow WillyPillow added P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. labels Jul 11, 2020
@marmarek
Copy link
Member

It seems sufficient to simply treat an input length of zero as a special case in admin.vm.volume.Import, though I am not sure if this is an ideal solution.

No, this error check was introduced specifically to avoid treating truncated input (including 0) as a valid data - in most cases truncated input means something gone wrong on the sending side.

I tried replacing the import call with resizing the volume to zero then back to the old size. However, the relevant qrexec call does not seem to support the shrinking of volumes.

Yes, that should work. There is a call admin.vm.volume.ImportWithSize that should do it (qvm-volume import --size=0 ...) and then resize back to the desired size.

@marmarek
Copy link
Member

Are you trying it on R4.1 or R4.0? On R4.1 there is a ImportWithSize call that should not have this limitation.

@marmarek
Copy link
Member

marmarek commented Jul 11, 2020

Just to be clear: do not try to resize down to 0 manually. Try to import zero-sized content, using qvm-volume import --size=0 (or "import_data_with_size(..., size=0)` function if you're doing that from python).

@WillyPillow
Copy link
Author

Just to be clear: do not try to resize down to 0 manually. Try to import zero-sized content, using qvm-volume import --size=0 (or "import_data_with_size(..., size=0)` function if you're doing that from python).

Indeed, I originally attempted to do this manually. (Soon realized what you meant and removed the previous comment.) I'll try this out and create a PR later.

@WillyPillow
Copy link
Author

Calling import_data_with_size(..., size=0) yields a permission denied error.

Poked around a bit and it seemed that this was due to https://github.com/QubesOS/qubes-core-admin/blob/master/qubes/api/internal.py#L101 requiring the size to be greater than zero.

Even when that line is removed, the underlying lvcreate does not seem to support a --virtualsize of zero. (Nor sizes that are not multiples of 512.)

Using something like the following works, but feels pretty hackish.

proc = subprocess.Popen(['head', '-c', '512', '/dev/zero'], stdout=subprocess.PIPE)
old_size = vm.volumes['private'].size
vm.volumes['private'].import_data_with_size(stream=proc.stdout, size=512)
vm.volumes['private'].resize(old_size)
proc.wait()

@marmarek
Copy link
Member

Hmm, in this case perhaps the best way is to introduce new call like admin.vm.volume.Erase (or .Clear)?
I can think of many other hackish solutions (like sending the full volume size of /dev/zero) but that doesn't sound nice either.

WillyPillow added a commit to WillyPillow/qubes-core-admin-client that referenced this issue Jul 12, 2020
@andrewdavidwong andrewdavidwong added this to the Release 4.1 milestone Jul 12, 2020
@andrewdavidwong andrewdavidwong added diagnosed Technical diagnosis has been performed (see issue comments). pr submitted A pull request has been submitted for this issue. labels Jul 12, 2020
@brendanhoar
Copy link

  1. For lvm, shall we consider that we want to ensure blkdiscard happens when erase/clear of existing volume is invoked (anti-forensics).

  2. Similarly, if resize-to-smaller ever ends up being supported, a similar consideration for the removed portion of a volume may want to be considered.

B

@WillyPillow
Copy link
Author

  1. For lvm, shall we consider that we want to ensure blkdiscard happens when erase/clear of existing volume is invoked (anti-forensics).

My understanding from reading the source code is that this happens automatically as long as I reuse the import mechanism internally, so hopefully, not much extra work needs to be done for this.

WillyPillow added a commit to WillyPillow/qubes-core-admin that referenced this issue Jul 13, 2020
WillyPillow added a commit to WillyPillow/qubes-core-admin that referenced this issue Jul 13, 2020
WillyPillow added a commit to WillyPillow/qubes-core-admin that referenced this issue Jul 13, 2020
WillyPillow added a commit to WillyPillow/qubes-core-admin-client that referenced this issue Jul 13, 2020
marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Jul 16, 2020
* origin/pr/359:
  Add tests for vm.volume.Clear.
  Use self.dest.storage.import* wrappers instead.
  Add admin.vm.volume.Clear call (QubesOS/qubes-issues#5946)
marmarek added a commit to marmarek/qubes-core-admin-client that referenced this issue Jul 16, 2020
* origin/pr/149:
  Add admin.vm.volume.Clear call (QubesOS/qubes-issues#5946)
WillyPillow added a commit to WillyPillow/qubes-core-admin-client that referenced this issue Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: core diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. pr submitted A pull request has been submitted for this issue. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Projects
None yet
4 participants