From 059089e940df67028c4af2390b83ac798c7c35f6 Mon Sep 17 00:00:00 2001 From: hermanntoast Date: Thu, 23 Jul 2026 12:29:28 +0200 Subject: [PATCH 1/9] Security: escape values interpolated into LDAP search filters Usernames, hostnames and policy names were embedded into LDAP filters via f-strings without escaping, allowing LDAP filter injection through crafted AD principal names. All values now go through ldap.filter.escape_filter_chars(). --- .../linuxmusterLinuxclient7/computer.py | 2 +- .../dist-packages/linuxmusterLinuxclient7/gpo.py | 4 ++-- .../linuxmusterLinuxclient7/ldapHelper.py | 16 ++++++++++++++-- .../tests/test_ldapHelper.py | 12 ++++++++++++ .../linuxmusterLinuxclient7/user.py | 2 +- 5 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/computer.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/computer.py index dd71347..09dde64 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/computer.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/computer.py @@ -26,7 +26,7 @@ def readAttributes(): :return: Tuple (success, dict of attributes) :rtype: tuple """ - return ldapHelper.searchOne(f"(sAMAccountName={hostname()}$)") + return ldapHelper.searchOne(f"(sAMAccountName={ldapHelper.escape(hostname())}$)") def isInGroup(groupName): """ diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/gpo.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/gpo.py index 1a845b2..4463331 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/gpo.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/gpo.py @@ -80,7 +80,7 @@ def _findApplicablePolicies(): policyName = f"sophomorix:school:{schoolName}" # find policy - rc, policyAdObject = ldapHelper.searchOne(f"(displayName={policyName})") + rc, policyAdObject = ldapHelper.searchOne(f"(displayName={ldapHelper.escape(policyName)})") if not rc: return False, None @@ -99,7 +99,7 @@ def _parsePolicy(policyDn): """ # Find policy in AD - rc, policyAdObject = ldapHelper.searchOne(f"(distinguishedName={policyDn[0]})") + rc, policyAdObject = ldapHelper.searchOne(f"(distinguishedName={ldapHelper.escape(policyDn[0])})") if not rc: logging.error("===> Could not find poilcy in AD! ===") return False diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/ldapHelper.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/ldapHelper.py index 9d8c124..f9d7b9f 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/ldapHelper.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/ldapHelper.py @@ -1,8 +1,20 @@ -import ldap, ldap.sasl, sys, getpass, subprocess +import ldap, ldap.sasl, ldap.filter, sys, getpass, subprocess from linuxmusterLinuxclient7 import logging, constants, config, user, computer _currentLdapConnection = None +def escape(value): + """ + Escapes a value so it can be safely embedded into an LDAP search filter. + This prevents LDAP filter injection (e.g. via crafted usernames or hostnames). + + :param value: The value to escape + :type value: str + :return: The escaped value + :rtype: str + """ + return ldap.filter.escape_filter_chars(str(value)) + def serverUrl(): """ Returns the server URL @@ -109,7 +121,7 @@ def isObjectInGroup(objectDn, groupName): :rtype: bool """ logging.debug(f"= Testing if object {objectDn} is a member of group {groupName} =") - rc, groupAdObject = searchOne(f"(&(member:1.2.840.113556.1.4.1941:={objectDn})(sAMAccountName={groupName}))") + rc, groupAdObject = searchOne(f"(&(member:1.2.840.113556.1.4.1941:={escape(objectDn)})(sAMAccountName={escape(groupName)}))") logging.debug(f"=> Result: {rc} =") return rc diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py new file mode 100644 index 0000000..f3d9741 --- /dev/null +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py @@ -0,0 +1,12 @@ +from .. import ldapHelper + + +def test_escape(): + # Plain values are unchanged + assert ldapHelper.escape("user1") == "user1" + # LDAP filter metacharacters are escaped so they cannot alter the filter + assert ldapHelper.escape("*") == r"\2a" + assert ldapHelper.escape("a)(b") == r"a\29\28b" + assert ldapHelper.escape("a\\b") == r"a\5cb" + # Non-str input is handled + assert ldapHelper.escape(123) == "123" diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/user.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/user.py index 6ed989b..f2050db 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/user.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/user.py @@ -12,7 +12,7 @@ def readAttributes(): if not user.isInAD(): return False, None - return ldapHelper.searchOne(f"(sAMAccountName={user.username()})") + return ldapHelper.searchOne(f"(sAMAccountName={ldapHelper.escape(user.username())})") def school(): """ From 874785a529c0ffd01a6d71fcd7e84f472d5af787 Mon Sep 17 00:00:00 2001 From: hermanntoast Date: Thu, 23 Jul 2026 12:29:38 +0200 Subject: [PATCH 2/9] Security: prevent path traversal / arbitrary CIFS mounts via sudoTools sudoTools is callable NOPASSWD by any member of 'domain users' and forwards the --path and --name arguments into shares._mountShare() as root. The share name was used unvalidated to build the mountpoint, so '--name ../../..' could escape the per-user base dir and mount an attacker-controlled server to an arbitrary location (root mkdir + mount.cifs). _mountShare() now validates the network path and share name and verifies the resolved mountpoint stays inside the user's base directory. Also fixes the sudoTools mount-share success check (a (False, None) tuple is truthy) and wraps mount.cifs in 'timeout' so an unreachable server cannot hang boot/login (part of #61). --- .../linuxmusterLinuxclient7/constants.py | 3 + .../linuxmusterLinuxclient7/shares.py | 58 ++++++++++++++++++- .../tests/test_shares.py | 42 ++++++++++++++ .../scripts/sudoTools | 6 +- 4 files changed, 106 insertions(+), 3 deletions(-) diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/constants.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/constants.py index f2dd633..27a8900 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/constants.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/constants.py @@ -9,6 +9,9 @@ shareMountBasepath = "/home/{}/media" hiddenShareMountBasepath = "/srv/samba/{}" +# Timeout (in seconds) for the mount.cifs call, so an unreachable server +# cannot block boot / login indefinitely (see issue #61). +mountTimeoutSeconds = 10 machineAccountSysvolMountPath = "/var/lib/samba/sysvol" defaultShareLetterTemplate = " ({letter}:)" diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/shares.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/shares.py index 2759759..4fa689e 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/shares.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/shares.py @@ -138,8 +138,28 @@ def getLocalSysvolPath(): # This parameter influences the `cruid` mount option. def _mountShare(username, networkPath, shareName, hiddenShare, useCruidOfExecutingUser=False): + # Normalize and validate the untrusted inputs before doing anything as root. + # _mountShare() is reachable by any domain user via the sudoTools helper, so + # unvalidated values would allow mounting an arbitrary CIFS server to an + # arbitrary location (path traversal via shareName). See security audit. + networkPath = networkPath.replace("\\", "/") + + if not _networkPathIsValid(networkPath): + logging.error(f"Refusing to mount: invalid network path '{networkPath}'") + return False, None + + if not _shareNameIsValid(shareName): + logging.error(f"Refusing to mount: invalid share name '{shareName}'") + return False, None + mountpoint = _getShareMountpoint(networkPath, username, hiddenShare, shareName) + # Defense in depth: make sure the resolved mountpoint really stays inside the + # per-user base directory and did not escape it through traversal. + if not _mountpointIsWithinBase(mountpoint, username, hiddenShare): + logging.error(f"Refusing to mount: mountpoint '{mountpoint}' escapes the allowed base directory") + return False, None + mountCommandOptions = f"file_mode=0700,dir_mode=0700,sec=krb5,nodev,nosuid,mfsymlinks,nobrl,vers=3.0,user={username}" rc, networkConfig = config.network() domain = None @@ -162,7 +182,10 @@ def _mountShare(username, networkPath, shareName, hiddenShare, useCruidOfExecuti gid = -1 logging.warning("Uid could not be found! Continuing anyway!") - mountCommand = [shutil.which("mount.cifs"), "-o", mountCommandOptions, networkPath, mountpoint] + mountBinary = shutil.which("mount.cifs") or "/usr/sbin/mount.cifs" + # Wrap the mount in `timeout` so an unreachable server cannot block + # boot / login indefinitely (issue #61). + mountCommand = ["timeout", str(constants.mountTimeoutSeconds), mountBinary, "-o", mountCommandOptions, networkPath, mountpoint] logging.debug(f"Trying to mount '{networkPath}' to '{mountpoint}'") logging.debug("* Creating directory...") @@ -259,3 +282,36 @@ def _getShareMountpoint(networkPath, username, hidden, shareName = None): def _directoryIsMountpoint(dir): return subprocess.call(["mountpoint", "-q", dir]) == 0 + +def _networkPathIsValid(networkPath): + """ + Validates a CIFS network path of the form `//server/share[/subdir...]`. + Rejects empty paths, path traversal and control characters. + """ + if not networkPath or "\x00" in networkPath or "\n" in networkPath: + return False + if ".." in networkPath.split("/"): + return False + return re.fullmatch(r"//[^/\x00]+/[^\x00]+", networkPath) is not None + +def _shareNameIsValid(shareName): + """ + Validates the share name, which becomes a single directory below the user's + mount base. It must therefore be a single path component without any slashes + or traversal. + """ + if not shareName or shareName in (".", ".."): + return False + if "/" in shareName or "\\" in shareName or "\x00" in shareName: + return False + return True + +def _mountpointIsWithinBase(mountpoint, username, hidden): + if hidden: + base = constants.hiddenShareMountBasepath.format(username) + else: + base = constants.shareMountBasepath.format(username) + + base = os.path.normpath(base) + normalized = os.path.normpath(mountpoint) + return normalized == base or normalized.startswith(base + "/") diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py index 9b6a44e..251001d 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py @@ -2,6 +2,48 @@ from unittest import mock from .. import shares + +def test_networkPathIsValid(): + assert shares._networkPathIsValid("//server/sysvol") + assert shares._networkPathIsValid("//server/share/subdir") + # invalid + assert not shares._networkPathIsValid("") + assert not shares._networkPathIsValid("/server/share") + assert not shares._networkPathIsValid("//server") + assert not shares._networkPathIsValid("//server/") + assert not shares._networkPathIsValid("//server/../etc") + assert not shares._networkPathIsValid("//server/share\nfoo") + + +def test_shareNameIsValid(): + assert shares._shareNameIsValid("sysvol") + assert shares._shareNameIsValid("My Share_1") + # invalid + assert not shares._shareNameIsValid("") + assert not shares._shareNameIsValid("..") + assert not shares._shareNameIsValid(".") + assert not shares._shareNameIsValid("../../etc/cron.d") + assert not shares._shareNameIsValid("a/b") + assert not shares._shareNameIsValid("a\\b") + + +def test_mountpointIsWithinBase(): + assert shares._mountpointIsWithinBase("/home/user1/media/share", "user1", False) + assert shares._mountpointIsWithinBase("/srv/samba/user1/share", "user1", True) + assert not shares._mountpointIsWithinBase("/home/user1/media/../../../etc/cron.d", "user1", False) + assert not shares._mountpointIsWithinBase("/etc/cron.d", "user1", False) + + +@mock.patch("subprocess.call") +def test_mountShareRejectsTraversal(mockSubprocessCall): + # A malicious --name (path traversal) must be rejected before anything runs as root. + assert shares._mountShare("user1", "//server/share", "../../../../etc/cron.d", False, False) == (False, None) + # A non-UNC / traversal network path must be rejected too. + assert shares._mountShare("user1", "//server/../etc", "share", False, False) == (False, None) + # The privileged mount command (wrapped in `timeout`) must never have run. + for call_args in mockSubprocessCall.call_args_list: + assert call_args.args[0][0] != "timeout" + @mock.patch("linuxmusterLinuxclient7.shares.mountShare") @mock.patch("linuxmusterLinuxclient7.shares.computer.hostname") @mock.patch("linuxmusterLinuxclient7.shares.user.isRoot") diff --git a/usr/share/linuxmuster-linuxclient7/scripts/sudoTools b/usr/share/linuxmuster-linuxclient7/scripts/sudoTools index 7459135..5dc8851 100755 --- a/usr/share/linuxmuster-linuxclient7/scripts/sudoTools +++ b/usr/share/linuxmuster-linuxclient7/scripts/sudoTools @@ -40,9 +40,11 @@ if task == "install-printer": sys.exit(0) else: sys.exit(1) - pass elif task == "mount-share": - if shares._mountShare(username, args.path, args.name, args.hidden, False): + # _mountShare() returns a (success, mountpoint) tuple; only the first + # element indicates success (a non-empty tuple is always truthy). + success, _ = shares._mountShare(username, args.path, args.name, args.hidden, False) + if success: sys.exit(0) else: sys.exit(1) From 8fd4aff6fd90fc4cd1cfcadf2e10e166b578abe8 Mon Sep 17 00:00:00 2001 From: hermanntoast Date: Thu, 23 Jul 2026 12:29:54 +0200 Subject: [PATCH 3/9] Security + Fix: validate printer args and use absolute lpadmin/lpstat paths (#93) install-printer (reachable as root via sudoTools by any domain user) now rejects unsafe device-uri schemes (notably file://, which would allow writing to an arbitrary file as root) and invalid printer names. Additionally resolve lpadmin/lpstat to absolute paths so printers are still uninstalled on logout, where the PAM environment only provides PATH=/bin:/usr/bin (#93). --- .../linuxmusterLinuxclient7/printers.py | 40 ++++++++++++++-- .../tests/test_printers.py | 47 ++++++++++++++++--- 2 files changed, 77 insertions(+), 10 deletions(-) diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/printers.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/printers.py index 8665058..d390650 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/printers.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/printers.py @@ -1,6 +1,17 @@ -import os, subprocess, re +import os, subprocess, re, shutil from linuxmusterLinuxclient7 import logging, user +# Resolve absolute paths at import time. When called from the logout PAM hook the +# PATH is reduced to /bin:/usr/bin, so lpadmin (/usr/sbin) is not found otherwise. +# See issue #93. +_lpadminPath = shutil.which("lpadmin") or "/usr/sbin/lpadmin" +_lpstatPath = shutil.which("lpstat") or "/usr/bin/lpstat" + +# CUPS device-uri schemes we allow to be installed via the (root) sudoTools helper. +# Notably excludes "file://", which would let an unprivileged domain user write to +# an arbitrary file as root. See security audit. +_allowedPrinterUriSchemes = ("ipp", "ipps", "http", "https", "smb", "lpd", "socket", "dnssd") + def installPrinter(networkPath, name=None, username=None): """ Installs a networked printer for a user @@ -70,8 +81,16 @@ def translateSambaToIpp(networkPath): # -------------------- def _installPrinter(username, networkPath, name): + if not _printerNetworkPathIsValid(networkPath): + logging.error(f"Refusing to install printer: invalid network path '{networkPath}'") + return False + + if not _printerNameIsValid(name): + logging.error(f"Refusing to install printer: invalid printer name '{name}'") + return False + logging.info(f"Install Printer {name} on {networkPath}") - installCommand = ["timeout", "10", "lpadmin", "-p", name, "-E", "-v", networkPath, "-m", "everywhere", "-u", f"allow:{username}"] + installCommand = ["timeout", "10", _lpadminPath, "-p", name, "-E", "-v", networkPath, "-m", "everywhere", "-u", f"allow:{username}"] logging.debug(f"* running '{' '.join(installCommand)}'") p = subprocess.call(installCommand, stdout=subprocess.PIPE, stderr=subprocess.PIPE) if p == 0: @@ -88,7 +107,7 @@ def _installPrinterWithoutRoot(networkPath, name): def _getInstalledPrintersOfUser(username): logging.info(f"Getting installed printers of {username}") - command = ["lpstat", "-U", username, "-p"] + command = [_lpstatPath, "-U", username, "-p"] #logging.debug(f"running '{command}'") result = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) @@ -113,7 +132,7 @@ def _getInstalledPrintersOfUser(username): def _uninstallPrinter(name): logging.info(f"Uninstall Printer {name}") - uninstallCommand = ["timeout", "10", "lpadmin", "-x", name] + uninstallCommand = ["timeout", "10", _lpadminPath, "-x", name] logging.debug(f"* running '{' '.join(uninstallCommand)}'") p = subprocess.call(uninstallCommand, stdout=subprocess.PIPE, stderr=subprocess.PIPE) if p == 0: @@ -124,3 +143,16 @@ def _uninstallPrinter(name): else: logging.fatal(f"* Error Uninstalling Printer {name}!") return False + +def _printerNetworkPathIsValid(networkPath): + if not networkPath or "\x00" in networkPath or "\n" in networkPath: + return False + match = re.match(r"^([a-zA-Z][a-zA-Z0-9+.-]*)://", networkPath) + if not match: + return False + return match.group(1).lower() in _allowedPrinterUriSchemes + +def _printerNameIsValid(name): + if not name: + return False + return re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._-]*", name) is not None diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_printers.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_printers.py index 5a86cee..54c1887 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_printers.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_printers.py @@ -2,6 +2,41 @@ from unittest import mock from .. import printers +def test_printerNetworkPathIsValid(): + assert printers._printerNetworkPathIsValid("ipp://server/printers/P1") + assert printers._printerNetworkPathIsValid("smb://server/P1") + assert printers._printerNetworkPathIsValid("https://server/printers/P1") + # dangerous / invalid schemes must be rejected + assert not printers._printerNetworkPathIsValid("file:///etc/passwd") + assert not printers._printerNetworkPathIsValid("/etc/passwd") + assert not printers._printerNetworkPathIsValid("") + assert not printers._printerNetworkPathIsValid("ipp://server/\nP1") + + +def test_printerNameIsValid(): + assert printers._printerNameIsValid("EW-farblaser") + assert printers._printerNameIsValid("printer1") + assert not printers._printerNameIsValid("") + assert not printers._printerNameIsValid("-flag") + assert not printers._printerNameIsValid("a/b") + assert not printers._printerNameIsValid("a b") + + +@mock.patch("subprocess.call") +@mock.patch("linuxmusterLinuxclient7.hooks.user.isRoot") +@mock.patch("linuxmusterLinuxclient7.hooks.user.username") +def test_installPrinterRejectsUnsafeInput(mockUserUsername, mockUserIsRoot, mockSubprocessCall): + mockUserIsRoot.return_value = True + mockUserUsername.return_value = "user1" + + # Unsafe device-uri scheme (file://) must be rejected without running lpadmin. + assert not printers.installPrinter("file:///etc/passwd", "printer1") + # Unsafe printer name must be rejected too. + assert not printers.installPrinter("ipp://server/printers/P1", "../evil") + # The privileged lpadmin command (wrapped in `timeout`) must never have run. + assert _getCallsTo(mockSubprocessCall, "timeout") == [] + + def test_translateSambaToIpp(): assert printers.translateSambaToIpp("\\\\linuxmuster.lan\\printer1") == (True, "ipp://linuxmuster.lan/printers/printer1") assert printers.translateSambaToIpp("\\\\linuxmuster.lan\\") == (False, None) @@ -18,13 +53,13 @@ def test_installPrinter(mockUserUsername, mockUserIsRoot, mockSubprocessCall): mockUserUsername.return_value = "user1" assert printers.installPrinter("ipp://linuxmuster.lan/printers/printer1") - assert _getCallsTo(mockSubprocessCall, "timeout")[-1] == ["timeout", "10", "lpadmin", "-p", "printer1", "-E", "-v", "ipp://linuxmuster.lan/printers/printer1", "-m", "everywhere", "-u", "allow:user1"] + assert _getCallsTo(mockSubprocessCall, "timeout")[-1] == ["timeout", "10", printers._lpadminPath, "-p", "printer1", "-E", "-v", "ipp://linuxmuster.lan/printers/printer1", "-m", "everywhere", "-u", "allow:user1"] assert printers.installPrinter("ipp://linuxmuster.lan/printers/printer1", "printer2") - assert _getCallsTo(mockSubprocessCall, "timeout")[-1] == ["timeout", "10", "lpadmin", "-p", "printer2", "-E", "-v", "ipp://linuxmuster.lan/printers/printer1", "-m", "everywhere", "-u", "allow:user1"] - + assert _getCallsTo(mockSubprocessCall, "timeout")[-1] == ["timeout", "10", printers._lpadminPath, "-p", "printer2", "-E", "-v", "ipp://linuxmuster.lan/printers/printer1", "-m", "everywhere", "-u", "allow:user1"] + assert printers.installPrinter("ipp://linuxmuster.lan/printers/printer1", "printer2", "user2") - assert _getCallsTo(mockSubprocessCall, "timeout")[-1] == ["timeout", "10", "lpadmin", "-p", "printer2", "-E", "-v", "ipp://linuxmuster.lan/printers/printer1", "-m", "everywhere", "-u", "allow:user2"] + assert _getCallsTo(mockSubprocessCall, "timeout")[-1] == ["timeout", "10", printers._lpadminPath, "-p", "printer2", "-E", "-v", "ipp://linuxmuster.lan/printers/printer1", "-m", "everywhere", "-u", "allow:user2"] mockUserIsRoot.return_value = False assert printers.installPrinter("ipp://linuxmuster.lan/printers/printer1") @@ -57,8 +92,8 @@ def test_uninstallAllPrintersOfUser(mockSubprocessRun, mockSubprocessCall): mockSubprocessRun.return_value = CompletedProcess(args=["lpstat", "-U", "user1", "-p"], returncode=0, stdout=lpstatStdout) assert printers.uninstallAllPrintersOfUser("user1") - assert _getCallsTo(mockSubprocessRun, "lpstat")[-1] == ['lpstat', '-U', 'user1', '-p'] - assert _getCallsTo(mockSubprocessCall, "timeout") == [["timeout", "10", "lpadmin", "-x", "printer1"], ["timeout", "10", "lpadmin", "-x", "printer2"]] + assert _getCallsTo(mockSubprocessRun, printers._lpstatPath)[-1] == [printers._lpstatPath, '-U', 'user1', '-p'] + assert _getCallsTo(mockSubprocessCall, "timeout") == [["timeout", "10", printers._lpadminPath, "-x", "printer1"], ["timeout", "10", printers._lpadminPath, "-x", "printer2"]] mockSubprocessRun.return_value = CompletedProcess(args=["lpstat", "-U", "user1", "-p"], returncode=1) assert printers.uninstallAllPrintersOfUser("user1") From 5359d2ade2692482f9e6bb33d978d84e7d1cd812 Mon Sep 17 00:00:00 2001 From: hermanntoast Date: Thu, 23 Jul 2026 12:29:54 +0200 Subject: [PATCH 4/9] Fix: don't let the onBoot hook block booting when the DC is unreachable (#61) pullKerberosTicketForComputerAccount() gained an optional timeout, and onBoot now pulls the ticket with a 5s timeout. Together with the mount.cifs timeout, an unreachable domain controller / sysvol no longer stalls the boot process. --- .../dist-packages/linuxmusterLinuxclient7/realm.py | 10 ++++++++-- .../linuxmusterLinuxclient7/tests/test_realm.py | 8 ++++++++ usr/share/linuxmuster-linuxclient7/scripts/onBoot | 5 ++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/realm.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/realm.py index f878586..e6bb13f 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/realm.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/realm.py @@ -69,14 +69,20 @@ def isJoined(): else: return len(joinedDomains) > 0 -def pullKerberosTicketForComputerAccount(): +def pullKerberosTicketForComputerAccount(timeoutSeconds=None): """ Pulls a kerberos ticket using the computer account from `/etc/krb5.keytab` + :param timeoutSeconds: If set, abort the kinit after this many seconds so an + unreachable domain controller cannot block the caller (e.g. boot). See issue #61. + :type timeoutSeconds: int, optional :return: True on success, False otherwise :rtype: bool """ - return subprocess.call(["kinit", "-k", computer.krbHostName()]) == 0 + command = ["kinit", "-k", computer.krbHostName()] + if timeoutSeconds is not None: + command = ["timeout", str(timeoutSeconds)] + command + return subprocess.call(command) == 0 def verifyDomainJoin(): """ diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_realm.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_realm.py index 979876d..c3d16c9 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_realm.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_realm.py @@ -80,6 +80,14 @@ def test_pullKerberosTicketForComputerAccount(mockSubprocessCall): mockSubprocessCall.return_value = 1 assert not realm.pullKerberosTicketForComputerAccount() +@mock.patch("subprocess.call") +@mock.patch("linuxmusterLinuxclient7.computer.krbHostName", lambda : "linuxmuster.lan") +def test_pullKerberosTicketForComputerAccountWithTimeout(mockSubprocessCall): + mockSubprocessCall.return_value = 0 + assert realm.pullKerberosTicketForComputerAccount(timeoutSeconds=5) + calls = _getCallsTo(mockSubprocessCall, "timeout") + assert ["timeout", "5", "kinit", "-k", "linuxmuster.lan"] in calls + @mock.patch("subprocess.call") @mock.patch("linuxmusterLinuxclient7.realm.isJoined") @mock.patch("linuxmusterLinuxclient7.realm.pullKerberosTicketForComputerAccount") diff --git a/usr/share/linuxmuster-linuxclient7/scripts/onBoot b/usr/share/linuxmuster-linuxclient7/scripts/onBoot index b91ffca..894e28a 100755 --- a/usr/share/linuxmuster-linuxclient7/scripts/onBoot +++ b/usr/share/linuxmuster-linuxclient7/scripts/onBoot @@ -11,7 +11,10 @@ try: logging.info("====== onBoot started ======") keytab.patchKeytab() - realm.pullKerberosTicketForComputerAccount() + # Use a timeout so an unreachable domain controller cannot block the boot + # process (see issue #61). The sysvol mount below is protected by its own + # timeout inside shares._mountShare(). + realm.pullKerberosTicketForComputerAccount(timeoutSeconds=5) # mount sysvol rc, sysvolPath = shares.getLocalSysvolPath() From 867ff359e7a073a73fcf5d70a5933f90606dcf38 Mon Sep 17 00:00:00 2001 From: hermanntoast Date: Thu, 23 Jul 2026 12:30:05 +0200 Subject: [PATCH 5/9] Compat: support Ubuntu 26.04 / Samba 4.2x Drop the deprecated 'client use spnego' smb.conf option (SPNEGO is always used in modern Samba and the option triggers warnings), and make SSSD version parsing tolerant of packaging suffixes (e.g. '2.10.0-1ubuntu1') so the krb5_child capability check keeps working on newer distributions. --- .../linuxmusterLinuxclient7/setup.py | 7 ++++++- .../tests/test_setup.py | 18 ++++++++++++++++++ .../templates/smb.conf | 3 ++- 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_setup.py diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/setup.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/setup.py index 836a7db..16bf0a8 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/setup.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/setup.py @@ -374,7 +374,12 @@ def _deleteObsoleteFiles(): return True def _parseVersion(versionString): - return tuple(int(p) for p in versionString.split(".")) + # Only look at the leading dotted-number part so packaging suffixes emitted by + # newer distributions (e.g. "2.10.0-1ubuntu1" on Ubuntu 26.04) do not break parsing. + match = re.match(r"\d+(?:\.\d+)*", versionString.strip()) + if not match: + raise ValueError(f"Could not parse version string: {versionString!r}") + return tuple(int(p) for p in match.group(0).split(".")) def _getSssdVersion(): try: diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_setup.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_setup.py new file mode 100644 index 0000000..aa7957e --- /dev/null +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_setup.py @@ -0,0 +1,18 @@ +import pytest +from .. import setup + + +def test_parseVersion(): + assert setup._parseVersion("2.9.5") == (2, 9, 5) + assert setup._parseVersion("2.10.0") == (2, 10, 0) + # packaging suffixes emitted by newer distributions must not break parsing + assert setup._parseVersion("2.10.0-1ubuntu1") == (2, 10, 0) + assert setup._parseVersion(" 2.9.4 \n") == (2, 9, 4) + with pytest.raises(ValueError): + setup._parseVersion("not-a-version") + + +def test_versionComparison(): + # the capability check compares against 2.9.5 + assert setup._parseVersion("2.10.0") > setup._parseVersion("2.9.5") + assert setup._parseVersion("2.9.4") < setup._parseVersion("2.9.5") diff --git a/usr/share/linuxmuster-linuxclient7/templates/smb.conf b/usr/share/linuxmuster-linuxclient7/templates/smb.conf index aac5bec..6d3abd9 100644 --- a/usr/share/linuxmuster-linuxclient7/templates/smb.conf +++ b/usr/share/linuxmuster-linuxclient7/templates/smb.conf @@ -5,7 +5,8 @@ [global] client signing = yes -client use spnego = yes +# "client use spnego" was removed as it is deprecated in modern Samba (>= 4.x); +# SPNEGO is always used and the option triggers warnings on Ubuntu 26.04 / Samba 4.2x. kerberos method = secrets and keytab security = user tls verify peer = ca_and_name From 494b694cf5ca7e5afea5fbbbe067dfe1a9f14037 Mon Sep 17 00:00:00 2001 From: hermanntoast Date: Thu, 23 Jul 2026 12:30:05 +0200 Subject: [PATCH 6/9] Feat: create user homedirs with umask=077 (#44) New homes are created mode 0700 so users can no longer read each other's data via an explicit path. Incorporates PR #44 by dorianim. --- usr/share/linuxmuster-linuxclient7/templates/common-session | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/usr/share/linuxmuster-linuxclient7/templates/common-session b/usr/share/linuxmuster-linuxclient7/templates/common-session index 970624b..7ebd63e 100644 --- a/usr/share/linuxmuster-linuxclient7/templates/common-session +++ b/usr/share/linuxmuster-linuxclient7/templates/common-session @@ -20,8 +20,9 @@ session [default=1] pam_permit.so # here's the fallback if no module succeeds session requisite pam_deny.so -## linuxmuster-linuxclient7: mount the homedir first using requisite -session requisite pam_mkhomedir.so skel=@@userTemplateDir@@ +## linuxmuster-linuxclient7: create the userhome using requisite +## umask=077 makes new homes mode 0700 so users cannot read each other's data (PR #44) +session requisite pam_mkhomedir.so umask=077 skel=@@userTemplateDir@@ ## linuxmuster-linuxclient7: exec more scripts as root using requisite session requisite pam_exec.so @@hookScriptLoginLogoutAsRoot@@ From cc6a1aff07c271ecd640cab7237c2a30453bf969 Mon Sep 17 00:00:00 2001 From: hermanntoast Date: Thu, 23 Jul 2026 12:30:05 +0200 Subject: [PATCH 7/9] Chore: changelog for 7.4.0 (lmn74) Version scheme switched to 7.4.x so the linuxmuster (lmn) version is obvious from the package version. --- debian/changelog | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/debian/changelog b/debian/changelog index 82d59e5..1f14a14 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,19 @@ +linuxmuster-linuxclient7 (7.4.0) lmn74; urgency=high + + * Security: validate the path and share name passed to the sudoTools mount-share + helper to prevent path traversal / arbitrary CIFS mounts as root + * Security: restrict install-printer to safe device-uri schemes and validate the + printer name (sudoTools is callable NOPASSWD by any domain user) + * Security: escape all values interpolated into LDAP search filters to prevent + LDAP filter injection + * Feat: create user homedirs with umask=077 so users cannot read each other's data (#44) + * Fix: printers are not uninstalled on logout because lpadmin is not in PATH (#93) + * Fix: onBoot hook could block the boot process when the DC/sysvol is unreachable (#61) + * Compat: support Ubuntu 26.04 / Samba 4.2x (drop deprecated "client use spnego", + robust SSSD version parsing, timeout on mount.cifs) + + -- linuxmuster.net Thu, 23 Jul 2026 12:00:00 +0200 + linuxmuster-linuxclient7 (1.1.4) lmn73; urgency=medium * Fix: regressions in setup and prepareImage (#91) From 315236221f230a1d73ffbbad29799375ae2f1e39 Mon Sep 17 00:00:00 2001 From: hermanntoast Date: Thu, 23 Jul 2026 12:40:15 +0200 Subject: [PATCH 8/9] Test: cover mount execution path, containment check and LDAP filter escaping --- .../tests/test_ldapHelper.py | 11 ++++++ .../tests/test_shares.py | 34 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py index f3d9741..a565420 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py @@ -1,3 +1,4 @@ +from unittest import mock from .. import ldapHelper @@ -10,3 +11,13 @@ def test_escape(): assert ldapHelper.escape("a\\b") == r"a\5cb" # Non-str input is handled assert ldapHelper.escape(123) == "123" + + +@mock.patch("linuxmusterLinuxclient7.ldapHelper.searchOne") +def test_isObjectInGroupEscapesFilter(mockSearchOne): + mockSearchOne.return_value = (True, {}) + # A crafted group name with filter metacharacters must be escaped, not injected. + assert ldapHelper.isObjectInGroup("cn=user,dc=x", "domain users)(cn=*") + usedFilter = mockSearchOne.call_args.args[0] + assert "*" not in usedFilter + assert r"\2a" in usedFilter diff --git a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py index 251001d..9208df7 100644 --- a/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py @@ -44,6 +44,40 @@ def test_mountShareRejectsTraversal(mockSubprocessCall): for call_args in mockSubprocessCall.call_args_list: assert call_args.args[0][0] != "timeout" + +@mock.patch("builtins.open", new_callable=mock.mock_open) +@mock.patch("subprocess.call") +@mock.patch("linuxmusterLinuxclient7.shares._directoryIsMountpoint") +@mock.patch("linuxmusterLinuxclient7.shares.Path") +@mock.patch("linuxmusterLinuxclient7.shares.pwd.getpwnam") +@mock.patch("linuxmusterLinuxclient7.shares.config.network") +def test_mountShareValidPathMounts(mockNetwork, mockGetpwnam, mockPath, mockIsMountpoint, mockSubprocessCall, mockOpen): + mockNetwork.return_value = (True, {"domain": "linuxmuster.lan"}) + mockGetpwnam.return_value = mock.Mock(pw_uid=1001, pw_gid=1001) + mockIsMountpoint.return_value = False + mockSubprocessCall.return_value = 0 + + rc, mountpoint = shares._mountShare("user1", "//server/share", "myshare", True, False) + assert rc is True + assert mountpoint == "/srv/samba/user1/myshare" + + mountCalls = [c.args[0] for c in mockSubprocessCall.call_args_list if c.args[0][0] == "timeout"] + assert len(mountCalls) == 1 + # mount.cifs is wrapped in `timeout` and mounts to the validated mountpoint + assert mountCalls[0][0:2] == ["timeout", "10"] + assert mountCalls[0][-2:] == ["//server/share", "/srv/samba/user1/myshare"] + + +@mock.patch("subprocess.call") +@mock.patch("linuxmusterLinuxclient7.shares._getShareMountpoint") +def test_mountShareRejectsMountpointOutsideBase(mockGetShareMountpoint, mockSubprocessCall): + # Even if the mountpoint calculation were to escape the base directory, + # the containment check must refuse the mount (defense in depth). + mockGetShareMountpoint.return_value = "/etc/cron.d" + assert shares._mountShare("user1", "//server/share", "share", False, False) == (False, None) + for call_args in mockSubprocessCall.call_args_list: + assert call_args.args[0][0] != "timeout" + @mock.patch("linuxmusterLinuxclient7.shares.mountShare") @mock.patch("linuxmusterLinuxclient7.shares.computer.hostname") @mock.patch("linuxmusterLinuxclient7.shares.user.isRoot") From 5cf4b869fad37ab04c31b544602ac59ea9a9a3af Mon Sep 17 00:00:00 2001 From: hermanntoast Date: Fri, 24 Jul 2026 11:35:50 +0200 Subject: [PATCH 9/9] Chore: set changelog maintainer for 7.4.0 release --- debian/changelog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/changelog b/debian/changelog index 1f14a14..ea5079d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -12,7 +12,7 @@ linuxmuster-linuxclient7 (7.4.0) lmn74; urgency=high * Compat: support Ubuntu 26.04 / Samba 4.2x (drop deprecated "client use spnego", robust SSSD version parsing, timeout on mount.cifs) - -- linuxmuster.net Thu, 23 Jul 2026 12:00:00 +0200 + -- Lukas Lukic-Spitznagel Thu, 23 Jul 2026 12:00:00 +0200 linuxmuster-linuxclient7 (1.1.4) lmn73; urgency=medium