Introduce paranoid mode for qvm-backup-restore

[rootkovska]

Extend the threat model for system backup restore to include a scenario where the user makes a backup on a fully compromised Qubes system, then restores on a clean, new system (machine). The goal is that new system doesn't get compromised.

Of course, the VMs restored will likely be compromised. BUT: 1) the user can create new clean VMs, and 2) also reasonably safely use the old VMs, without infecting the other parts of the system. This allows also to 3) gradually investigate the restored VMs for signs of malice, e.g by mounting their private.img's to the new clean VMs.

Is seems that not much work is needed to extend qvm-backup-restore/Qubes core to handle this scenario. Here's a list we came up with together with @marmarek, which we should edit in the future in case more things pop up:

1. Don't restore Dom0 dir
2. Disable option to decompression (no support for compressed backups in paranoid mode)
3. Sanitize paths and select arguments (e.g. kernel) not to start with / and not to include /..

Note: it should not be a problem that we would be able to restore only a subset of all possible backups in the paranoid mode. This is because the anticipated scenario is that a user creates a backup especially for this mode, e.g. after new fatal QSB gets released. In that case it is fine to expect the user will make sure to create a paranoid-mode-compatible backup (e.g. don't user compression).

[jpouellet]

I like this idea very much. Both from an incident-handling perspective as well for abusing the backup system for transferring VMs between machines.

---

Here's a list we came up with together with @marmarek, which we should edit in the future in case more things pop up:

I think we should also be weary of our archive format. We currently shell out to tar, which is of considerable complexity. Of recent memory, there's CVE-2016-6321. It is not difficult to imagine other such issues.

I suggest that sandboxing tar (e.g. extract in DispVM & send back over qfilecopy) may not be a bad idea.

---

Then, if we're going to be sandboxing parts of the restore process anyway, I do not see any benefit to disabling compression (of raw disk images) rather than just sandboxing decompression in a DispVM as well.

Don't get me wrong, I completely agree that compression is something to be concerned about (IMO the LZO / LZ4 vulns from a few years ago makes a good case for this), but assuming proper sandboxing the best an attacker gains from a fully compromised decompressor is control over the extracted {root,private,...}.img, which they already have full control over anyway.

Keeping compression support would mean the ability to restore existing older compressed backups instead of only those made in response to suspected compromise. I think it is reasonable to expect that some people may place more trust in older backups (possibly over concerns for adversaries with access to pre-disclosure lists), or are interested in importing the older archives in addition to the new ones for comparison & forensics purposes, and would like to do so also in "paranoid mode".

[jpouellet]

I like this idea very much. Both from an incident-handling perspective as well for abusing the backup system for transferring VMs between machines.

---

Here's a list we came up with together with @marmarek, which we should edit in the future in case more things pop up:

I think we should also be weary of our archive format. We currently shell out to tar, which is of considerable complexity. Of recent memory, there's CVE-2016-6321. It is not difficult to imagine other such issues.

I suggest that sandboxing tar (e.g. extract in DispVM & send back over qfilecopy) may not be a bad idea.

---

Then, if we're going to be sandboxing parts of the restore process anyway, I do not see any benefit to disabling compression (of raw disk images) rather than just sandboxing decompression in a DispVM as well.

Don't get me wrong, I completely agree that compression is something to be concerned about (IMO the LZO / LZ4 vulns from a few years ago makes a good case for this), but assuming proper sandboxing the best an attacker gains from a fully compromised decompressor is control over the extracted {root,private,...}.img, which they already have full control over anyway.

Keeping compression support would mean the ability to restore existing older compressed backups instead of only those made in response to suspected compromise. I think it is reasonable to expect that some people may place more trust in older backups (possibly over concerns for adversaries with access to pre-disclosure lists), or are interested in importing the older archives in addition to the new ones for comparison & forensics purposes, and would like to do so also in "paranoid mode".

[rootkovska]

I like the idea with using DispVM for image unpacking/decompression and agree that we should allow decompression in that case. Your argument about attractiveness of restoring also some older backups sounds convincing.

[rootkovska]

I like the idea with using DispVM for image unpacking/decompression and agree that we should allow decompression in that case. Your argument about attractiveness of restoring also some older backups sounds convincing.

[marmarek]

While using DispVM here sounds like a good idea from architecture POV, this suddenly introduce two issues:

• we need to choose which DispVM we want to use (based on which template etc), which gets tricky as this DispVM will have access to all the data included in the backup (including VMs based on other templates etc)
• it is no longer a trivial task, including not being suitable as a "stable update" for Qubes 3.2

So, I propose to postpone this advanced paranoid mode to Qubes 4.0 or even 4.1 (to not postpone 4.0 any longer). And go with original plan for now.

[marmarek]

While using DispVM here sounds like a good idea from architecture POV, this suddenly introduce two issues:

• we need to choose which DispVM we want to use (based on which template etc), which gets tricky as this DispVM will have access to all the data included in the backup (including VMs based on other templates etc)
• it is no longer a trivial task, including not being suitable as a "stable update" for Qubes 3.2

So, I propose to postpone this advanced paranoid mode to Qubes 4.0 or even 4.1 (to not postpone 4.0 any longer). And go with original plan for now.

[marmarek]

Ouch, if we consider loading untrusted qubes.xml (which is one of first steps in backup restore), some more work is needed than just excluding .. in paths. For example here you get code execution at qubes.xml load time...
I'll carefully review code involved in qubes.xml loading, but as you can guess I'm less and less convinced this is such a simple task in Qubes 3.2. And BTW this is one more thing that is much better in Qubes 4.0.

[marmarek]

Ouch, if we consider loading untrusted qubes.xml (which is one of first steps in backup restore), some more work is needed than just excluding .. in paths. For example here you get code execution at qubes.xml load time...
I'll carefully review code involved in qubes.xml loading, but as you can guess I'm less and less convinced this is such a simple task in Qubes 3.2. And BTW this is one more thing that is much better in Qubes 4.0.

[jpouellet]

hah, fun.

And what are your thoughts on worrying about about xml parser vulns? That's also historically a minefield.

From a language-theoretic security perspective it is desirable to avoid using formats which necessitate parsers of higher complexity than made necessary by the actual data being represented in that format. Ideally we'd only need a regular grammar, perhaps a k=v file per VM or such.

[jpouellet]

hah, fun.

And what are your thoughts on worrying about about xml parser vulns? That's also historically a minefield.

From a language-theoretic security perspective it is desirable to avoid using formats which necessitate parsers of higher complexity than made necessary by the actual data being represented in that format. Ideally we'd only need a regular grammar, perhaps a k=v file per VM or such.

[marmarek]

And what are your thoughts on worrying about about xml parser vulns? That's also historically a minefield.

We've discussed this too, but concluded that it's unrealistic to guard against this one in Qubes 3.2.
Generally qubes.xml format by design is not supposed by be coming from untrusted sources. And actually using features of advanced file formats like XML means we don't need need to write our parser for more complex structures (like the above example...). Example qubes.xml in Qubes 4.0: https://github.com/QubesOS/qubes-core-admin/blob/core3-devel/doc/example.xml

On the other hand, in Qubes 4.0, you can have management VM having power to create VM, adjust properties etc, so all the format parsing can be sandboxed outside of dom0. Besides feature discussed here, it will allow cool things like implementing VM import from almost arbitrary complex format (OVA/OVF?). Without compromising dom0/other VMs.

[marmarek]

And what are your thoughts on worrying about about xml parser vulns? That's also historically a minefield.

We've discussed this too, but concluded that it's unrealistic to guard against this one in Qubes 3.2.
Generally qubes.xml format by design is not supposed by be coming from untrusted sources. And actually using features of advanced file formats like XML means we don't need need to write our parser for more complex structures (like the above example...). Example qubes.xml in Qubes 4.0: https://github.com/QubesOS/qubes-core-admin/blob/core3-devel/doc/example.xml

On the other hand, in Qubes 4.0, you can have management VM having power to create VM, adjust properties etc, so all the format parsing can be sandboxed outside of dom0. Besides feature discussed here, it will allow cool things like implementing VM import from almost arbitrary complex format (OVA/OVF?). Without compromising dom0/other VMs.

[jpouellet]

Is the goal to nest everything into a single qubes.xml? For example the currently separate per-vm firewall.xml?

[jpouellet]

Is the goal to nest everything into a single qubes.xml? For example the currently separate per-vm firewall.xml?

[marmarek]

Not necessary everything, for example see #899 (comment)
As for firewall.xml, that was the plan but it will probably be left as separate file for now - mostly to have 4.0 earlier.

[marmarek]

Not necessary everything, for example see #899 (comment)
As for firewall.xml, that was the plan but it will probably be left as separate file for now - mostly to have 4.0 earlier.

[rootkovska]

Yes, we have decided that the implementation of paranoid backups for Qubes 3.2 should not try to sandbox XML parser. So, in 3.2 we would like to address only "semantic-level" attacks, but trust that python can handle "syntactic ones" well. In 4.0 we will use the new mgmt API to fully contain the XML parsing code within a DispVM.

[rootkovska]

Yes, we have decided that the implementation of paranoid backups for Qubes 3.2 should not try to sandbox XML parser. So, in 3.2 we would like to address only "semantic-level" attacks, but trust that python can handle "syntactic ones" well. In 4.0 we will use the new mgmt API to fully contain the XML parsing code within a DispVM.

[rootkovska]

As for the "eval vuln" you pointed out above -- AFAICT this is the "semantic-level" problem, and it's not magically solvable by switching to Mgmt API, is it? Rather it could be solved by carefully choosing which properties we would like to be restoring when running in the paranoid mode (and it's fine that we skip restoration of most of the VM attributes). And ISTM we could easily do that by having a simplified version of the restore function which ignores most of the attributes from the XML. In Qubes 4 this would be the function that would be running in DispVM and would be using MgmAPI to request VM creations in Dom0. But in 3.2 this will be just local function calls.

