Mark dbus methods as unprivileged

CHANGELOG.md

@@ -1,3 +1,8 @@
+yyyy/mm/dd Version 4.0.1-1
+--------------------------
+- Dbus methods now marked as unprivileged
+- sdbus library errors have improved logging
+
 2025/02/06 Version 4.0.0-1
 --------------------------
 - `plusdeckctl` connects to the system bus by default

docs/dbus.md

@@ -53,8 +53,6 @@ The interface is similar to the vanilla `plusdeck` CLI. However, there are a few
 
 ## Dbus Access Policies
 
-**NOTE: Full access for `plusdeck` group access is an area of active development. This feature does not work - at least, on Fedora.** To follow along, view [this StackExchange post](https://unix.stackexchange.com/questions/790750/dbus-policy-that-allows-group-to-access-system-service). and this [Fedora discussion post](https://discussion.fedoraproject.org/t/dbus-policy-that-allows-group-to-access-system-service/144265).
-
 When running services under the `system` bus, care must be taken to manage access policies. Dbus does this primarily with [an XML-based policy language](https://dbus.freedesktop.org/doc/dbus-daemon.1.html). Systemd additionally manages access to privileged methods, seemingly with the intent of delegating to polkit.
 
 By default, Dbus is configured with the following policies:
@@ -69,12 +67,6 @@ sudo groupadd plusdeck
 sudo usermod -a -G plusdeck "${USER}"
 ```
 
-### Polkit
-
-**NOTE: The Polkit policies have not been shown to work at this time.**
-
-Prototype Polkit policies/rules may be found in the `./polkit` folder.
-
 ## Running `plusdeckd` Directly
 
 The DBus service can be launched directly using `plusdeckd`:
@@ -123,19 +115,3 @@ I have a just task for that:
 ```sh
 just get-dbus-iface
 ```
-
-### Debugging SELinux
-
-While I haven't seen this to be the case, it seems theoretically possible for SELinux to block access to Dbus.
-
-You should be able to see access denials due to SELinux by running either:
-
-```sh
-sudo ausearch -ts recent
-```
-
-or:
-
-```sh
-sudo tail -f /var/log/audit/audit.log
-```

plusdeck/dbus/client.py

@@ -18,6 +18,7 @@
 from plusdeck.client import State
 from plusdeck.config import Config
 from plusdeck.dbus.config import StagedConfig
+from plusdeck.dbus.error import handle_dbus_error
 from plusdeck.dbus.interface import DBUS_NAME, DbusInterface
 
 logger = logging.getLogger(__name__)
@@ -72,18 +73,8 @@ def pass_client(fn: AsyncCommand) -> AsyncCommand:
     @click.pass_obj
     @functools.wraps(fn)
     async def wrapped(obj: Obj, *args, **kwargs) -> None:
-        try:
+        async with handle_dbus_error(logger):
             await fn(obj.client, *args, **kwargs)
-        except Exception as exc:
-            if hasattr(exc, "dbus_error_name"):
-                dbus_error_name = cast(Any, exc).dbus_error_name
-                dbus_msg = str(exc)
-                if dbus_msg:
-                    logger.error(f"{dbus_error_name}: {dbus_msg}")
-                else:
-                    logger.error(f"{dbus_error_name}")
-                sys.exit(1)
-            raise
 
     return wrapped
 

plusdeck/dbus/error.py

@@ -0,0 +1,38 @@
+from contextlib import asynccontextmanager
+from logging import Logger
+import re
+import sys
+from traceback import format_exc
+from typing import Any, AsyncGenerator, cast, List, Optional
+
+from sdbus.sd_bus_internals import SdBusLibraryError
+
+ERROR_NUMBER_RE = r"returned error number: (\d+)"
+
+
+@asynccontextmanager
+async def handle_dbus_error(logger: Logger) -> AsyncGenerator[None, None]:
+    try:
+        yield
+    except Exception as exc:
+        exit_code: Optional[int] = None
+        if isinstance(exc, SdBusLibraryError):
+            logger.debug(format_exc())
+
+            error_numbers: List[str] = re.findall(ERROR_NUMBER_RE, str(exc))
+            exit_code = int(error_numbers[0]) if error_numbers else 1
+
+            logger.error(f"SdBusLibraryError: {exc}")
+        elif hasattr(exc, "dbus_error_name"):
+            exit_code = 1
+            dbus_error_name = cast(Any, exc).dbus_error_name
+            dbus_msg = str(exc)
+            if dbus_msg:
+                logger.error(f"{dbus_error_name}: {dbus_msg}")
+            else:
+                logger.error(f"{dbus_error_name}")
+
+        if exit_code is not None:
+            sys.exit(exit_code)
+
+        raise exc

plusdeck/dbus/interface.py

@@ -7,6 +7,7 @@
     dbus_property_async,
     dbus_signal_async,
     DbusInterfaceCommonAsync,
+    DbusUnprivilegedFlag,
 )
 
 from plusdeck.client import Client, create_connection, Receiver, State
@@ -88,78 +89,78 @@ def closed(self: Self) -> asyncio.Future:
 
         return self.client.closed
 
-    @dbus_method_async("")
+    @dbus_method_async("", flags=DbusUnprivilegedFlag)
     async def play_a(self: Self) -> None:
         """
         Play side A.
         """
         self.client.play_a()
 
-    @dbus_method_async("")
+    @dbus_method_async("", flags=DbusUnprivilegedFlag)
     async def play_b(self: Self) -> None:
         """
         Play side B.
         """
 
         self.client.play_b()
 
-    @dbus_method_async("")
+    @dbus_method_async("", flags=DbusUnprivilegedFlag)
     async def fast_forward_a(self: Self) -> None:
         """
         Fast-forward side A.
         """
 
         self.client.fast_forward_a()
 
-    @dbus_method_async("")
+    @dbus_method_async("", flags=DbusUnprivilegedFlag)
     async def fast_forward_b(self: Self) -> None:
         """
         Fast-forward side B.
         """
 
         self.client.fast_forward_b()
 
-    @dbus_method_async("")
+    @dbus_method_async("", flags=DbusUnprivilegedFlag)
     async def rewind_a(self: Self) -> None:
         """
         Rewind side A. Equivalent to fast-forwarding side B.
         """
 
         self.client.rewind_a()
 
-    @dbus_method_async("")
+    @dbus_method_async("", flags=DbusUnprivilegedFlag)
     async def rewind_b(self: Self) -> None:
         """
         Rewind side B. Equivalent to fast-forwarding side A.
         """
 
         self.client.rewind_b()
 
-    @dbus_method_async("")
+    @dbus_method_async("", flags=DbusUnprivilegedFlag)
     async def pause(self: Self) -> None:
         """
         Pause if playing, or start playing if paused.
         """
 
         self.client.pause()
 
-    @dbus_method_async("")
+    @dbus_method_async("", flags=DbusUnprivilegedFlag)
     async def stop(self: Self) -> None:
         """
         Stop the tape.
         """
 
         self.client.stop()
 
-    @dbus_method_async("")
+    @dbus_method_async("", flags=DbusUnprivilegedFlag)
     async def eject(self: Self) -> None:
         """
         Eject the tape.
         """
 
         self.client.eject()
 
-    @dbus_method_async("sd", "b")
+    @dbus_method_async("sd", "b", flags=DbusUnprivilegedFlag)
     async def wait_for(self: Self, state: str, timeout: float) -> bool:
         """
         Wait for an expected state, with an optional timeout. When timeout is negative,

plusdeck/dbus/service.py

@@ -10,6 +10,7 @@
 
 from plusdeck.cli import LogLevel
 from plusdeck.config import GLOBAL_FILE
+from plusdeck.dbus.error import handle_dbus_error
 from plusdeck.dbus.interface import DBUS_NAME, DbusInterface, load_client
 
 logger = logging.getLogger(__name__)
@@ -40,9 +41,10 @@ async def serve(config_file: Optional[str] = None) -> None:
     Create and serve configure DBus service with a supplied config file.
     """
 
-    srv = await service(config_file)
+    async with handle_dbus_error(logger):
+        srv = await service(config_file)
 
-    await srv.closed
+        await srv.closed
 
 
 @click.command