From 61c702b156febe8f79e8d884e0964cbd1476fd9c Mon Sep 17 00:00:00 2001 From: very-amused Date: Mon, 8 Aug 2022 14:48:08 -0400 Subject: [PATCH 1/6] Make Unix sockets writable for mopidy group --- README.rst | 4 +++- mopidy_mpd/network.py | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 93cffec..031ce6b 100644 --- a/README.rst +++ b/README.rst @@ -88,8 +88,10 @@ The following configuration values are available: - ``::1``: Listens only on the IPv6 loopback interface. - ``0.0.0.0``: Listens on all IPv4 interfaces. - ``::``: Listens on all interfaces, both IPv4 and IPv6. - - ``unix:/path/to/unix/socket.sock``: Listen on the Unix socket at the + - ``unix:/var/run/mopidy/mpd.sock``: Listen on the Unix socket at the specified path. Must be prefixed with ``unix:``. + Users must be added to the `mopidy` group (`usermod -a -G mopidy user`) + to communicate with MPD over a Unix socket. - ``mpd/port``: Which TCP port the MPD server should listen to. diff --git a/mopidy_mpd/network.py b/mopidy_mpd/network.py index 63f83d9..6727d56 100644 --- a/mopidy_mpd/network.py +++ b/mopidy_mpd/network.py @@ -125,7 +125,9 @@ def create_server_socket(self, host, port): socket_path = get_unix_socket_path(host) if socket_path is not None: # host is a path so use unix socket sock = create_unix_socket() + oldmask = os.umask(0o002) # make socket writable for users in the 'mopidy' group sock.bind(socket_path) + os.umask(oldmask) else: # ensure the port is supplied if not isinstance(port, int): From 2aa5259fe2618ce67e02b134d0c7ff48b92d1afb Mon Sep 17 00:00:00 2001 From: very-amused Date: Mon, 8 Aug 2022 14:56:40 -0400 Subject: [PATCH 2/6] Fix Unix socket info formatting --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 031ce6b..b86e644 100644 --- a/README.rst +++ b/README.rst @@ -90,7 +90,7 @@ The following configuration values are available: - ``::``: Listens on all interfaces, both IPv4 and IPv6. - ``unix:/var/run/mopidy/mpd.sock``: Listen on the Unix socket at the specified path. Must be prefixed with ``unix:``. - Users must be added to the `mopidy` group (`usermod -a -G mopidy user`) + Users must be added to the ``mopidy`` group (``usermod -a -G mopidy user``) to communicate with MPD over a Unix socket. - ``mpd/port``: From 1cbeb2b745d542b1201aaf862877f4dd684acc15 Mon Sep 17 00:00:00 2001 From: very-amused Date: Mon, 8 Aug 2022 15:09:22 -0400 Subject: [PATCH 3/6] Clarify Unix socket requirements --- README.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.rst b/README.rst index b86e644..c22e3de 100644 --- a/README.rst +++ b/README.rst @@ -82,7 +82,7 @@ The following configuration values are available: - ``mpd/hostname``: Which address the MPD server should bind to. - This can be a network address or the path toa Unix socket: + This can be a network address or the path to a Unix socket: - ``127.0.0.1``: Listens only on the IPv4 loopback interface (default). - ``::1``: Listens only on the IPv6 loopback interface. @@ -90,8 +90,9 @@ The following configuration values are available: - ``::``: Listens on all interfaces, both IPv4 and IPv6. - ``unix:/var/run/mopidy/mpd.sock``: Listen on the Unix socket at the specified path. Must be prefixed with ``unix:``. - Users must be added to the ``mopidy`` group (``usermod -a -G mopidy user``) - to communicate with MPD over a Unix socket. + If `Mopidy is run as a system service `_, + users must be added to the ``mopidy`` group (``usermod -a -G mopidy user``) + to communicate with the MPD server over a Unix socket. - ``mpd/port``: Which TCP port the MPD server should listen to. From c64add2297455e775df7ba05a6c37c58b05a4ad4 Mon Sep 17 00:00:00 2001 From: very-amused Date: Mon, 15 Aug 2022 08:25:41 -0400 Subject: [PATCH 4/6] Wrap Unix socket bind in try/finally clause --- mopidy_mpd/network.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mopidy_mpd/network.py b/mopidy_mpd/network.py index 6727d56..705fccb 100644 --- a/mopidy_mpd/network.py +++ b/mopidy_mpd/network.py @@ -125,9 +125,12 @@ def create_server_socket(self, host, port): socket_path = get_unix_socket_path(host) if socket_path is not None: # host is a path so use unix socket sock = create_unix_socket() - oldmask = os.umask(0o002) # make socket writable for users in the 'mopidy' group - sock.bind(socket_path) - os.umask(oldmask) + # make socket writable for users in the 'mopidy' group + oldmask = os.umask(0o002) + try: + sock.bind(socket_path) + finally: + os.umask(oldmask) else: # ensure the port is supplied if not isinstance(port, int): From 08efe7af9f0e526b2ac22f7b55fd2a076a25dd11 Mon Sep 17 00:00:00 2001 From: very-amused Date: Sun, 16 Oct 2022 08:52:03 -0400 Subject: [PATCH 5/6] Configurable Unix socket permissions --- README.rst | 11 ++++++++++- mopidy_mpd/__init__.py | 1 + mopidy_mpd/actor.py | 2 ++ mopidy_mpd/ext.conf | 1 + mopidy_mpd/network.py | 25 +++++++++++++++++++++++-- 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/README.rst b/README.rst index c22e3de..953b728 100644 --- a/README.rst +++ b/README.rst @@ -91,13 +91,22 @@ The following configuration values are available: - ``unix:/var/run/mopidy/mpd.sock``: Listen on the Unix socket at the specified path. Must be prefixed with ``unix:``. If `Mopidy is run as a system service `_, - users must be added to the ``mopidy`` group (``usermod -a -G mopidy user``) + ``mpd/socket_permissions`` must allow group write access (default) + and users must be added to the ``mopidy`` group (``usermod -a -G mopidy user``) to communicate with the MPD server over a Unix socket. - ``mpd/port``: Which TCP port the MPD server should listen to. Default: 6600. +- ``mpd/socket_permissions``: + The octal permission value used for the Unix socket created by the MPD server + (only applies if ``mpd/hostname`` is a unix socket). + + - ``775``: rwx for user and group, r-x for others (default). + - ``777``: rwx for all users. + - ``755``: rwx for user, r-x for others. + - ``mpd/password``: The password required for connecting to the MPD server. If blank, no password is required. diff --git a/mopidy_mpd/__init__.py b/mopidy_mpd/__init__.py index 49f47e1..d52c353 100644 --- a/mopidy_mpd/__init__.py +++ b/mopidy_mpd/__init__.py @@ -20,6 +20,7 @@ def get_config_schema(self): schema = super().get_config_schema() schema["hostname"] = config.Hostname() schema["port"] = config.Port(optional=True) + schema["socket_permissions"] = config.String() schema["password"] = config.Secret(optional=True) schema["max_connections"] = config.Integer(minimum=1) schema["connection_timeout"] = config.Integer(minimum=1) diff --git a/mopidy_mpd/actor.py b/mopidy_mpd/actor.py index 882a187..37976ba 100644 --- a/mopidy_mpd/actor.py +++ b/mopidy_mpd/actor.py @@ -32,6 +32,7 @@ def __init__(self, config, core): self.hostname = network.format_hostname(config["mpd"]["hostname"]) self.port = config["mpd"]["port"] + self.socket_permissions = config["mpd"]["socket_permissions"] self.uri_map = uri_mapper.MpdUriMapper(core) self.zeroconf_name = config["mpd"]["zeroconf"] @@ -44,6 +45,7 @@ def _setup_server(self, config, core): server = network.Server( self.hostname, self.port, + self.socket_permissions, protocol=session.MpdSession, protocol_kwargs={ "config": config, diff --git a/mopidy_mpd/ext.conf b/mopidy_mpd/ext.conf index ee518a8..017f21a 100644 --- a/mopidy_mpd/ext.conf +++ b/mopidy_mpd/ext.conf @@ -2,6 +2,7 @@ enabled = true hostname = 127.0.0.1 port = 6600 +socket_permissions = 775 password = max_connections = 20 connection_timeout = 60 diff --git a/mopidy_mpd/network.py b/mopidy_mpd/network.py index 705fccb..5dfafcd 100644 --- a/mopidy_mpd/network.py +++ b/mopidy_mpd/network.py @@ -36,6 +36,24 @@ def get_socket_address(host, port): else: return (host, port) +def get_socket_umask(perms): + default_umask = 0o002 + all_perms = 0o777 + mask = all_perms - int(perms, 8) + if mask < 0: + logger.error( + f"Invalid Unix socket permission value: {perms}, " + f"reverting to default permission of 775." + ) + return default_umask + elif mask >= 0o100: + logger.error( + f"Unix socket permission must allow user rwx, " + f"reverting to default permission of 775." + ) + return default_umask + else: + return mask class ShouldRetrySocketCall(Exception): @@ -99,6 +117,7 @@ def format_hostname(hostname): return hostname + class Server: """Setup listener and register it with GLib's event loop.""" @@ -107,6 +126,7 @@ def __init__( self, host, port, + socket_permissions, protocol, protocol_kwargs=None, max_connections=5, @@ -118,6 +138,7 @@ def __init__( self.timeout = timeout self.server_socket = self.create_server_socket(host, port) self.address = get_socket_address(host, port) + self.umask = get_socket_umask(socket_permissions) self.watcher = self.register_server_socket(self.server_socket.fileno()) @@ -125,8 +146,8 @@ def create_server_socket(self, host, port): socket_path = get_unix_socket_path(host) if socket_path is not None: # host is a path so use unix socket sock = create_unix_socket() - # make socket writable for users in the 'mopidy' group - oldmask = os.umask(0o002) + # apply socket perms from config + oldmask = os.umask(self.umask) try: sock.bind(socket_path) finally: From 84636ba50780104f481f9f236dff030c9d211a9e Mon Sep 17 00:00:00 2001 From: very-amused Date: Sun, 16 Oct 2022 09:31:59 -0400 Subject: [PATCH 6/6] Make socket perms optional (use default value) Modified the call signature Server.create_server_socket to accept socket_permissions as an optional parameter. get_socket_umask will return the default umask of `0o002` when called with no explicit permissions. --- mopidy_mpd/__init__.py | 2 +- mopidy_mpd/actor.py | 2 +- mopidy_mpd/network.py | 11 +++++++---- tests/network/test_server.py | 2 +- tests/test_actor.py | 1 + 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/mopidy_mpd/__init__.py b/mopidy_mpd/__init__.py index d52c353..113faf3 100644 --- a/mopidy_mpd/__init__.py +++ b/mopidy_mpd/__init__.py @@ -20,7 +20,7 @@ def get_config_schema(self): schema = super().get_config_schema() schema["hostname"] = config.Hostname() schema["port"] = config.Port(optional=True) - schema["socket_permissions"] = config.String() + schema["socket_permissions"] = config.String(optional=True) schema["password"] = config.Secret(optional=True) schema["max_connections"] = config.Integer(minimum=1) schema["connection_timeout"] = config.Integer(minimum=1) diff --git a/mopidy_mpd/actor.py b/mopidy_mpd/actor.py index 37976ba..514e849 100644 --- a/mopidy_mpd/actor.py +++ b/mopidy_mpd/actor.py @@ -45,7 +45,6 @@ def _setup_server(self, config, core): server = network.Server( self.hostname, self.port, - self.socket_permissions, protocol=session.MpdSession, protocol_kwargs={ "config": config, @@ -54,6 +53,7 @@ def _setup_server(self, config, core): }, max_connections=config["mpd"]["max_connections"], timeout=config["mpd"]["connection_timeout"], + socket_permissions=self.socket_permissions, ) except OSError as exc: raise exceptions.FrontendError(f"MPD server startup failed: {exc}") diff --git a/mopidy_mpd/network.py b/mopidy_mpd/network.py index 5dfafcd..e736de9 100644 --- a/mopidy_mpd/network.py +++ b/mopidy_mpd/network.py @@ -38,6 +38,8 @@ def get_socket_address(host, port): def get_socket_umask(perms): default_umask = 0o002 + if perms is None: + return default_umask all_perms = 0o777 mask = all_perms - int(perms, 8) if mask < 0: @@ -126,28 +128,29 @@ def __init__( self, host, port, - socket_permissions, protocol, protocol_kwargs=None, max_connections=5, timeout=30, + socket_permissions=None, ): self.protocol = protocol self.protocol_kwargs = protocol_kwargs or {} self.max_connections = max_connections self.timeout = timeout - self.server_socket = self.create_server_socket(host, port) + self.server_socket = self.create_server_socket(host, port, socket_permissions) self.address = get_socket_address(host, port) self.umask = get_socket_umask(socket_permissions) self.watcher = self.register_server_socket(self.server_socket.fileno()) - def create_server_socket(self, host, port): + def create_server_socket(self, host, port, socket_permissions=None): socket_path = get_unix_socket_path(host) if socket_path is not None: # host is a path so use unix socket sock = create_unix_socket() # apply socket perms from config - oldmask = os.umask(self.umask) + socket_umask = get_socket_umask(socket_permissions) + oldmask = os.umask(socket_umask) try: sock.bind(socket_path) finally: diff --git a/tests/network/test_server.py b/tests/network/test_server.py index 467d979..6e09b6c 100644 --- a/tests/network/test_server.py +++ b/tests/network/test_server.py @@ -21,7 +21,7 @@ def test_init_calls_create_server_socket(self): self.mock, sentinel.host, sentinel.port, sentinel.protocol ) self.mock.create_server_socket.assert_called_once_with( - sentinel.host, sentinel.port + sentinel.host, sentinel.port, None ) self.mock.stop() diff --git a/tests/test_actor.py b/tests/test_actor.py index cb85fad..83a418f 100644 --- a/tests/test_actor.py +++ b/tests/test_actor.py @@ -31,6 +31,7 @@ def test_idle_hooked_up_correctly(event, expected): "mpd": { "hostname": "foobar", "port": 1234, + "socket_permissions": "775", "zeroconf": None, "max_connections": None, "connection_timeout": None,