diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 6379b82ce8..f9f8344052 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -28,6 +28,7 @@ on: - 'Tests/test_stdin_password.py' - 'Tests/test_mount_helper.py' - 'Tests/test_linux_mount_point_protection.py' + - 'Tests/test_linux_parallel_mounts.py' - 'Tests/test.*.hc' - 'Tests/test_volume_read.py' - 'Tests/volume_read_test.cpp' @@ -64,6 +65,7 @@ on: - 'Tests/test_stdin_password.py' - 'Tests/test_mount_helper.py' - 'Tests/test_linux_mount_point_protection.py' + - 'Tests/test_linux_parallel_mounts.py' - 'Tests/test.*.hc' - 'Tests/test_volume_read.py' - 'Tests/volume_read_test.cpp' @@ -221,6 +223,7 @@ jobs: sudo python3 Tests/test_stdin_password.py --require-locales sudo python3 Tests/test_mount_helper.py --helper /usr/sbin/mount.veracrypt sudo python3 Tests/test_linux_mount_point_protection.py + sudo python3 Tests/test_linux_parallel_mounts.py sudo apt remove -y veracrypt - name: Install and test VeraCrypt Console .deb packages diff --git a/Tests/linux_mount_points_test.cpp b/Tests/linux_mount_points_test.cpp index 3073a618a8..633658e65a 100644 --- a/Tests/linux_mount_points_test.cpp +++ b/Tests/linux_mount_points_test.cpp @@ -77,6 +77,8 @@ struct TestCore : CoreLinux MountedFilesystemList GetMountedFilesystems (const DevicePath & = DevicePath(), const DirectoryPath & = DirectoryPath()) const override { return MountedFilesystemList(); } string GetTempDirectory () const override { return Root; } + // The production locks, in a directory that the unprivileged test can create + string GetMountLockDirectory () const override { return Root + "/locks"; } shared_ptr GetMountedVolume (const VolumePath &) const override { shared_ptr info (new VolumeInfo); diff --git a/Tests/linux_mount_read_only_test.cpp b/Tests/linux_mount_read_only_test.cpp index 9fc3f2892f..7f0ed2b378 100644 --- a/Tests/linux_mount_read_only_test.cpp +++ b/Tests/linux_mount_read_only_test.cpp @@ -133,6 +133,8 @@ struct FallbackCore : TestCore bool, VolumeType::Enum, bool, bool) const override { return Opened; } bool IsVolumeMounted (const VolumePath &) const override { return false; } VolumeInfoList GetMountedVolumes (const VolumePath & = VolumePath()) const override { return VolumeInfoList(); } + // The production locks, in a directory that the unprivileged test can create + string GetMountLockDirectory () const override { return GetTempDirectory() + "/locks"; } shared_ptr GetMountedVolume (const VolumePath &) const override { return shared_ptr (new VolumeInfo); diff --git a/Tests/test_linux_parallel_mounts.py b/Tests/test_linux_parallel_mounts.py new file mode 100644 index 0000000000..0b798e7ccc --- /dev/null +++ b/Tests/test_linux_parallel_mounts.py @@ -0,0 +1,230 @@ +#!/usr/bin/env python3 +"""Regression test for issue #1960, against the installed veracrypt. + +Usage (as root, after installing a package): + python3 Tests/test_linux_parallel_mounts.py [--veracrypt PATH] [--rounds N] + +Mounts two small volumes at the same time, repeatedly, and checks that both +mounts succeed and each volume appears in its own slot at its own mount +point: + +- with --slot and a mount directory, through FUSE (nokernelcrypto); +- without --slot, with a mount directory, through FUSE; +- without --slot or a mount directory, so that each mount chooses a free + slot and its default mount point, through FUSE and with kernel crypto + (counted only if the volumes use device-mapper devices). + +Also checks that occupying all names of the former auxiliary mount points +(.veracrypt_aux_mnt1 to 256, in a private TMPDIR) does not prevent +mounting. Only the volumes created by the test are dismounted; if that +fails, its files are kept. +""" + +import argparse +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +PASSWORDS = {"a": "regression-test-password-a", + "b": "regression-test-password-b"} +# The default PIM, so that opening a volume takes long enough for two mounts +# to overlap, as with ordinary volumes. +COMMON = ["--pim=0", "--keyfiles=", "--hash=sha512", "--protect-hidden=no"] +FUSE = ["--mount-options=nokernelcrypto"] + + +class Checks: + def __init__(self): + self.failures = 0 + + def check(self, desc, cond, detail=""): + print(("ok " if cond else "FAIL ") + desc, flush=True) + if not cond: + self.failures += 1 + if detail: + print(" " + detail.strip().replace("\n", "\n ")) + + +def text(result): + return (result.stdout or b"").decode("utf-8", "replace") + + +class VeraCrypt: + def __init__(self, path, work): + self.path = path + self.work = work + + def command(self, args): + return [self.path, "--text"] + args + + def run(self, args, env=None, timeout=300): + return subprocess.run(self.command(args), stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, cwd=self.work, + env=env, timeout=timeout) + + def mount_args(self, name, options, mount_point=None, slot=None): + args = ["--non-interactive", "-p", PASSWORDS[name]] + COMMON + options + if slot: + args.append("--slot=%d" % slot) + args.append(str(self.work / ("%s.hc" % name))) + if mount_point: + args.append(str(mount_point)) + return args + + def listed_volumes(self): + """(slot, volume, device, mount point) of each mounted volume; + raises if the volumes cannot be listed.""" + r = self.run(["--list"]) + if r.returncode != 0: + if "No volumes mounted" in text(r): + return [] + raise RuntimeError("veracrypt --list failed: " + text(r)) + volumes = [] + for line in text(r).splitlines(): + fields = line.split() + if len(fields) >= 4 and fields[0].endswith(":"): + volumes.append((fields[0][:-1], fields[1], fields[2], + fields[-1])) + return volumes + + def dismount_volumes(self, volumes): + """Dismounts only these volumes; returns whether none remains.""" + paths = {str(volume) for volume in volumes} + for _, path, _, _ in self.listed_volumes(): + if path in paths: + self.run(["--non-interactive", "-d", path]) + return not any(path in paths for _, path, _, _ in + self.listed_volumes()) + + +def parallel_rounds(vc, c, desc, rounds, requests, kernel_crypto=False): + """Mounts both volumes at the same time, 'rounds' times. 'requests' + gives the mount arguments of each volume.""" + volumes = [str(vc.work / ("%s.hc" % name)) for name in requests] + failed = [] + fallback = False + for _ in range(rounds): + jobs = [subprocess.Popen(vc.command(args), stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, cwd=vc.work) + for args in requests.values()] + results = [(job.communicate(timeout=300)[0].decode( + "utf-8", "replace"), job.returncode) for job in jobs] + try: + listed = vc.listed_volumes() + except RuntimeError as e: + failed.append(str(e)) + break + ours = [entry for entry in listed if entry[1] in volumes] + markers = [] + for _, path, _, mount_point in ours: + try: + markers.append((Path(path).stem, (Path(mount_point) + / "marker").read_text().strip())) + except OSError: + markers.append((Path(path).stem, None)) + if (any(code != 0 for _, code in results) or len(ours) != 2 + or len({slot for slot, _, _, _ in ours}) != 2 + or len({point for _, _, _, point in ours}) != 2 + or any(name != marker for name, marker in markers)): + failed.append("mounted: %s, markers: %s\n%s" % ( + ours, markers, "\n".join(output for output, _ in results))) + if kernel_crypto and not all(device.startswith("/dev/mapper/") + for _, _, device, _ in ours): + fallback = True + # A mount left from this round must not supply the next one + if not vc.dismount_volumes(volumes): + failed.append("the volumes could not be dismounted") + break + if fallback and not failed: + print("skip #1960: %s: kernel crypto was not used" % desc) + return + c.check("#1960: %s" % desc, not failed, "\n".join(failed[:3])) + + +def test_old_aux_names(vc, c): + # All names that earlier versions tried, so that skipping a blocked one + # does not help + temporary = vc.work / "tmp" + temporary.mkdir() + for i in range(1, 257): + old = temporary / (".veracrypt_aux_mnt%d" % i) + old.mkdir() + (old / "file").write_text("x") + os.chown(old, 65534, 65534) + env = dict(os.environ, TMPDIR=str(temporary)) + r = vc.run(vc.mount_args("a", FUSE, vc.work / "a"), env=env) + c.check("#1960: mounting with all former auxiliary mount point names " + "taken", r.returncode == 0, text(r)) + c.check("#1960: dismount", vc.dismount_volumes([vc.work / "a.hc"])) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--veracrypt", + default=shutil.which("veracrypt") or "veracrypt") + parser.add_argument("--rounds", type=int, default=10, + help="rounds of parallel mounts of each kind") + args = parser.parse_args() + if os.geteuid() != 0: + sys.exit("Run as root: the test mounts volumes") + + work = Path(tempfile.mkdtemp(prefix="vc-regression-")) + vc = VeraCrypt(args.veracrypt, work) + c = Checks() + mount_points = [work / name for name in ("a", "b")] + for mount_point in mount_points: + mount_point.mkdir() + (work / "random").write_bytes(os.urandom(4096)) + volumes = [work / "a.hc", work / "b.hc"] + try: + for name in PASSWORDS: + r = vc.run(["--non-interactive", "--create", + str(work / ("%s.hc" % name)), "--size=10M", + "--volume-type=normal", "--encryption=AES", + "--hash=SHA-512", "--filesystem=ext4", "-p", + PASSWORDS[name], "--pim=0", "--keyfiles=", + "--random-source=" + str(work / "random")]) + c.check("create %s.hc" % name, r.returncode == 0, text(r)) + r = vc.run(vc.mount_args(name, FUSE, mount_points[0])) + if r.returncode == 0: + (mount_points[0] / "marker").write_text(name) + c.check("write the marker of %s.hc" % name, + r.returncode == 0 and vc.dismount_volumes(volumes), + text(r)) + if c.failures: + sys.exit("Cannot create the test volumes") + + parallel_rounds(vc, c, "%d rounds with --slot" % args.rounds, + args.rounds, + {"a": vc.mount_args("a", FUSE, mount_points[0], 1), + "b": vc.mount_args("b", FUSE, mount_points[1], 2)}) + parallel_rounds(vc, c, "%d rounds without --slot, with mount " + "directories" % args.rounds, args.rounds, + {"a": vc.mount_args("a", FUSE, mount_points[0]), + "b": vc.mount_args("b", FUSE, mount_points[1])}) + parallel_rounds(vc, c, "%d rounds without --slot or mount directory " + "(FUSE)" % args.rounds, args.rounds, + {"a": vc.mount_args("a", FUSE), + "b": vc.mount_args("b", FUSE)}) + parallel_rounds(vc, c, "%d rounds without --slot or mount directory " + "(kernel crypto)" % args.rounds, args.rounds, + {"a": vc.mount_args("a", []), + "b": vc.mount_args("b", [])}, kernel_crypto=True) + test_old_aux_names(vc, c) + finally: + # Only the volumes of this test: unrelated ones may be mounted + if (not vc.dismount_volumes(volumes) + or any(os.path.ismount(m) for m in mount_points)): + sys.exit("A test volume is still mounted; not removing %s" % work) + shutil.rmtree(work, ignore_errors=True) + + if c.failures: + sys.exit("%d check(s) failed" % c.failures) + print("All checks passed") + + +if __name__ == "__main__": + main() diff --git a/src/Core/Unix/CoreUnix.cpp b/src/Core/Unix/CoreUnix.cpp index 6055c4c8e6..fe78f4c32d 100644 --- a/src/Core/Unix/CoreUnix.cpp +++ b/src/Core/Unix/CoreUnix.cpp @@ -25,6 +25,8 @@ #include #endif #ifdef TC_LINUX +#include +#include #include #endif #include @@ -1245,9 +1247,112 @@ namespace VeraCrypt throw MountPointNotAllowed (SRC_POS); } +#ifdef TC_LINUX + // Opens the lock file 'name' in the lock directory, which must belong to the + // effective user (root, /var/run/veracrypt) and be writable only by it. The + // locks are taken with flock, so they last until the last copy of the + // descriptor is closed: the copy that the FUSE service inherits lasts until it + // exits. + static int OpenMountLockFile (const string &lockDirectory, const string &name) + { + throw_sys_sub_if (mkdir (lockDirectory.c_str(), S_IRWXU) == -1 && errno != EEXIST, lockDirectory); + + // Keep using the directory that was checked + int directoryFd = open (lockDirectory.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + throw_sys_sub_if (directoryFd == -1, lockDirectory); + finally_do_arg (int, directoryFd, { close (finally_arg); }); + + struct stat directoryInfo; + throw_sys_sub_if (fstat (directoryFd, &directoryInfo) == -1, lockDirectory); + if (directoryInfo.st_uid != geteuid() || (directoryInfo.st_mode & (S_IWGRP | S_IWOTH))) + { + errno = EPERM; + throw SystemException (SRC_POS, lockDirectory + ": the directory must belong to the user running VeraCrypt and be writable only by it."); + } + + int fd = openat (directoryFd, name.c_str(), O_RDWR | O_CREAT | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR); + throw_sys_sub_if (fd == -1, lockDirectory + "/" + name); + return fd; + } + // Reserves the slot of the mount, whose number also names its default mount + // point and device-mapper device, until the FUSE service exits: parallel mounts + // could otherwise choose the same free slot. The slot is reserved before its + // availability is checked, so that a mount published meanwhile is seen. An + // automatically chosen slot that is taken is replaced by the next free one. + // Returns the descriptor holding the lock. + static int ReserveSlotForMount (const CoreUnix &core, MountOptions &options, bool slotChosen, bool mountPointFromSlot) + { + VolumeSlotNumber slot = slotChosen ? core.GetFirstFreeSlotNumber() : options.SlotNumber; + + for (int t = 0; true; ) + { + stringstream lockName; + lockName << "slot-" << slot; + + int fd = OpenMountLockFile (core.GetMountLockDirectory(), lockName.str()); + bool reserved = false; + finally_do_arg2 (int, fd, bool &, reserved, { if (!finally_arg2) close (finally_arg); }); + + if (flock (fd, LOCK_EX | LOCK_NB) == 0) + { + if (core.IsSlotNumberAvailable (slot)) + { + options.SlotNumber = slot; + if (mountPointFromSlot) + options.MountPoint.reset (new DirectoryPath (core.SlotNumberToMountPoint (slot))); + + reserved = true; + return fd; + } + + if (!slotChosen) + throw VolumeSlotUnavailable (SRC_POS); + } + else + { + const int error = errno; + if (error == EINTR) + continue; + + if (error != EWOULDBLOCK) + { + errno = error; + throw SystemException (SRC_POS, lockName.str()); + } + + if (!slotChosen) + { + // A service that has just been dismounted normally may still be exiting + if (++t > 100) + throw VolumeSlotUnavailable (SRC_POS); + + Thread::Sleep (100); + continue; + } + } + + // Taken by another mount: try the next free slot + slot = core.GetFirstFreeSlotNumber (slot + 1); + } + } +#endif + shared_ptr CoreUnix::MountVolume (MountOptions &options) { +#ifdef TC_LINUX + // A slot is chosen automatically without --slot, also for a custom mount + // directory, unless a default mount point names the slot + const bool mountPointGiven = options.MountPoint && !options.MountPoint->IsEmpty(); + const VolumeSlotNumber mountPointSlot = mountPointGiven ? GetDefaultMountPointSlot (*options.MountPoint) : 0; + const bool slotChosen = options.SlotNumber < GetFirstSlotNumber() && mountPointSlot == 0; + if (options.SlotNumber < GetFirstSlotNumber() && mountPointSlot != 0) + options.SlotNumber = mountPointSlot; + + int slotLock = ReserveSlotForMount (*this, options, slotChosen, !options.NoFilesystem && !mountPointGiven); + finally_do_arg (int, slotLock, { close (finally_arg); }); +#else CoalesceSlotNumberAndMountPoint (options); +#endif if (IsVolumeMounted (*options.Path)) throw VolumeAlreadyMounted (SRC_POS); @@ -1331,44 +1436,15 @@ namespace VeraCrypt throw_sys_if (mkdtemp (&mountDirectory[0]) == NULL); fuseMountPoint = &mountDirectory[0]; #else - // Find a free mount point for FUSE service - MountedFilesystemList mountedFilesystems = GetMountedFilesystems (); - for (int i = 1; true; i++) - { - stringstream path; - path << GetTempDirectory() << "/" << GetFuseMountDirPrefix() << i; - FilesystemPath fsPath (path.str()); - - bool inUse = false; - - foreach_ref (const MountedFilesystem &mf, mountedFilesystems) - { - if (mf.MountPoint == path.str()) - { - inUse = true; - break; - } - } - - if (!inUse) - { - try - { - if (fsPath.IsDirectory()) - fsPath.Delete(); - - throw_sys_sub_if (mkdir (path.str().c_str(), S_IRUSR | S_IXUSR) == -1, path.str()); - - fuseMountPoint = fsPath; - break; - } - catch (...) - { - if (i > 255) - throw TemporaryDirectoryFailure (SRC_POS, StringConverter::ToWide (path.str())); - } - } - } + // Create a new directory with an unpredictable name. Numbered names were + // reused by parallel mounts, and any local user could create them all in + // advance to block mounting. + string mountTemplate = GetTempDirectory() + "/" + GetFuseMountDirPrefix() + "-XXXXXX"; + vector mountDirectory (mountTemplate.begin(), mountTemplate.end()); + mountDirectory.push_back ('\0'); + if (mkdtemp (&mountDirectory[0]) == NULL) + throw TemporaryDirectoryFailure (SRC_POS, StringConverter::ToWide (mountTemplate)); + fuseMountPoint = &mountDirectory[0]; #endif #ifdef VC_MACOSX_FUSET diff --git a/src/Core/Unix/CoreUnix.h b/src/Core/Unix/CoreUnix.h index 968a7c2d4d..c621a63182 100644 --- a/src/Core/Unix/CoreUnix.h +++ b/src/Core/Unix/CoreUnix.h @@ -43,6 +43,8 @@ namespace VeraCrypt virtual bool IsDevicePresent (const DevicePath &device) const { throw NotApplicable (SRC_POS); } virtual bool IsInPortableMode () const { return false; } virtual bool IsMountPointAvailable (const DirectoryPath &mountPoint) const; + // Directory of the locks that reserve volumes and slots while mounted + virtual string GetMountLockDirectory () const { return "/var/run/veracrypt"; } virtual bool IsOSVersion (int major, int minor) const { throw NotApplicable (SRC_POS); } virtual bool IsOSVersionLower (int major, int minor) const { throw NotApplicable (SRC_POS); } virtual bool IsPasswordCacheEmpty () const { throw NotApplicable (SRC_POS); } diff --git a/src/Main/UserInterface.cpp b/src/Main/UserInterface.cpp index da14623e8a..ef390ef70a 100644 --- a/src/Main/UserInterface.cpp +++ b/src/Main/UserInterface.cpp @@ -765,6 +765,8 @@ namespace VeraCrypt if (!options.MountPoint) options.MountPoint.reset (new DirectoryPath); + // Without --slot, the core chooses the slot of each mount + const bool slotChosen = options.SlotNumber < Core->GetFirstSlotNumber(); Core->CoalesceSlotNumberAndMountPoint (options); bool sharedAccessAllowed = options.SharedAccessAllowed; @@ -796,7 +798,7 @@ namespace VeraCrypt continue; Yield(); - options.SlotNumber = Core->GetFirstFreeSlotNumber (options.SlotNumber); + options.SlotNumber = slotChosen ? 0 : Core->GetFirstFreeSlotNumber (options.SlotNumber); options.MountPoint.reset (new DirectoryPath); options.Path.reset (new VolumePath (device.Path));