Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions qubes_config/global_config/policy_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,18 @@
import subprocess
from typing import Optional, List, Tuple

from qubes_config.widgets.utils import compare_rule_lists
from qrexec.policy.admin_client import PolicyClient
from qrexec.policy.parser import StringPolicy, Rule
from qubes_config.widgets.utils import compare_rule_lists

try:
from qrexec.policy.admin import (
PolicyAdminException,
PolicyAdminFileNotFoundException,
)
except ImportError:
PolicyAdminException = subprocess.CalledProcessError
PolicyAdminFileNotFoundException = subprocess.CalledProcessError

import gettext

Expand Down Expand Up @@ -51,7 +60,7 @@ def get_all_policy_files(self, service: str) -> List[str]:
"""Just get a straightforward list of all relevant policy files."""
try:
return self.policy_client.policy_get_files(service)
except subprocess.CalledProcessError:
except (PolicyAdminException, subprocess.CalledProcessError):
return []

def get_conflicting_policy_files(self, service: str, own_file: str) -> List[str]:
Expand Down Expand Up @@ -83,7 +92,7 @@ def get_rules_from_filename(
for the file."""
try:
rules_text, token = self.policy_client.policy_get(filename)
except subprocess.CalledProcessError:
except (PolicyAdminFileNotFoundException, subprocess.CalledProcessError):
if not default_policy:
return [], None
rules_text, token = default_policy, None
Expand Down
22 changes: 17 additions & 5 deletions qubes_config/policy_editor/policy_editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@
from qrexec.policy.parser import StringPolicy
from qrexec.exc import PolicySyntaxError

try:
from qrexec.policy.admin import (
PolicyAdminException,
PolicyAdminFileNotFoundException,
)
except ImportError:
PolicyAdminException = subprocess.CalledProcessError
PolicyAdminFileNotFoundException = subprocess.CalledProcessError

from qubes_config.widgets.gtk_utils import (
load_theme,
show_error,
Expand Down Expand Up @@ -450,9 +459,9 @@ def _save(self, *_args):
self.policy_client.policy_replace(
self.filename, self.policy_text, self.token
)
except subprocess.CalledProcessError as ex:
err_msg = "An error occurred while trying to save the policy file:\n"
if ex.stdout:
except (PolicyAdminException, subprocess.CalledProcessError) as ex:
err_msg = f"Failed to replace policy {self.filename!r}: "
if hasattr(ex, "stdout"):
err_msg += ex.stdout.decode()
else:
err_msg += str(ex)
Expand Down Expand Up @@ -533,8 +542,11 @@ def open_policy_file(self, name: Optional[str]):
else:
try:
text, self.token = self.policy_client.policy_get(name)
except subprocess.CalledProcessError as ex:
if ex.returncode == 126:
except (
PolicyAdminFileNotFoundException,
subprocess.CalledProcessError,
) as ex:
if getattr(ex, "returncode", None) == 126:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong - it makes real subprocess.CalledProcessError not handled anymore, for example when the call was denied by the policy (which is the exit code 126), but could be also other cases.

Same comment for the other functions.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started with separate except clauses but wanted to avoid duplication. I will return to that, but will have to find a way to deal in case the exception class is not present yet (outdated qrexec). You solution to reassign the exception to subprocess.CalledProcessError might be the one.

show_error(
self.main_window,
"Access denied",
Expand Down
20 changes: 20 additions & 0 deletions qubes_config/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@
MockDevice,
)

try:
from qrexec.policy.admin import (
PolicyAdminFileNotFoundException,
PolicyAdminTokenException,
)

admin_exc = True
except ImportError:
admin_exc = False


@pytest.fixture
def test_qapp():
Expand Down Expand Up @@ -291,6 +301,8 @@ def policy_get(self, file_name):
"""Get file contents; takes into account policy_replace."""
if file_name in self.files:
return self.files[file_name], self.file_tokens[file_name]
if admin_exc:
raise PolicyAdminFileNotFoundException
raise subprocess.CalledProcessError(2, "test")

def policy_include_get(self, file_name):
Expand All @@ -300,15 +312,21 @@ def policy_include_get(self, file_name):
self.include_files[file_name],
self.include_file_tokens[file_name],
)
if admin_exc:
raise PolicyAdminFileNotFoundException
raise subprocess.CalledProcessError(2, "test")

def policy_replace(self, filename, policy_text, token="any"):
"""Replace file contents with provided contents."""
if token == "new":
if filename in self.file_tokens:
if admin_exc:
raise PolicyAdminTokenException
raise subprocess.CalledProcessError(2, "test")
elif token != "any":
if token != self.file_tokens.get(filename, ""):
if admin_exc:
raise PolicyAdminTokenException
raise subprocess.CalledProcessError(2, "test")
self.files[filename] = policy_text
self.file_tokens[filename] = str(len(policy_text))
Expand All @@ -317,6 +335,8 @@ def policy_include_replace(self, filename, policy_text, token="any"):
"""Replace file contents with provided contents."""
if token != "any":
if token != self.include_file_tokens.get(filename, ""):
if admin_exc:
raise PolicyAdminTokenException
raise subprocess.CalledProcessError(2, "test")
self.include_files[filename] = policy_text
self.include_file_tokens[filename] = str(len(policy_text))
Expand Down
15 changes: 14 additions & 1 deletion qubes_config/tests/test_policy_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
from ..global_config.policy_manager import PolicyManager
from qrexec.policy.parser import Rule

try:
from qrexec.policy.admin import PolicyAdminFileNotFoundException

admin_exc = True
except ImportError:
admin_exc = False


def test_conflict_files():
def return_files(service_name):
Expand Down Expand Up @@ -58,10 +65,14 @@ def test_get_policy_from_file_new_no_default(mock_replace, mock_get):
manager = PolicyManager()

mock_get.side_effect = subprocess.CalledProcessError(2, "test")

assert manager.get_rules_from_filename("test", "") == ([], None)
assert not mock_replace.mock_calls

if admin_exc:
mock_get.side_effect = PolicyAdminFileNotFoundException
assert manager.get_rules_from_filename("test", "") == ([], None)
assert not mock_replace.mock_calls


def test_get_policy_from_file_new():
class MockPolicy:
Expand All @@ -71,6 +82,8 @@ def __init__(self):
def policy_get(self, filename):
if filename in self.files:
return self.files[filename], filename
if admin_exc:
raise PolicyAdminFileNotFoundException
raise subprocess.CalledProcessError(2, "test")

def policy_replace(self, filename, text):
Expand Down