diff --git a/debian/changelog b/debian/changelog index 82d59e5..ea5079d 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) + + -- Lukas Lukic-Spitznagel Thu, 23 Jul 2026 12:00:00 +0200 + linuxmuster-linuxclient7 (1.1.4) lmn73; urgency=medium * Fix: regressions in setup and prepareImage (#91) 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/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/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/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/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/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/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_ldapHelper.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py new file mode 100644 index 0000000..a565420 --- /dev/null +++ b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_ldapHelper.py @@ -0,0 +1,23 @@ +from unittest import mock +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" + + +@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_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") 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/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/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py b/usr/lib/python3/dist-packages/linuxmusterLinuxclient7/tests/test_shares.py index 9b6a44e..9208df7 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,82 @@ 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("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") 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(): """ 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() 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) 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@@ 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