From cc62fe5159d871e5b89394a4dd6301c7a3063450 Mon Sep 17 00:00:00 2001 From: Ville Takio Date: Tue, 27 Oct 2026 01:50:43 +0200 Subject: [PATCH 1/6] Unix: wipe the hidden volume protection password after opening the volume MountVolume released the outer volume password once the volume was open, but kept the protection password referenced by the mount request. The forked FUSE service inherited it, so the hidden volume password stayed in plaintext in both service processes until dismount. Release it at the same points as the outer volume password. Fixes #1961 --- src/Core/Unix/CoreUnix.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Core/Unix/CoreUnix.cpp b/src/Core/Unix/CoreUnix.cpp index b28f9b6b34..6569b1bad2 100644 --- a/src/Core/Unix/CoreUnix.cpp +++ b/src/Core/Unix/CoreUnix.cpp @@ -1234,7 +1234,10 @@ namespace VeraCrypt options.PartitionInSystemEncryptionScope ); + // The volume no longer needs the passwords; do not keep them while + // the service, and the process forked for it, stay running. options.Password.reset(); + options.ProtectionPassword.reset(); } catch (SystemException &e) { @@ -1247,6 +1250,7 @@ namespace VeraCrypt } options.Password.reset(); + options.ProtectionPassword.reset(); throw; } From 4fc2edecd2fff945797dd047887d22c84bd38b63 Mon Sep 17 00:00:00 2001 From: Ville Takio Date: Tue, 27 Oct 2026 01:51:56 +0200 Subject: [PATCH 2/6] Unix: ask before overwriting keyfiles and header backups in text mode --create-keyfile and --backup-headers opened the destination with O_CREAT|O_TRUNC without checking whether it existed. Recreating an existing keyfile replaced it with new random data, so every volume that used it could no longer be mounted, and an existing file chosen for a header backup was truncated. Ask before replacing an existing file, as the graphical interface does, and as text mode already does for new containers (4edad72b). With --non-interactive the operation fails instead, and --force overwrites without asking. The keyfile prompt is shown before the random pool is enriched, so a refusal does not waste that input. Fixes #1962 --- src/Main/TextUserInterface.cpp | 39 +++++++++++++++++++++++++--------- src/Main/TextUserInterface.h | 1 + 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/Main/TextUserInterface.cpp b/src/Main/TextUserInterface.cpp index 7f00579556..7b87a80c08 100644 --- a/src/Main/TextUserInterface.cpp +++ b/src/Main/TextUserInterface.cpp @@ -460,6 +460,9 @@ namespace VeraCrypt if (filePath.IsEmpty()) throw UserAbort (SRC_POS); + // Same prompt as the file dialog of the graphical interface + ConfirmFileOverwrite (filePath, wxString::Format (_("File '%s' already exists, do you really want to overwrite it?"), wstring (filePath).c_str())); + File backupFile; backupFile.Open (filePath, File::CreateWrite); @@ -617,28 +620,44 @@ namespace VeraCrypt ShowInfo ("PASSWORD_CHANGED"); } + void TextUserInterface::ConfirmFileOverwrite (const FilePath &path, const wxString &message) const + { + // Replacing a keyfile or header backup cannot be undone. As in the + // graphical interface, ask first; --force overwrites without asking. + if (CmdLine->ArgForce || !FilesystemPath (wstring (path)).IsFile()) + return; + + if (Preferences.NonInteractive) + throw_err (message); + + if (!AskYesNo (message, false, true)) + throw UserAbort (SRC_POS); + } + void TextUserInterface::CreateKeyfile (shared_ptr keyfilePath) const { FilePath path; - RandomNumberGenerator::Start(); - /* force the display of the random enriching interface */ - RandomNumberGenerator::SetEnrichedByUserStatus (false); - UserEnrichRandomPool(); - if (keyfilePath) { - Core->CreateKeyfile (*keyfilePath); + path = *keyfilePath; } else { - wstring fileName = AskFilePath(); - if (fileName.empty()) + path = AskFilePath(); + if (path.IsEmpty()) return; - - Core->CreateKeyfile (fileName); } + ConfirmFileOverwrite (path, wxString::Format (LangString["KEYFILE_ALREADY_EXISTS"], wstring (path).c_str())); + + RandomNumberGenerator::Start(); + /* force the display of the random enriching interface */ + RandomNumberGenerator::SetEnrichedByUserStatus (false); + UserEnrichRandomPool(); + + Core->CreateKeyfile (path); + ShowInfo ("KEYFILE_CREATED"); } diff --git a/src/Main/TextUserInterface.h b/src/Main/TextUserInterface.h index e6367826c4..3248e2b9f0 100644 --- a/src/Main/TextUserInterface.h +++ b/src/Main/TextUserInterface.h @@ -74,6 +74,7 @@ namespace VeraCrypt virtual void Yield () const { } protected: + void ConfirmFileOverwrite (const FilePath &path, const wxString &message) const; static void OnSignal (int signal); virtual void ReadInputStreamLine (wxString &line) const; virtual wxString ReadInputStreamLine () const; From c0e925df99486744222840e2544b72160dcf59e5 Mon Sep 17 00:00:00 2001 From: Ville Takio Date: Tue, 27 Oct 2026 05:11:43 +0200 Subject: [PATCH 3/6] Unix: create the auxiliary FUSE mount point with mkdtemp Outside FUSE-T builds, the auxiliary mount point was the first /tmp/.veracrypt_aux_mntN not listed in the mount table, deleted and created again with mkdir. Two volumes mounted at the same time could pick the same directory, so a mount failed or the file system of one volume was mounted at the mount point of the other, also across users. Any local user could block all mounts by creating the 256 directories. Create a new directory with mkdtemp, as FUSE-T builds already do. The name keeps the .veracrypt_aux_mnt prefix that identifies auxiliary mounts. mkdtemp creates it with mode 0700 instead of the previous 0500, which makes no difference to other users, so its mode is not changed by pathname afterwards, which could follow a replaced directory when the temporary directory belongs to the user (pam_tmpdir). Fixes #1960 --- src/Core/Unix/CoreUnix.cpp | 47 ++++++++------------------------------ 1 file changed, 9 insertions(+), 38 deletions(-) diff --git a/src/Core/Unix/CoreUnix.cpp b/src/Core/Unix/CoreUnix.cpp index 6569b1bad2..f18122f4ea 100644 --- a/src/Core/Unix/CoreUnix.cpp +++ b/src/Core/Unix/CoreUnix.cpp @@ -1287,44 +1287,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 From 4274a05d0bc2e5315cd586fac8682cd05a5c081f Mon Sep 17 00:00:00 2001 From: Ville Takio Date: Tue, 27 Oct 2026 05:12:12 +0200 Subject: [PATCH 4/6] Linux: do not mount a volume again while a detached service keeps it open An emergency dismount lazily detaches the file systems. The FUSE service keeps running with the volume open until the last file on the detached file system is closed, but the volume is no longer in the mount table, so it could be mounted again and two services wrote to it. Once the volume is open, take an exclusive flock for it. The FUSE service inherits the descriptor and holds the lock until it exits; a second mount fails with "already mounted" meanwhile. Because a normally dismounted service may still be exiting, a busy lock is retried for up to 10 seconds. The lock is named after the device number, or the file system and inode numbers, that fstat reports for the opened volume, so it applies to the file actually served, whatever path leads to it. It is not taken on the volume file itself, whose locking is disabled to avoid remote file system issues. The lock files are created with openat in the root owned /var/run/veracrypt, which is opened without following symbolic links and checked through its descriptor. /var/run also exists on systems without /run. Without root or a usable lock directory, mounting works as before. Fixes #1959 --- src/Core/Unix/CoreUnix.cpp | 72 ++++++++++++++++++++++++++++++++++++++ src/Platform/File.h | 1 + 2 files changed, 73 insertions(+) diff --git a/src/Core/Unix/CoreUnix.cpp b/src/Core/Unix/CoreUnix.cpp index f18122f4ea..90057c16e9 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 @@ -1189,6 +1191,66 @@ namespace VeraCrypt return GetFirstFreeSlotNumber(); } +#ifdef TC_LINUX + // Returns a descriptor holding an exclusive lock for the open volume file, or + // -1 if no lock can be created. The lock is released when the last copy of the + // descriptor is closed: the one the FUSE service inherits lasts until the + // service exits. + static int LockVolumeForMount (const File &volumeFile) + { + struct stat volumeInfo; + if (geteuid() != 0 || fstat (volumeFile.GetSystemHandle(), &volumeInfo) == -1) + return -1; + + // /var/run also exists on systems without /run + const char *lockDirectory = "/var/run/veracrypt"; + if (mkdir (lockDirectory, S_IRWXU) == -1 && errno != EEXIST) + return -1; + + // Keep using the directory that was checked + int directoryFd = open (lockDirectory, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (directoryFd == -1) + return -1; + finally_do_arg (int, directoryFd, { close (finally_arg); }); + + struct stat directoryInfo; + if (fstat (directoryFd, &directoryInfo) == -1 || directoryInfo.st_uid != 0 + || (directoryInfo.st_mode & (S_IWGRP | S_IWOTH))) + return -1; + + stringstream lockName; + lockName << hex; + if (S_ISBLK (volumeInfo.st_mode)) + lockName << "device-" << (uint64) volumeInfo.st_rdev; + else + lockName << "file-" << (uint64) volumeInfo.st_dev << "-" << (uint64) volumeInfo.st_ino; + + int fd = openat (directoryFd, lockName.str().c_str(), O_RDWR | O_CREAT | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR); + if (fd == -1) + return -1; + + // A service that has just been dismounted normally may still be exiting + for (int t = 0; flock (fd, LOCK_EX | LOCK_NB) == -1; t++) + { + const int error = errno; + if (error == EINTR) + continue; + + if (error != EWOULDBLOCK || t >= 100) + { + close (fd); + if (error == EWOULDBLOCK) + throw VolumeAlreadyMounted (SRC_POS); + return -1; + } + + Thread::Sleep (100); + } + + return fd; + } +#endif + shared_ptr CoreUnix::MountVolume (MountOptions &options) { CoalesceSlotNumberAndMountPoint (options); @@ -1257,6 +1319,16 @@ namespace VeraCrypt break; } +#ifdef TC_LINUX + // An emergency dismount only detaches the file systems: the FUSE service + // keeps the volume open after it has left the mount table, until the + // last file on the volume is closed. Mounting the volume again meanwhile + // would let two services write to it. The lock identifies the file that + // was opened, whatever path leads to it. + int volumeLock = LockVolumeForMount (*volume->GetFile()); + finally_do_arg (int, volumeLock, { if (finally_arg != -1) close (finally_arg); }); +#endif + // Opening a partially encrypted system volume can force read-only // protection. Carry that effective mode into either mount backend. if (volume->GetProtectionType() == VolumeProtection::ReadOnly) diff --git a/src/Platform/File.h b/src/Platform/File.h index 9c870c739c..99a9e02f21 100644 --- a/src/Platform/File.h +++ b/src/Platform/File.h @@ -71,6 +71,7 @@ namespace VeraCrypt { } virtual ~File (); + SystemFileHandleType GetSystemHandle () const { return FileHandle; } void AssignSystemHandle (SystemFileHandleType openFileHandle, bool sharedHandle = true) { if (FileIsOpen) From 7872271f247949c780d2a062d7490a68e999d37c Mon Sep 17 00:00:00 2001 From: Ville Takio Date: Tue, 27 Oct 2026 03:03:37 +0200 Subject: [PATCH 5/6] Unix: lock key buffers in memory and exclude them from core dumps SecureBuffer only erased its memory when freed. The FUSE service that holds the master keys had no locked memory and could be core dumped, and a core file of it contained both halves of the master key. Give each SecureBuffer whole pages of its own, lock them with mlock and exclude them from core dumps (MADV_DONTDUMP, or MADV_NOCORE on FreeBSD). Separate pages let a freed buffer be unlocked without affecting another one. Memory locks are not inherited by fork, so the FUSE service locks the live buffers again once FUSE has started it in the background. If RLIMIT_MEMLOCK does not allow locking, the buffers are used unlocked as before. Key material can also be copied outside SecureBuffer, so the core service and the FUSE service additionally disable core dumps: they set RLIMIT_CORE to 0 and, on Linux, clear the dumpable flag, which a core_pattern pipe does not ignore. Volume::WriteSectors encrypts each write in a buffer that is encrypted in place right after the copy. Allocating it as a SecureBuffer would lock and unlock its pages on every write and cut write throughput by about 20 to 60 percent depending on the write size, so it becomes a Buffer that is still erased when freed. Fixes #1958 --- src/Core/Unix/CoreService.cpp | 2 + src/Driver/Fuse/FuseService.cpp | 6 ++ src/Platform/Buffer.cpp | 108 +++++++++++++++++++++++++++++++- src/Platform/Buffer.h | 10 ++- src/Platform/Unix/Process.cpp | 16 +++++ src/Platform/Unix/Process.h | 1 + src/Volume/Volume.cpp | 5 +- 7 files changed, 144 insertions(+), 4 deletions(-) diff --git a/src/Core/Unix/CoreService.cpp b/src/Core/Unix/CoreService.cpp index 2e71c9c238..116c80a331 100644 --- a/src/Core/Unix/CoreService.cpp +++ b/src/Core/Unix/CoreService.cpp @@ -1136,6 +1136,8 @@ namespace VeraCrypt { try { + Process::DisableCoreDumps(); + if (forkProcess) RedirectStandardErrorToDevNull (); diff --git a/src/Driver/Fuse/FuseService.cpp b/src/Driver/Fuse/FuseService.cpp index 055eb1de0a..5b6a0527a9 100644 --- a/src/Driver/Fuse/FuseService.cpp +++ b/src/Driver/Fuse/FuseService.cpp @@ -664,6 +664,9 @@ namespace VeraCrypt { try { + // FUSE may have forked to run in the background + SecureBuffer::LockAllAfterFork(); + // Termination signals are handled by a separate process to allow clean dismount on shutdown struct sigaction action; Memory::Zero (&action, sizeof (action)); @@ -1749,6 +1752,9 @@ namespace VeraCrypt void FuseService::ExecFunctor::operator() (int argc, char *argv[]) { + // The service holds the master keys until the volume is dismounted + Process::DisableCoreDumps(); + #if defined(TC_MACOSX) && defined(VC_MACOSX_FUSET) close (StartupPeerFd); #endif diff --git a/src/Platform/Buffer.cpp b/src/Platform/Buffer.cpp index ac55c7bb93..db63bbaf90 100644 --- a/src/Platform/Buffer.cpp +++ b/src/Platform/Buffer.cpp @@ -12,9 +12,51 @@ #include "Buffer.h" #include "Exception.h" +#ifdef TC_UNIX +#include +#include +#include +#include "ForEach.h" +#include "Mutex.h" +#endif namespace VeraCrypt { +#ifdef TC_UNIX + // Secure buffers occupy whole pages of their own, which are locked in memory + // and excluded from core dumps. Unlocking or including the pages again when a + // buffer is freed cannot then affect another allocation. + struct SecureBufferList + { + Mutex ListMutex; + set Buffers; + }; + + static SecureBufferList &GetSecureBufferList () + { + // Never destroyed: buffers may be freed after static destructors have run + static SecureBufferList *list = new SecureBufferList; + return *list; + } + + static size_t GetPageSize () + { + long pageSize = sysconf (_SC_PAGESIZE); + return pageSize > 0 ? (size_t) pageSize : 4096; + } + + static void ExcludeFromCoreDump (void *memory, size_t size, bool exclude) + { +#if defined (MADV_DONTDUMP) + madvise (memory, size, exclude ? MADV_DONTDUMP : MADV_DODUMP); +#elif defined (MADV_NOCORE) + madvise (memory, size, exclude ? MADV_NOCORE : MADV_CORE); +#else + (void) memory; (void) size; (void) exclude; +#endif + } +#endif + Buffer::Buffer () : DataPtr (nullptr), DataSize (0), DataAlignment (0) { } @@ -108,7 +150,7 @@ namespace VeraCrypt Memory::Zero (DataPtr, DataSize); } - SecureBuffer::SecureBuffer (size_t size, size_t alignment) + SecureBuffer::SecureBuffer (size_t size, size_t alignment) : AllocatedSize (0), Locked (false) { Allocate (size, alignment); } @@ -121,7 +163,40 @@ namespace VeraCrypt void SecureBuffer::Allocate (size_t size, size_t alignment) { +#ifdef TC_UNIX + if (size < 1) + throw ParameterIncorrect (SRC_POS); + + if (DataPtr != nullptr) + { + if ((DataSize == size) && (DataAlignment == alignment)) + return; + Free(); + } + + const size_t pageSize = GetPageSize(); + if (size > ((size_t) -1) - pageSize) + throw ParameterTooLarge (SRC_POS); + + const size_t allocatedSize = (size + pageSize - 1) / pageSize * pageSize; + // Alignments are powers of two: the larger one satisfies both + uint8 *memory = static_cast (Memory::AllocateAligned (allocatedSize, alignment > pageSize ? alignment : pageSize)); + + // Locking fails if RLIMIT_MEMLOCK is exceeded; the buffer is still usable + Locked = mlock (memory, allocatedSize) == 0; + ExcludeFromCoreDump (memory, allocatedSize, true); + + DataPtr = memory; + DataSize = size; + DataAlignment = alignment; + AllocatedSize = allocatedSize; + + SecureBufferList &list = GetSecureBufferList(); + ScopeLock lock (list.ListMutex); + list.Buffers.insert (this); +#else Buffer::Allocate (size, alignment); +#endif } void SecureBuffer::Free () @@ -129,8 +204,39 @@ namespace VeraCrypt if (DataPtr == nullptr) throw NotInitialized (SRC_POS); +#ifdef TC_UNIX + { + SecureBufferList &list = GetSecureBufferList(); + ScopeLock lock (list.ListMutex); + list.Buffers.erase (this); + } + + burn (DataPtr, AllocatedSize); + ExcludeFromCoreDump (DataPtr, AllocatedSize, false); + if (Locked) + munlock (DataPtr, AllocatedSize); + + Memory::FreeAligned (DataPtr); + DataPtr = nullptr; + DataSize = 0; + DataAlignment = 0; + AllocatedSize = 0; + Locked = false; +#else Erase (); Buffer::Free (); +#endif + } + + void SecureBuffer::LockAllAfterFork () + { +#ifdef TC_UNIX + SecureBufferList &list = GetSecureBufferList(); + ScopeLock lock (list.ListMutex); + + foreach (SecureBuffer *buffer, list.Buffers) + buffer->Locked = mlock (buffer->DataPtr, buffer->AllocatedSize) == 0; +#endif } void BufferPtr::CopyFrom (const ConstBufferPtr &bufferPtr) const diff --git a/src/Platform/Buffer.h b/src/Platform/Buffer.h index 5d14d6a6b8..05e4cc4d91 100644 --- a/src/Platform/Buffer.h +++ b/src/Platform/Buffer.h @@ -107,15 +107,21 @@ namespace VeraCrypt class SecureBuffer : public Buffer { public: - SecureBuffer () { } + SecureBuffer () : AllocatedSize (0), Locked (false) { } SecureBuffer (size_t size, size_t alignment = 0); - SecureBuffer (const ConstBufferPtr &bufferPtr) { CopyFrom (bufferPtr); } + SecureBuffer (const ConstBufferPtr &bufferPtr) : AllocatedSize (0), Locked (false) { CopyFrom (bufferPtr); } virtual ~SecureBuffer (); virtual void Allocate (size_t size, size_t alignment = 0); virtual void Free (); + // Memory locks are not inherited by a forked process + static void LockAllAfterFork (); + private: + size_t AllocatedSize; + bool Locked; + SecureBuffer (const SecureBuffer &); SecureBuffer &operator= (const SecureBuffer &); }; diff --git a/src/Platform/Unix/Process.cpp b/src/Platform/Unix/Process.cpp index 916978b988..a01d95b0af 100644 --- a/src/Platform/Unix/Process.cpp +++ b/src/Platform/Unix/Process.cpp @@ -16,9 +16,11 @@ #include #include #include +#include #include #include #ifdef TC_LINUX +#include #include #endif #include "Process.h" @@ -210,6 +212,20 @@ namespace VeraCrypt } #endif + void Process::DisableCoreDumps () + { + // For processes that hold key material. A core_pattern pipe ignores + // RLIMIT_CORE on Linux, but not the dumpable flag, which also keeps + // other processes of the same user from attaching. + struct rlimit limit; + limit.rlim_cur = 0; + limit.rlim_max = 0; + setrlimit (RLIMIT_CORE, &limit); +#ifdef TC_LINUX + prctl (PR_SET_DUMPABLE, 0, 0, 0, 0); +#endif + } + bool Process::IsExecutable(const std::string& path) { struct stat sb; if (stat(path.c_str(), &sb) == 0) { diff --git a/src/Platform/Unix/Process.h b/src/Platform/Unix/Process.h index 3e3a61503a..cf4999e0c5 100644 --- a/src/Platform/Unix/Process.h +++ b/src/Platform/Unix/Process.h @@ -31,6 +31,7 @@ namespace VeraCrypt Process (); virtual ~Process (); + static void DisableCoreDumps (); static bool IsExecutable(const std::string& path); static std::string FindSystemBinary(const char* name, std::string& errorMsg); static string Execute (const string &processName, const list &arguments, int timeOut = -1, ProcessExecFunctor *execFunctor = nullptr, const Buffer *inputData = nullptr); diff --git a/src/Volume/Volume.cpp b/src/Volume/Volume.cpp index 0cee6344c6..9536a5b059 100644 --- a/src/Volume/Volume.cpp +++ b/src/Volume/Volume.cpp @@ -400,7 +400,10 @@ namespace VeraCrypt if (Protection == VolumeProtection::HiddenVolumeReadOnly) CheckProtectedRange (hostOffset, length); - SecureBuffer encBuf (buffer.Size()); + // Encrypted in place right after the copy. A SecureBuffer would lock the + // pages of every write in memory, which slows down writes considerably. + Buffer encBuf (buffer.Size()); + finally_do_arg (Buffer *, &encBuf, { finally_arg->Erase(); }); encBuf.CopyFrom (buffer); EA->EncryptSectors (encBuf, hostOffset / SectorSize, length / SectorSize, SectorSize); From 355c25fe723e05925fb4af328186fb787fabe6e7 Mon Sep 17 00:00:00 2001 From: Ville Takio Date: Tue, 27 Oct 2026 06:16:17 +0200 Subject: [PATCH 6/6] Tests: add regression tests for issues #1958 to #1962 Run against the installed package as root in the Linux workflow: locked memory and no core file for the FUSE service (#1958), no second mount while an emergency-detached service keeps the volume open (#1959), parallel mounts and an existing /tmp/.veracrypt_aux_mnt1 (#1960), the hidden volume protection password in the service memory (#1961), and the overwrite checks of --create-keyfile and --backup-headers (#1962). --- .github/workflows/build-linux.yml | 3 + Tests/test_unix_security_regressions.py | 348 ++++++++++++++++++++++++ 2 files changed, 351 insertions(+) create mode 100644 Tests/test_unix_security_regressions.py diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 204d29af8b..08183a763f 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -24,6 +24,7 @@ on: - 'Tests/test_linux_privilege_helpers.py' - 'Tests/test_stdin_password.py' - 'Tests/test_mount_helper.py' + - 'Tests/test_unix_security_regressions.py' - 'Tests/test.*.hc' - 'Tests/test_volume_read.py' - 'Tests/volume_read_test.cpp' @@ -54,6 +55,7 @@ on: - 'Tests/test_linux_privilege_helpers.py' - 'Tests/test_stdin_password.py' - 'Tests/test_mount_helper.py' + - 'Tests/test_unix_security_regressions.py' - 'Tests/test.*.hc' - 'Tests/test_volume_read.py' - 'Tests/volume_read_test.cpp' @@ -201,6 +203,7 @@ jobs: sudo locale-gen en_US en_US.UTF-8 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_unix_security_regressions.py sudo apt remove -y veracrypt - name: Install and test VeraCrypt Console .deb packages diff --git a/Tests/test_unix_security_regressions.py b/Tests/test_unix_security_regressions.py new file mode 100644 index 0000000000..5ab3247e2d --- /dev/null +++ b/Tests/test_unix_security_regressions.py @@ -0,0 +1,348 @@ +#!/usr/bin/env python3 +"""Regression tests for issues #1958 to #1962, against the installed +veracrypt. + +Usage (as root, after installing a package): + python3 Tests/test_unix_security_regressions.py [--veracrypt PATH] + [--rounds N] + +Creates small volumes in a temporary directory and mounts them through FUSE +(nokernelcrypto), as in the reports. Checked: + +- #1958: the FUSE service has locked memory and a core file size limit of + zero; +- #1959: after an emergency dismount, while a file on the detached file + system is still open, the volume cannot be mounted again; it can once the + old service has exited; +- #1960: two volumes mounted at the same time each appear at their own + mount point, and an existing /tmp/.veracrypt_aux_mnt1 does not prevent + mounting; +- #1961: the hidden volume protection password is not in the memory of the + FUSE service, apart from the copy in its command line ([stack]); +- #1962: --create-keyfile does not replace an existing keyfile in + non-interactive mode unless --force is given, and --backup-headers asks + before replacing an existing file. +""" + +import argparse +import os +import shutil +import subprocess +import sys +import tempfile +import time +from pathlib import Path + +PASSWORD_A = "regression-test-password-a" +PASSWORD_B = "regression-test-password-b" +PASSWORD_OUTER = "regression-outer-password-c" +PASSWORD_HIDDEN = "regression-hidden-password-d" +# A low PIM keeps the tests fast; it needs passwords of 20 characters or more. +COMMON = ["--pim=1", "--keyfiles="] +MOUNT = COMMON + ["--protect-hidden=no", "--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.binary = os.path.realpath(path) + self.work = work + + def run(self, args, stdin=None, timeout=300): + return subprocess.run([self.path, "--text"] + args, input=stdin, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, cwd=self.work, + timeout=timeout) + + def create(self, path, size, password, filesystem, volume_type="normal"): + return self.run(["--non-interactive", "--create", str(path), + "--size=" + size, "--volume-type=" + volume_type, + "--encryption=AES", "--hash=SHA-512", + "--filesystem=" + filesystem, "-p", password, + "--random-source=" + str(self.work / "random")] + + COMMON) + + def mount(self, volume, password, mount_point=None, extra=()): + args = ["--non-interactive", "-p", password] + MOUNT + list(extra) + args.append(str(volume)) + if mount_point: + args.append(str(mount_point)) + return self.run(args) + + def dismount(self, target=None): + return self.run(["--non-interactive", "-d"] + + ([str(target)] if target else [])) + + def service(self, volume): + """PID of the FUSE service that has the volume file open.""" + volume = os.path.realpath(volume) + for entry in os.listdir("/proc"): + if not entry.isdigit(): + continue + try: + if os.readlink("/proc/%s/exe" % entry) != self.binary: + continue + fds = ["/proc/%s/fd/%s" % (entry, fd) + for fd in os.listdir("/proc/%s/fd" % entry)] + targets = {os.readlink(fd) for fd in fds} + except OSError: + continue + if "/dev/fuse" in targets and volume in targets: + return int(entry) + return None + + +def process_status(pid, field): + with open("/proc/%d/status" % pid) as status: + for line in status: + if line.startswith(field + ":"): + return line.split(":", 1)[1].strip() + return "" + + +def core_limit(pid): + with open("/proc/%d/limits" % pid) as limits: + for line in limits: + if line.startswith("Max core file size"): + return line.split()[4:6] + return [] + + +def memory_hits(pid, needle): + """Occurrences of needle per mapping name in the memory of pid.""" + hits = {} + with open("/proc/%d/maps" % pid) as maps, \ + open("/proc/%d/mem" % pid, "rb", 0) as mem: + for line in maps: + fields = line.split() + if not fields[1].startswith("r") or "[vvar" in line: + continue + start, end = (int(x, 16) for x in fields[0].split("-")) + name = fields[5] if len(fields) > 5 else "anonymous" + try: + mem.seek(start) + data = mem.read(end - start) + except (OSError, ValueError, OverflowError): + continue + count = data.count(needle) + if count: + hits[name] = hits.get(name, 0) + count + return hits + + +def wait_for_exit(pid, timeout): + end = time.monotonic() + timeout + while time.monotonic() < end: + if not os.path.exists("/proc/%d" % pid): + return True + time.sleep(0.5) + return False + + +def test_memory(vc, c, volume, mount_point): + r = vc.mount(volume, PASSWORD_A, mount_point) + c.check("#1958: mount", r.returncode == 0, text(r)) + pid = vc.service(volume) + c.check("#1958: find the FUSE service", pid is not None) + if pid: + locked = process_status(pid, "VmLck") + c.check("#1958: the FUSE service has locked memory", + locked.split()[:1] != ["0"], "VmLck: " + locked) + limit = core_limit(pid) + c.check("#1958: the FUSE service cannot write a core file", + limit == ["0", "0"], "core file size limit: %s" % limit) + vc.dismount(mount_point) + + +def test_detached_service(vc, c, volume, mount_point, second_mount_point): + r = vc.mount(volume, PASSWORD_A, mount_point) + c.check("#1959: mount", r.returncode == 0, text(r)) + pid = vc.service(volume) + busy = open(mount_point / "busy", "w") + try: + r = vc.run(["--non-interactive", "-d", "--emergency-unmount", + str(mount_point)]) + c.check("#1959: emergency dismount", r.returncode == 0, text(r)) + c.check("#1959: the old service is still running", + pid is not None and os.path.exists("/proc/%d" % pid)) + r = vc.mount(volume, PASSWORD_A, second_mount_point) + c.check("#1959: mounting again while the old service runs fails", + r.returncode != 0 and "already mounted" in text(r), text(r)) + if r.returncode == 0: + vc.dismount(second_mount_point) + finally: + busy.close() + c.check("#1959: the old service exits once the file is closed", + pid is None or wait_for_exit(pid, 30)) + r = vc.mount(volume, PASSWORD_A, second_mount_point) + c.check("#1959: mounting again after the old service exited works", + r.returncode == 0, text(r)) + vc.dismount(second_mount_point) + + +def test_parallel_mounts(vc, c, volume_a, volume_b, mount_a, mount_b, rounds): + failed = [] + for _ in range(rounds): + jobs = [subprocess.Popen( + [vc.path, "--text", "--non-interactive", "--slot=%d" % slot, + "-p", password] + MOUNT + [str(volume), str(mount_point)], + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, cwd=vc.work) + for slot, password, volume, mount_point in ( + (1, PASSWORD_A, volume_a, mount_a), + (2, PASSWORD_B, volume_b, mount_b))] + outputs = [job.communicate(timeout=300)[0] for job in jobs] + for mount_point, marker, output in ((mount_a, "a", outputs[0]), + (mount_b, "b", outputs[1])): + try: + found = (mount_point / "marker").read_text().strip() + except OSError: + found = None + if found != marker: + failed.append("%s holds %r: %s" % ( + mount_point, found, output.decode("utf-8", "replace"))) + vc.dismount() + c.check("#1960: %d rounds of two parallel mounts" % rounds, not failed, + "\n".join(failed[:3])) + + foreign = Path("/tmp/.veracrypt_aux_mnt1") + created = not foreign.exists() + if created: + foreign.mkdir(mode=0o755) + (foreign / "file").write_text("x") + os.chown(foreign, 65534, 65534) + try: + r = vc.mount(volume_a, PASSWORD_A, mount_a) + c.check("#1960: an existing %s does not prevent mounting" % foreign, + r.returncode == 0, text(r)) + vc.dismount(mount_a) + finally: + if created: + shutil.rmtree(foreign, ignore_errors=True) + + +def test_protection_password(vc, c, work): + volume = work / "hidden.hc" + r = vc.create(volume, "10M", PASSWORD_OUTER, "none") + c.check("#1961: create an outer volume", r.returncode == 0, text(r)) + r = vc.create(volume, "3M", PASSWORD_HIDDEN, "none", "hidden") + c.check("#1961: create a hidden volume", r.returncode == 0, text(r)) + r = vc.run(["--non-interactive", "-p", PASSWORD_OUTER] + COMMON + + ["--protect-hidden=yes", + "--protection-password=" + PASSWORD_HIDDEN, + "--protection-pim=1", "--protection-keyfiles=", + "--mount-options=nokernelcrypto", "--filesystem=none", + str(volume)]) + c.check("#1961: mount with hidden volume protection", r.returncode == 0, + text(r)) + pid = vc.service(volume) + c.check("#1961: find the FUSE service", pid is not None) + if pid: + hits = memory_hits(pid, PASSWORD_HIDDEN.encode()) + hits.pop("[stack]", None) + c.check("#1961: the protection password is only in the command line", + not hits, "found in %s" % hits) + vc.dismount(volume) + + +def test_overwrite(vc, c, work): + keyfile = work / "keyfile" + r = vc.run(["--non-interactive", "--create-keyfile", str(keyfile), + "--random-source=" + str(work / "random")]) + c.check("#1962: create a keyfile", r.returncode == 0, text(r)) + original = keyfile.read_bytes() if keyfile.exists() else b"" + r = vc.run(["--non-interactive", "--create-keyfile", str(keyfile), + "--random-source=" + str(work / "random")]) + c.check("#1962: --create-keyfile refuses an existing keyfile", + r.returncode != 0 and "already exists" in text(r), text(r)) + c.check("#1962: the existing keyfile is unchanged", + keyfile.read_bytes() == original) + r = vc.run(["--non-interactive", "--force", "--create-keyfile", + str(keyfile), "--random-source=" + str(work / "random")]) + c.check("#1962: --force replaces the keyfile", + r.returncode == 0 and keyfile.read_bytes() != original, text(r)) + + volume = work / "backup.hc" + r = vc.create(volume, "2M", PASSWORD_A, "none") + c.check("#1962: create a volume", r.returncode == 0, text(r)) + target = work / "existing" + for answer, replaced in (("n", False), ("y", True)): + target.write_text("keep") + # password, PIM, keyfiles, hidden volume, confirmation, file name, + # overwrite + answers = "%s\n1\n\nn\ny\n%s\n%s\n" % (PASSWORD_A, target, answer) + r = vc.run(["--random-source=" + str(work / "random"), + "--backup-headers", str(volume)], + stdin=answers.encode()) + c.check("#1962: --backup-headers asks before replacing a file " + "(answer %s)" % answer, + "already exists" in text(r) + and (target.read_text(errors="replace") != "keep") == replaced, + text(r)) + + +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=20, + help="rounds of parallel mounts for #1960") + args = parser.parse_args() + if os.geteuid() != 0: + sys.exit("Run as root: the tests mount volumes") + + work = Path(tempfile.mkdtemp(prefix="vc-regressions-")) + vc = VeraCrypt(args.veracrypt, work) + c = Checks() + mount_points = [work / name for name in ("a", "b", "c")] + for mount_point in mount_points: + mount_point.mkdir() + (work / "random").write_bytes(os.urandom(4096)) + try: + volume_a, volume_b = work / "a.hc", work / "b.hc" + for volume, password, marker in ((volume_a, PASSWORD_A, "a"), + (volume_b, PASSWORD_B, "b")): + r = vc.create(volume, "10M", password, "ext4") + c.check("create %s" % volume.name, r.returncode == 0, text(r)) + r = vc.mount(volume, password, mount_points[0]) + if r.returncode == 0: + (mount_points[0] / "marker").write_text(marker) + vc.dismount(mount_points[0]) + if c.failures: + sys.exit("Cannot create the test volumes") + + test_memory(vc, c, volume_a, mount_points[0]) + test_detached_service(vc, c, volume_a, mount_points[0], + mount_points[1]) + test_parallel_mounts(vc, c, volume_a, volume_b, mount_points[0], + mount_points[1], args.rounds) + test_protection_password(vc, c, work) + test_overwrite(vc, c, work) + finally: + vc.dismount() + if 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()