Prevent creation of VMs named "none"

[rootkovska]

If such a VM exists then it is very likely for the user to misselect it when intending to create network-disconnected VM. Admittedly there is no way for attackers to create it, only the user/admin could do that.
But in a (future) systems with split admin roles, one of the mgmt VMs might create such a VM in order to trick the other mgmtVM (hence I'm adding Bug + Security labels to this ticket).

Another (better or additional?) solution would be to use $none instead of "none" to express network-not-havingness.

I think similar case might apply to "default"?

[rootkovska]

FWIW, I verified a "simple attack" with creating VM named "none", and it "works" on 4.0-rc1. Admittedly, qvm-ls properly distinguish between the cases, displaying either "-" or "none". But the GUI window for VM creation does not and user needs to take chances.

Also, qvm-create in 4.0-rc1 doesn't seem to offer a possibility to create network-disconnected VM anymore. On a system where there is no VM named "none", the following invocation doesn't work:

qvm-create somevm --label blue --property netvm=none

On a system with a fake "none" VM, the above would of course lead to undesirable effect.

We need to bring back the --offline-mode--offline switch to qvm-create.
Edited: --offline-mode in 3.2 did something else than I mean here... #SelfDocumentedCLI ;)

[rootkovska]

FWIW, I verified a "simple attack" with creating VM named "none", and it "works" on 4.0-rc1. Admittedly, qvm-ls properly distinguish between the cases, displaying either "-" or "none". But the GUI window for VM creation does not and user needs to take chances.

Also, qvm-create in 4.0-rc1 doesn't seem to offer a possibility to create network-disconnected VM anymore. On a system where there is no VM named "none", the following invocation doesn't work:

qvm-create somevm --label blue --property netvm=none

On a system with a fake "none" VM, the above would of course lead to undesirable effect.

We need to bring back the --offline-mode--offline switch to qvm-create.
Edited: --offline-mode in 3.2 did something else than I mean here... #SelfDocumentedCLI ;)

[rootkovska]

/CC @woju

[rootkovska]

/CC @woju

[marmarek]

I think forbidding certain VM names is wrong approach to the problem (should we also blacklist a hundreds of other names, like "no-vm", "offline", etc, and the same in every language on the planet?). Better would be changing VM settings GUI to have unambiguous entry for no VM set, instead of just "none" string. Maybe a checkbox before netvm dropdown?

As for CLI - the proper representation of "no VM" is empty string:

qvm-prefs somevm netvm ""
qvm-create somevm --label blue --property netvm=

[marmarek]

I think forbidding certain VM names is wrong approach to the problem (should we also blacklist a hundreds of other names, like "no-vm", "offline", etc, and the same in every language on the planet?). Better would be changing VM settings GUI to have unambiguous entry for no VM set, instead of just "none" string. Maybe a checkbox before netvm dropdown?

As for CLI - the proper representation of "no VM" is empty string:

qvm-prefs somevm netvm ""
qvm-create somevm --label blue --property netvm=

[marmarek]

We need to bring back the --offline-mode switch, as it was in R3.

Offtopic

[marmarek]

We need to bring back the --offline-mode switch, as it was in R3.

Offtopic

[rootkovska]

As for GUI: I like the solution with a checkbox ("Allow networking for this VM") and only if selected the dropdown is active, and shows only actual VMs. I think the dropdown should not show "default" -- this perhaps should be another checkbox?

[X] Allow networking [X] Use default netvm ("sys-net") [grayed dropdown]

[rootkovska]

As for GUI: I like the solution with a checkbox ("Allow networking for this VM") and only if selected the dropdown is active, and shows only actual VMs. I think the dropdown should not show "default" -- this perhaps should be another checkbox?

[X] Allow networking [X] Use default netvm ("sys-net") [grayed dropdown]

[rootkovska]

As for CLI - the proper representation of "no VM" is empty string:

qvm-prefs somevm netvm ""
qvm-create somevm --label blue --property netvm=

This is not only non-compatible with 3.x, but also non-trivial for users to figure out. Hence #3004 is important.

Anyway, how should the user (or mgmtVM) change back the netvm to default in 4.0? Of course by...:

qvm-prefs --unset myvm netvm

In case of netvm this is potentially playing with fire, because lots of users would interpret the above as: "remove netvm", so... make "myvm" offline, rather then... connect it back to the default netvm. Perhaps we want to add a switch "--set-default", which would be aliased to --unset?

[rootkovska]

As for CLI - the proper representation of "no VM" is empty string:

qvm-prefs somevm netvm ""
qvm-create somevm --label blue --property netvm=

This is not only non-compatible with 3.x, but also non-trivial for users to figure out. Hence #3004 is important.

Anyway, how should the user (or mgmtVM) change back the netvm to default in 4.0? Of course by...:

qvm-prefs --unset myvm netvm

In case of netvm this is potentially playing with fire, because lots of users would interpret the above as: "remove netvm", so... make "myvm" offline, rather then... connect it back to the default netvm. Perhaps we want to add a switch "--set-default", which would be aliased to --unset?

[marmarek]

Should this be just with other checkbox below those dropdowns? If you put label(s) there, it will not fit into a single line.
I was rather thinking about putting a checkbox just before dropdown, without any additional label:

NetVM:     [X]  [dropdown]

NetVM:     [ ]  [grayed dropdown]

And keep "default (name of vm)" entry inside - technically it is unambiguous (spaces and brackets are forbidden in VM name), but also clearly show you name of the VM set there.
Adding a label to this checkbox is IMO incompatible with keeping it in a single line. But we can add a tooltip.

[marmarek]

Should this be just with other checkbox below those dropdowns? If you put label(s) there, it will not fit into a single line.
I was rather thinking about putting a checkbox just before dropdown, without any additional label:

NetVM:     [X]  [dropdown]

NetVM:     [ ]  [grayed dropdown]

And keep "default (name of vm)" entry inside - technically it is unambiguous (spaces and brackets are forbidden in VM name), but also clearly show you name of the VM set there.
Adding a label to this checkbox is IMO incompatible with keeping it in a single line. But we can add a tooltip.

[marmarek]

Perhaps we want to add a switch "--set-default", which would be aliased to --unset?

There is already --default.

I know this breaks compatibility, but we've changing this syntax exactly to avoid the issues you brought up here initially (confusing VM named "none"/"default" with property state).

[marmarek]

Perhaps we want to add a switch "--set-default", which would be aliased to --unset?

There is already --default.

I know this breaks compatibility, but we've changing this syntax exactly to avoid the issues you brought up here initially (confusing VM named "none"/"default" with property state).

[rootkovska]

[X] Allow networking for this VM
[ ] Use default netvm ("name of the actual default netvm in quotes")
[Dropdown list active, only if "Use default" not active"

Anytime we can afford to put a human-readable sentence in GUI, instead of just some shortcut understandable only to developers who have spent the last year thinking about it, we should opt for the first option.

[rootkovska]

[X] Allow networking for this VM
[ ] Use default netvm ("name of the actual default netvm in quotes")
[Dropdown list active, only if "Use default" not active"

Anytime we can afford to put a human-readable sentence in GUI, instead of just some shortcut understandable only to developers who have spent the last year thinking about it, we should opt for the first option.

[marmarek]

This is not only non-compatible with 3.x, but also non-trivial for users to figure out. Hence #3004 is important.

I don't understand reference to #3004 here?

[marmarek]

This is not only non-compatible with 3.x, but also non-trivial for users to figure out. Hence #3004 is important.

I don't understand reference to #3004 here?

[rootkovska]

Oh, I missed the --default alias, this is fine, yes.

I wonder, however, if we should leave the --unset and --delete switches? I can't think of a situation when this is better then --default, while at the same time allows to easily make (fatal) mistakes, as demoed above.

I also still think it might be desirable to forbid creation of VM named "none" and "default", and the reason is... compatibility and prevention of users shooting themselves in their feet.

[rootkovska]

Oh, I missed the --default alias, this is fine, yes.

I wonder, however, if we should leave the --unset and --delete switches? I can't think of a situation when this is better then --default, while at the same time allows to easily make (fatal) mistakes, as demoed above.

I also still think it might be desirable to forbid creation of VM named "none" and "default", and the reason is... compatibility and prevention of users shooting themselves in their feet.

[woju]

--delete was the original name, since this is done using python's del statement and __del__ method. But I'm not attached to those, since that causes the value of default= argument to property constructor to take over.

[woju]

--delete was the original name, since this is done using python's del statement and __del__ method. But I'm not attached to those, since that causes the value of default= argument to property constructor to take over.

[rootkovska]

So, are we agreement to leave only the --default out of the three aliases?

[rootkovska]

So, are we agreement to leave only the --default out of the three aliases?

[woju]

Some time ago (maybe a year, I don't remember) @marmarek pointed out that --delete is not the best name, since you actually don't delete a property, only it's value. So if anything, I'd prefer --default.

But we are already after rc1, so I'd leave it as is. To change it now, I'd like to see an explicit decision of the Council of the Elders.

[woju]

Some time ago (maybe a year, I don't remember) @marmarek pointed out that --delete is not the best name, since you actually don't delete a property, only it's value. So if anything, I'd prefer --default.

But we are already after rc1, so I'd leave it as is. To change it now, I'd like to see an explicit decision of the Council of the Elders.

[rootkovska]

Sorry, of course I mean to leave only the --default, as per what I wrote above.

[rootkovska]

Sorry, of course I mean to leave only the --default, as per what I wrote above.

[rootkovska]

I also think it's better to remove the other aliases ASAP, as otherwise we would need to keep them forever...

[rootkovska]

I also think it's better to remove the other aliases ASAP, as otherwise we would need to keep them forever...

[marmarek]

I also still think it might be desirable to forbid creation of VM named "none" and "default", and the reason is... compatibility and prevention of users shooting themselves in their feet.

Can you provide full, exhaustive list of names that could cause confusion, ever?

I also think it's better to remove the other aliases ASAP, as otherwise we would need to keep them forever...

If anything, maybe remove --delete name. But I'd keep others, including --unset. This is for example logical for qubes-prefs --unset default_template. Or anything else that have None/empty default value.

[marmarek]

I also still think it might be desirable to forbid creation of VM named "none" and "default", and the reason is... compatibility and prevention of users shooting themselves in their feet.

Can you provide full, exhaustive list of names that could cause confusion, ever?

I also think it's better to remove the other aliases ASAP, as otherwise we would need to keep them forever...

If anything, maybe remove --delete name. But I'd keep others, including --unset. This is for example logical for qubes-prefs --unset default_template. Or anything else that have None/empty default value.

[rootkovska]

Can you provide full, exhaustive list of names that could cause confusion, ever?

Sure: "none", "default", as per backward compatibility with 3.x

If anything, maybe remove --delete name. But I'd keep others, including --unset. This is for example logical for qubes-prefs --unset default_template. Or anything else that have None/empty default value.

I think that qubes-prefs --default default_template sounds just as good, while at the same time we avoid potentially disastrous qvm-prefs --unset netvm, which could be interpreted by many users as "disable netvm", while in fact it reverts back to the default one. Let's try to stick to the principle of "least harmful surprises to casual users".

[rootkovska]

Can you provide full, exhaustive list of names that could cause confusion, ever?

Sure: "none", "default", as per backward compatibility with 3.x

If anything, maybe remove --delete name. But I'd keep others, including --unset. This is for example logical for qubes-prefs --unset default_template. Or anything else that have None/empty default value.

I think that qubes-prefs --default default_template sounds just as good, while at the same time we avoid potentially disastrous qvm-prefs --unset netvm, which could be interpreted by many users as "disable netvm", while in fact it reverts back to the default one. Let's try to stick to the principle of "least harmful surprises to casual users".

[rootkovska]

Also just take a look at #3011 which is a confirmation of my exact concerns!

[rootkovska]

Also just take a look at #3011 which is a confirmation of my exact concerns!

[qubesos-bot]

Automated announcement from builder-github

The package qubes-core-dom0-4.0.6-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 qubes-core-dom0-4.0.6-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 qubes-core-dom0-4.0.11-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-dom0-4.0.11-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