Add a tool to convert legacy policy files to new format #123

marmarta · 2023-08-29T22:01:52Z

codecov · 2023-08-30T13:41:22Z

Codecov Report

Merging #123 (2849c90) into main (02ff1aa) will increase coverage by 1.56%.
Report is 9 commits behind head on main.
The diff coverage is 90.63%.

@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
+ Coverage   85.09%   86.66%   +1.56%     
==========================================
  Files          31       33       +2     
  Lines        5174     5503     +329     
==========================================
+ Hits         4403     4769     +366     
+ Misses        771      734      -37

Files	Coverage Δ
qrexec/tests/qrexec_legacy_convert.py	`100.00% <100.00%> (ø)`
qrexec/tools/qrexec_policy_graph.py	`80.26% <100.00%> (+80.26%)`	⬆️
qrexec/tools/qrexec_legacy_convert.py	`81.65% <81.65%> (ø)`

... and 3 files with indirect coverage changes

marmarek

Those are some initial comments. I'll do more tests on some policies I have.

But also, maybe add some tests in this repo too (run it with synthetic input dir and system_info.json file)? it will be easier to test weirder setups this way

marmarek · 2023-09-18T01:45:06Z

qrexec/tools/qrexec_legacy_convert.py

+                    rules_to_save[CONFIG_FILE].append(rule)
+                    continue
+                rules_to_save[filename].append(rule)
+            rules_to_save[filename] = missing


This overrides what could be added above. I don't think this assignment is supposed to be here.

marmarek · 2023-09-18T01:48:29Z

qrexec/tools/qrexec_legacy_convert.py

+            print(line)
+        if input("Do you want to restore initial state? [Y/n] ").upper() != "N":
+            # rename all old
+            for file in OLD_POLICY_DIR.iterdir():


Better store list of renamed files, there could be some rpmsave files before (not created by this tool).

marmarek · 2023-09-18T01:50:27Z

qrexec/tools/qrexec_legacy_convert.py

+            for filename in rules_to_save:
+                file = NEW_POLICY_DIR / (filename + ".policy")
+                if file.exists():
+                    file.unlink()
+
+            # revert all new
+            for file in NEW_POLICY_DIR.iterdir():
+                if file.is_file() and file.name.endswith('.bak'):
+                    file.rename(file.with_suffix(''))


Here too - you do use list of files you saved to remove new files, but you should use that for restoring from .bak too.

marmarek · 2023-09-18T02:38:57Z

qrexec/tools/qrexec_legacy_convert.py

+    def __str__(self):
+        return str(self.rule)
+
+def main(_args=None):


This tool should handle some arguments, even if it's just help that no arguments are there. Otherwise running qrexec-legacy-convert --help will actually make the conversion, which isn't nice.

marmarek · 2023-09-18T02:40:43Z

qrexec/tools/qrexec_legacy_convert.py