[rootkovska]

As for the "eval vuln" you pointed out above -- AFAICT this is the "semantic-level" problem, and it's not magically solvable by switching to Mgmt API, is it? Rather it could be solved by carefully choosing which properties we would like to be restoring when running in the paranoid mode (and it's fine that we skip restoration of most of the VM attributes). And ISTM we could easily do that by having a simplified version of the restore function which ignores most of the attributes from the XML. In Qubes 4 this would be the function that would be running in DispVM and would be using MgmAPI to request VM creations in Dom0. But in 3.2 this will be just local function calls.

[marmarek]

As for the "eval vuln" you pointed out above -- AFAICT this is the "semantic-level" problem, and it's not magically solvable by switching to Mgmt API, is it?

Yes, it's semantic one. Come from design choice that qubes.xml does not originate from untrusted source. On the other hand, Mgmt API is designed with the assumption that client side may be untrusted to some degree.

And ISTM we could easily do that by having a simplified version of the restore function which ignores most of the attributes from the XML. (...) But in 3.2 this will be just local function calls.

The restore process looks like this (simplified):

1. Extract backup-header, qubes.xml and qubes.xml.hmac
2. Validate qubes.xml using qubes.xml.hmac
3. Load qubes.xml as it were just another VM collection (QubesVMCollection object, exactly the same as used to load /var/lib/qubes/qubes.xml)
4. List VMs, check conflicts, missing templates etc
5. Extract VMs data into temporary directory
6. For each VM (selected to restore):
   1. Create VM in /var/lib/qubes/qubes.xml (for now set only basic properties like name and template).
   2. Move VM data restored in step 5 into the right place
   3. Migrate other properties
   4. Some fixups like appmenus

