QubesOS / qubes-issues

The Qubes OS Project issue tracker
https://www.qubes-os.org/doc/issue-tracking/
539 stars 48 forks source link

Renaming `sys-net` or `sys-whonix` breaks updates without warning due to hardcoded names in default RPC policies #8490

Open adrelanos opened 1 year ago

adrelanos commented 1 year ago

Qubes OS release

R4.2

Brief summary

After renaming,

Template upgrades are broken.

Steps to reproduce

  1. rename VMs UpdateVM (sys-net or sys-whonix) to something else
  2. try to upgrade a TemplateVM

Expected behavior

Functional upgrades or at least some error message with advice how to fix the issue.

Actual behavior

Broken upgrades.

Additional

Maybe there should be an error popup (or something):

"Check your Qubes UpdatesProxy configuration."

Still not terribly useful. Hence I will shortly suggest an alternative in a related ticket:

andrewdavidwong commented 1 year ago

I think the real problem here is that VM names should never be hardcoded in default RPC policy rules. Users aren't supposed to edit the default policies. Even if they do, those changes can be overwritten by an update at any time. Users who copy the relevant rules into 30-user.policy with their new VM names won't get any important changes to those rules that are pushed to the default policy files.

Besides, it's bad practice for the system to rely on any user-defined property having a specific value; that should be handled by some kind of system-defined property instead. The names of VMs must be assumed to be user-defined properties, since all the user interfaces in the system present these VMs alongside user-created VMs, and the system presents the user with a GUI to change the name on the "Basic" settings tab, which suggests to the user that it's a safe and reasonable thing to do.

If there's a component of the system that the user isn't supposed to touch, don't display it prominently alongside the user's own data, and don't invite the user to edit it in any GUIs. Instead, tuck it away in some "Expert" or "Advanced" area, or make it accessible only via the command line (possibly requiring an --expert flag).

andrewdavidwong commented 1 year ago

I can rename my Whonix templates without causing any problems for the default RPC policy rules, since they use @tag:whonix-updatevm instead of hardcoding the default Whonix template names. Is a similar solution available for sys-net and sys-whonix, at least as a short-term stopgap measure?

adrelanos commented 1 year ago

Your suggestion is valid however should that be a separate ticket?

The feature / bugfix that I'd like to see for this ticket would be for Qubes to make it more prominent if the user attempts to use a non-existing UpdateVM or UpdatesProxy. Only showing this in dom0 journal makes it too hard to debug.

It does not matter how the error case was reached.

marmarek commented 1 year ago

Tags work great for selecting which qube given policy rule applies to, in this case "all Whonix-related templates" or similar. But tags (currently) cannot be used to select call target (allow target=... part in the policy), this needs explicit name to guarantee it points at a specific thing, not multiple qubes (with the same tag). This part is not updated automatically when you rename qube.

I see several possible solutions here:

  1. When renaming, check also if any (user) policy rules would be affected, including those set by global config tool and propose to update those too. This is a very specific solution for the issue described here, and may not cover other similar cases (like removing a qube, cloning maybe?)
  2. Extend qrexec policy syntax to support more flexible call target selection. This can be either:
    • allow using tags, but verify if there is exactly one qube matching (if there are multiple, or none, deny the call); the failure mode here is still similar, but the rename case would work automatically; OTOH, the clone case will break now, unless user remove tag from one of the clones
    • allow using some other properties (or "features"?) to select call target, for example global "updatevm", or then add also per-vm "updatevm" property; this is more or less https://github.com/QubesOS/qubes-issues/issues/5397
  3. Point the user to the Qubes Global Config tool in the qrexec deny notification; the elegant approach could be a message parameter to the deny action, a final deny rule like this: qubes.UpdatesProxy * @anyvm @anyvm deny message="UpdateVM not set for this qube, check Qubes Global Config tool", even though I don't like explicit final deny... or maybe service-specific messages defined elsewhere, similar to discussed service-specific prompt dialogs https://github.com/QubesOS/qubes-issues/issues/5853
adrelanos commented 1 year ago

These are all good ideas.

  1. Seems hard to implement bug free. You'd need a tool that does string search and replace in user config files. That does not seem clean, reliable. Also if it cannot cover all cases such as cloning then it's not the best solution. Happy to be wrong about this.
  2. Also nice. Tags + verify exactly one qube matching only.
  3. That seems the most simple, straight forward, most realistic to implement. Since this is only a corner case then at least the user would have some sort of notification pointing into the right direction and allowing the support forums to more easily debug this and advice a solution.

These could all be worth doing and could all go into separate tickets.

andrewdavidwong commented 1 year ago
  1. This bug arises even if there are zero user policy rules; it happens with just the default rules. So, I worry that this either won't work or will lead to editing default policy files, which can be overwritten by an update at any time.
  2. The property one sounds nice, and it's great that there's already an issue for that.
  3. Yeah, any kind of user-facing warning for this would be a great first step (and sufficient to close this issue, IMHO, with subsequent fixes and enhancements having separate issues later).