+
+def main(_args=None):
+    # get initial state
+    initial_state = set(subprocess.check_output(


This takes a bit of time, add some message before what is happening.

marmarek · 2023-09-18T13:50:55Z

qrexec/tools/qrexec_legacy_convert.py

+            if missing:
+                rules_to_save[CONFIG_FILE].extend(missing)
+
+        elif service in ('u2f.Authenticate', 'u2f.Register',


This is not specific enough - it puts rules for specific U2F keys in the policy file too, but they aren't currently handled correctly by the global config tool: QubesOS/qubes-issues#8525

this probably means those rules with a specified argument should go into their appropriate file - 60-registered-arguments.policy

fixes QubesOS/qubes-issues#8000

marmarek

I've found few more cases that are mishandled:

qubes.Gpg with @anyvm @anyvm ask at the end, but some other rules earlier. For example:

work1 keys1 allow
work2 keys2 allow
work2-bis keys2 allow
@anyvm  keys1 deny
@anyvm  keys2 deny
@anyvm  keys3 deny
@anyvm  @anyvm  ask

This sets special handling for keys1/2/3 (and some qubes that use them), but otherwise allows user to choose. And more importantly, for interactive selection, those keys1/2/3 are not available (due to the deny rules). The conversion tool puts @anyvm @anyvm ask into 30-user.policy, and other rules to 50-config-splitgpg.policy. And then correctly detects it isn't the same semantics (because in this case order matters).
This case is important, because default qubes.Gpg policy used to have @anyvm @anyvm ask, so this case will trigger for many users. Maybe put this wildcard rule to 51-config-splitgpg-default.policy (if found in legacy policy) ? The removal of the wildcard rule in default config was intentional (since we have nice GUI for setting specific rules now), but I don't know how to nicely handle it during conversion, to not risk breaking existing setup (removing this rule might be okay in some cases, but not some others).
Or maybe, just put all qubes.Gpg rules into 30-user.policy (preserving their order), and let the user migrate this part manually (see 1a below)?

1a. The global config GUI complains it doesn't understand the "allow" rules for qubes.Gpg service from the point above. So, those shouldn't be added to 50-config-splitgpg.policy.

Some updates proxy rules were put in 30-user.policy, instead of 50-config-updates.policy:

qubes.UpdatesProxy	*	fedora-39-mintest	@default	allow target=sys-net
qubes.UpdatesProxy	*	@type:TemplateVM	@default	allow target=sys-whonix

The first one is expected as custom rule I think, but the second one looks standard, no?

policy.RegisterArgument +u2f.Authenticate put into 30-user.policy instead of 50-config-u2f.policy:

policy.RegisterArgument	+u2f.Authenticate	sys-usb	vm1	allow target=@adminvm
policy.RegisterArgument	+u2f.Authenticate	sys-usb	vm2	allow target=@adminvm
policy.RegisterArgument	+u2f.Authenticate	sys-usb	vm3	allow target=@adminvm
policy.RegisterArgument	+u2f.Authenticate	sys-usb	@anyvm	ask default_target=@adminvm

Not sure about the last one, but others look standard. This one I found why - see below.

qrexec/tools/qrexec_legacy_convert.py

Enable outputting more information about permissions/actions Make output more flexible (can be provided as output parameter, not just sys.stdout)

marmarta force-pushed the policyconvert branch from 34d6af8 to 28211c1 Compare August 30, 2023 13:13

aronowski mentioned this pull request Sep 3, 2023

In-place upgrade from 4.1 to 4.2-RC2 creates qubes.Input*.rpmsave files, preventing boot after --all-pre-reboot stage QubesOS/qubes-issues#8480

Closed

marmarek requested changes Sep 18, 2023

View reviewed changes

marmarta force-pushed the policyconvert branch 2 times, most recently from cbe7ef2 to c3a82be Compare September 25, 2023 21:13

Add a tool to convert legacy policy files to new format

b570f1f

fixes QubesOS/qubes-issues#8000

marmarta force-pushed the policyconvert branch 2 times, most recently from b3fb06a to 8e96e9a Compare September 25, 2023 21:31

marmarek requested changes Oct 6, 2023

View reviewed changes

qrexec/tools/qrexec_legacy_convert.py Outdated Show resolved Hide resolved

qrexec/tools/qrexec_legacy_convert.py Outdated Show resolved Hide resolved

qrexec/tools/qrexec_legacy_convert.py Outdated Show resolved Hide resolved

marmarta force-pushed the policyconvert branch from 8e96e9a to bb40592 Compare October 10, 2023 13:59

marmarek requested changes Oct 10, 2023

View reviewed changes

qrexec/tools/qrexec_legacy_convert.py Outdated Show resolved Hide resolved

marmarta added 2 commits October 10, 2023 23:17

Modify qrexec-policy-graph for more flexibility

8988142

Enable outputting more information about permissions/actions Make output more flexible (can be provided as output parameter, not just sys.stdout)

Tests for legacy policy convert tool

2849c90

marmarta force-pushed the policyconvert branch from bb40592 to 2849c90 Compare October 10, 2023 21:17

marmarek approved these changes Oct 10, 2023

View reviewed changes

marmarek merged commit 45af0f0 into QubesOS:main Oct 10, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a tool to convert legacy policy files to new format #123

Add a tool to convert legacy policy files to new format #123

marmarta commented Aug 29, 2023

codecov bot commented Aug 30, 2023 •

edited

marmarek left a comment

marmarek Sep 18, 2023

marmarek Sep 18, 2023

marmarek Sep 18, 2023 •

edited

marmarek Sep 18, 2023

marmarek Sep 18, 2023

marmarek Sep 18, 2023

marmarta Sep 19, 2023

marmarek left a comment

Add a tool to convert legacy policy files to new format #123

Add a tool to convert legacy policy files to new format #123

Conversation

marmarta commented Aug 29, 2023

codecov bot commented Aug 30, 2023 • edited

Codecov Report

marmarek left a comment

Choose a reason for hiding this comment

marmarek Sep 18, 2023

Choose a reason for hiding this comment

marmarek Sep 18, 2023

Choose a reason for hiding this comment

marmarek Sep 18, 2023 • edited

Choose a reason for hiding this comment

marmarek Sep 18, 2023

Choose a reason for hiding this comment

marmarek Sep 18, 2023

Choose a reason for hiding this comment

marmarek Sep 18, 2023

Choose a reason for hiding this comment

marmarta Sep 19, 2023

Choose a reason for hiding this comment

marmarek left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 30, 2023 •

edited

marmarek Sep 18, 2023 •

edited