Changing step 3 in paranoid mode requires a lot of work, basically reimplementing qubes.xml parser. This is the part I'm reviewing for 3.2 version. But step 6.3 is much simpler to do differently in paranoid mode and it's probably the way to go.
BTW I've already checked that none of properties included in step 6.3 are absolute paths to some files. The only things that is used in path building are: VM name (I've added additional validation) and kernel version (it was already properly validated using whitelist approach). Other path-containing properties are already ignored during restore process.

[marmarek]

As for the "eval vuln" you pointed out above -- AFAICT this is the "semantic-level" problem, and it's not magically solvable by switching to Mgmt API, is it?

Yes, it's semantic one. Come from design choice that qubes.xml does not originate from untrusted source. On the other hand, Mgmt API is designed with the assumption that client side may be untrusted to some degree.

And ISTM we could easily do that by having a simplified version of the restore function which ignores most of the attributes from the XML. (...) But in 3.2 this will be just local function calls.

The restore process looks like this (simplified):

1. Extract backup-header, qubes.xml and qubes.xml.hmac
2. Validate qubes.xml using qubes.xml.hmac
3. Load qubes.xml as it were just another VM collection (QubesVMCollection object, exactly the same as used to load /var/lib/qubes/qubes.xml)
4. List VMs, check conflicts, missing templates etc
5. Extract VMs data into temporary directory
6. For each VM (selected to restore):
   1. Create VM in /var/lib/qubes/qubes.xml (for now set only basic properties like name and template).
   2. Move VM data restored in step 5 into the right place
   3. Migrate other properties
   4. Some fixups like appmenus

Changing step 3 in paranoid mode requires a lot of work, basically reimplementing qubes.xml parser. This is the part I'm reviewing for 3.2 version. But step 6.3 is much simpler to do differently in paranoid mode and it's probably the way to go.
BTW I've already checked that none of properties included in step 6.3 are absolute paths to some files. The only things that is used in path building are: VM name (I've added additional validation) and kernel version (it was already properly validated using whitelist approach). Other path-containing properties are already ignored during restore process.

[marmarek]

Should paranoid mode avoid assigning PCI devices to a VM? I've already excluded block devices (which also include disk images in arbitrary location).

[marmarek]

Should paranoid mode avoid assigning PCI devices to a VM? I've already excluded block devices (which also include disk images in arbitrary location).

[marmarek]

Similar question about firewall rules - currently those are not sanitized properly, giving code execution in sys-firewall. Two options:

• exclude firewall.xml from restore in paranoid mode
• try to verify firewall.xml and drop it if something fishy is detected
  I'm somehow in favor of the first option.

[marmarek]

Similar question about firewall rules - currently those are not sanitized properly, giving code execution in sys-firewall. Two options:

• exclude firewall.xml from restore in paranoid mode
• try to verify firewall.xml and drop it if something fishy is detected
  I'm somehow in favor of the first option.

[rootkovska]

Yes:

1. Ignore any PCI devs assignment (these are usually meanigless when restoring on a different machine anyway),
2. Ignore firewall rules -- even if not for code execution, what good are firewall rules that might have been tampered by the attacker? Perhaps a good strategy would be to apply DENY ALL for all VMs restored from such a backup, and then require the user to manually edit rules for select VMs?

[rootkovska]

Yes:

1. Ignore any PCI devs assignment (these are usually meanigless when restoring on a different machine anyway),
2. Ignore firewall rules -- even if not for code execution, what good are firewall rules that might have been tampered by the attacker? Perhaps a good strategy would be to apply DENY ALL for all VMs restored from such a backup, and then require the user to manually edit rules for select VMs?

[marmarek]

On Tue, Apr 18, 2017 at 01:06:24AM -0700, Joanna Rutkowska wrote: 2. Ignore firewall rules -- even if not for code execution, what good are firewall rules that might have been tampered by the attacker? Perhaps a good strategy would be to apply DENY ALL for all VMs restored from such a backup, and then require the user to manually edit rules for select VMs?

In such a case, better set netvm=None.

…

-- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing?

[marmarek]

On Tue, Apr 18, 2017 at 01:06:24AM -0700, Joanna Rutkowska wrote: 2. Ignore firewall rules -- even if not for code execution, what good are firewall rules that might have been tampered by the attacker? Perhaps a good strategy would be to apply DENY ALL for all VMs restored from such a backup, and then require the user to manually edit rules for select VMs?

In such a case, better set netvm=None.

…

-- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing?

[marmarek]

Additional things rejected/excluded in paranoid mode:

• old backup formats (like Qubes R1)
• appmenus (user need to regenerate them, including launching template to fetch them again)

[marmarek]

Additional things rejected/excluded in paranoid mode:

• old backup formats (like Qubes R1)
• appmenus (user need to regenerate them, including launching template to fetch them again)

[marmarek]

Implemented here: QubesOS/qubes-core-admin#99
I'll wait some time before merging it, so anyone wanting to review it or suggest something more, have a chance now.

[marmarek]

Implemented here: QubesOS/qubes-core-admin#99
I'll wait some time before merging it, so anyone wanting to review it or suggest something more, have a chance now.

[marmarek]

Force-pushed some changes there - the most important one: replace the whole qubes.xml parsing with a separate, simplified logic, carefully checking each value.

[marmarek]

Force-pushed some changes there - the most important one: replace the whole qubes.xml parsing with a separate, simplified logic, carefully checking each value.

[rootkovska]

Great, thanks!

FTR, in Qubes 4.0 we will want to use this class/functions (SafeQubesVmCollection), but we will use the MgmtAPI to actually run these functions within a DispVM (and so it will request creation of restored VMs over the policy-controlled MgtmAPI). This should provide us nearly automatically with sandboxing for the XML parser, which currently (in 3.2) must be part of the TCB, unfortunately.

[rootkovska]

Great, thanks!

FTR, in Qubes 4.0 we will want to use this class/functions (SafeQubesVmCollection), but we will use the MgmtAPI to actually run these functions within a DispVM (and so it will request creation of restored VMs over the policy-controlled MgtmAPI). This should provide us nearly automatically with sandboxing for the XML parser, which currently (in 3.2) must be part of the TCB, unfortunately.

[qubesos-bot]

Automated announcement from builder-github

The package qubes-core-dom0-3.2.14-1.fc23 has been pushed to the r3.2 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 qubes-core-dom0-3.2.14-1.fc23 has been pushed to the r3.2 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 qubes-core-dom0-3.2.14-1.fc23 has been pushed to the r3.2 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-dom0-3.2.14-1.fc23 has been pushed to the r3.2 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

[marmarek]

As noted above - implemented in Qubes 3.2. And qvm-backup-restore in Qubes 4.0 can work from Disposable VM using Admin API. Enabling "paranoid mode" there is a matter of policy for Admin API calls (for example forbidding firewall or block device settings).

[marmarek]

As noted above - implemented in Qubes 3.2. And qvm-backup-restore in Qubes 4.0 can work from Disposable VM using Admin API. Enabling "paranoid mode" there is a matter of policy for Admin API calls (for example forbidding firewall or block device settings).