From 5381c1b36559311e246c9aeb34dbe3aa926faa60 Mon Sep 17 00:00:00 2001 From: Mounir IDRASSI Date: Tue, 30 Jun 2026 11:03:54 +0900 Subject: [PATCH] Unix: harden OpenBSD doas authentication startup Keep OpenBSD doas stderr on the private authentication PTY because doas requires stderr to be a terminal while prompting. Capture authentication-terminal diagnostics, strip prompts from user-facing errors, and fail promptly on explicit authentication denial. Wait for actual prompt bytes before sending the password so OpenBSD PTY POLLIN|POLLHUP before slave open cannot race with readpassphrase terminal flushing. --- src/Core/Unix/CoreService.cpp | 335 +++++++++++++++++++++++++++++++--- 1 file changed, 312 insertions(+), 23 deletions(-) diff --git a/src/Core/Unix/CoreService.cpp b/src/Core/Unix/CoreService.cpp index d649950b..13e6c5bb 100644 --- a/src/Core/Unix/CoreService.cpp +++ b/src/Core/Unix/CoreService.cpp @@ -211,7 +211,7 @@ namespace VeraCrypt } } - static void AttachDoasAuthTerminal (const string &slavePath) + static void AttachDoasAuthTerminal (const string &slavePath, bool keepStderrOnTerminal) { throw_sys_if (setsid () == -1); #ifdef O_CLOEXEC @@ -253,7 +253,34 @@ namespace VeraCrypt throw SystemException (SRC_POS, "Failed to set doas authentication terminal as controlling terminal"); } #endif - close (ttyFd); +#ifdef TC_OPENBSD + if (tcsetpgrp (ttyFd, getpgrp()) == -1) + { + int err = errno; + close (ttyFd); + errno = err; + throw SystemException (SRC_POS, "Failed to set doas authentication terminal foreground process group"); + } +#else + tcsetpgrp (ttyFd, getpgrp()); +#endif + bool ttyFdKeptOnStderr = keepStderrOnTerminal && ttyFd == STDERR_FILENO; + if (keepStderrOnTerminal) + { + try + { + DupToStandardFd (ttyFd, STDERR_FILENO); + } + catch (...) + { + if (!ttyFdKeptOnStderr) + close (ttyFd); + throw; + } + } + + if (!ttyFdKeptOnStderr) + close (ttyFd); } static void ReapChildProcessAsync (int pid) @@ -334,6 +361,115 @@ namespace VeraCrypt return string (errOutput.begin(), errOutput.end()); } + static string NormalizeTerminalOutput (const string &terminalOutput) + { + string normalized; + bool previousNewline = false; + + for (size_t i = 0; i < terminalOutput.size(); ++i) + { + char c = terminalOutput[i]; + if (c == '\r') + { + if (i + 1 < terminalOutput.size() && terminalOutput[i + 1] == '\n') + continue; + c = '\n'; + } + + if (c == '\n') + { + if (previousNewline) + continue; + previousNewline = true; + } + else + previousNewline = false; + + normalized += c; + } + + while (!normalized.empty() && normalized[0] == '\n') + normalized.erase (0, 1); + while (!normalized.empty() && normalized[normalized.size() - 1] == '\n') + normalized.erase (normalized.size() - 1); + + return normalized; + } + + static bool StringEndsWith (const string &str, const string &suffix) + { + return str.size() >= suffix.size() && str.compare (str.size() - suffix.size(), suffix.size(), suffix) == 0; + } + + static string TrimTerminalLine (const string &line) + { + size_t first = line.find_first_not_of (" \t"); + if (first == string::npos) + return string(); + + size_t last = line.find_last_not_of (" \t"); + return line.substr (first, last - first + 1); + } + + static bool IsDoasPasswordPromptLine (const string &line) + { + string trimmed = TrimTerminalLine (line); + return trimmed.find ("doas (") == 0 && StringEndsWith (trimmed, " password:"); + } + + static string NormalizeDoasAuthTerminalOutput (const string &terminalOutput) + { + string normalized = NormalizeTerminalOutput (terminalOutput); + string filtered; + size_t lineStart = 0; + + while (lineStart <= normalized.size()) + { + size_t lineEnd = normalized.find ('\n', lineStart); + string line = lineEnd == string::npos ? normalized.substr (lineStart) : normalized.substr (lineStart, lineEnd - lineStart); + + if (!IsDoasPasswordPromptLine (line)) + { + if (!filtered.empty()) + filtered += '\n'; + filtered += line; + } + + if (lineEnd == string::npos) + break; + + lineStart = lineEnd + 1; + } + + return NormalizeTerminalOutput (filtered); + } + + static bool DoasAuthenticationFailed (const vector &authOutput) + { + return ErrorOutputToString (authOutput).find ("doas: Authentication failed") != string::npos; + } + + static string CombineElevationErrorOutput (const vector &errOutput, const vector &authOutput) + { + string output = ErrorOutputToString (errOutput); + string authText = NormalizeDoasAuthTerminalOutput (ErrorOutputToString (authOutput)); + + if (!authText.empty()) + { + if (!output.empty() && output[output.size() - 1] != '\n') + output += "\n"; + output += authText; + } + + return output; + } + + static void ReadAvailableDataIfAny (int fd, vector &output) + { + if (fd != -1) + ReadAvailableData (fd, output); + } + static void ThrowSerializedExceptionIfAny (const vector &errOutput) { if (errOutput.empty()) @@ -354,8 +490,10 @@ namespace VeraCrypt deserializedException->Throw(); } - static void WriteAllBestEffort (int fd, const char *data, size_t size) + static void WriteAllBestEffort (int fd, const char *data, size_t size, int retryTimeout = 0) { + const int retryDelay = 50; + int retryTimeLeft = retryTimeout; size_t offset = 0; while (offset < size) { @@ -369,11 +507,97 @@ namespace VeraCrypt if (bytesWritten == -1 && errno == EINTR) continue; + if (bytesWritten == -1 && retryTimeLeft > 0 && (errno == EIO || errno == EAGAIN || errno == EWOULDBLOCK)) + { + Thread::Sleep (retryDelay); + retryTimeLeft -= retryDelay; + continue; + } + return; } } - static void SendElevatedServiceSyncWithTimeout (shared_ptr inputStream, int outputFd, int errorFd, int childPid, const string &helperName, int timeout) +#ifdef TC_OPENBSD + static bool ReadDoasAuthTerminalPromptData (int fd, vector &authOutput) + { + char buffer[256]; + bool dataRead = false; + + while (true) + { + ssize_t bytesRead = read (fd, buffer, sizeof (buffer)); + if (bytesRead > 0) + { + authOutput.insert (authOutput.end(), buffer, buffer + bytesRead); + dataRead = true; + continue; + } + + if (bytesRead == -1 && errno == EINTR) + continue; + + if (bytesRead == -1 && (errno == EAGAIN || errno == EWOULDBLOCK)) + return dataRead; + + return dataRead; + } + } + + static void WaitForDoasAuthTerminalPrompt (int fd, vector &authOutput, int timeout) + { + const int pollInterval = 50; + int timeLeft = timeout; + int terminalFlags = fcntl (fd, F_GETFL, 0); + if (terminalFlags == -1) + return; + + bool restoreFlags = (terminalFlags & O_NONBLOCK) == 0; + if (restoreFlags && fcntl (fd, F_SETFL, terminalFlags | O_NONBLOCK) == -1) + return; + + while (timeLeft > 0) + { + struct pollfd pfd; + memset (&pfd, 0, sizeof (pfd)); + pfd.fd = fd; + pfd.events = POLLIN; + + int pollTimeout = timeLeft < pollInterval ? timeLeft : pollInterval; + int pollResult; + do + { + pollResult = poll (&pfd, 1, pollTimeout); + } while (pollResult == -1 && errno == EINTR); + + if (pollResult == -1) + break; + + if (pollResult == 0) + { + timeLeft -= pollTimeout; + continue; + } + + if ((pfd.revents & POLLIN) && ReadDoasAuthTerminalPromptData (fd, authOutput)) + break; + + if (pfd.revents & (POLLERR | POLLNVAL)) + break; + + // On OpenBSD a PTY master reports POLLIN|POLLHUP before the + // slave is opened. A zero-byte read distinguishes that state from + // the real doas prompt; wait real time instead of spinning. + Thread::Sleep (pollTimeout); + timeLeft -= pollTimeout; + } + + if (restoreFlags) + fcntl (fd, F_SETFL, terminalFlags); + } +#endif + + static void SendElevatedServiceSyncWithTimeout (shared_ptr inputStream, int outputFd, int errorFd, int authFd, vector authOutput, int childPid, const string &helperName, int timeout) { vector errOutput; uint8 sync[] = { 0, 0x11, 0x22 }; @@ -384,35 +608,71 @@ namespace VeraCrypt } catch (...) { - ReadAvailableData (errorFd, errOutput); + ReadAvailableDataIfAny (errorFd, errOutput); + ReadAvailableDataIfAny (authFd, authOutput); TerminateChildProcessAsync (childPid); ThrowSerializedExceptionIfAny (errOutput); - throw ElevationFailed (SRC_POS, helperName, 1, ErrorOutputToString (errOutput)); + throw ElevationFailed (SRC_POS, helperName, 1, CombineElevationErrorOutput (errOutput, authOutput)); } const int pollInterval = 200; int timeLeft = timeout; + bool errorFdActive = errorFd != -1; + bool authFdActive = authFd != -1; while (timeLeft > 0) { - struct pollfd fds[2]; + struct pollfd fds[3]; memset (fds, 0, sizeof (fds)); fds[0].fd = outputFd; fds[0].events = POLLIN; - fds[1].fd = errorFd; - fds[1].events = POLLIN; + nfds_t fdCount = 1; + nfds_t errorFdIndex = 0; + nfds_t authFdIndex = 0; + if (errorFdActive) + { + errorFdIndex = fdCount; + fds[fdCount].fd = errorFd; + fds[fdCount].events = POLLIN; + ++fdCount; + } + if (authFdActive) + { + authFdIndex = fdCount; + fds[fdCount].fd = authFd; + fds[fdCount].events = POLLIN; + ++fdCount; + } int pollTimeout = timeLeft < pollInterval ? timeLeft : pollInterval; int pollResult; do { - pollResult = poll (fds, array_capacity (fds), pollTimeout); + pollResult = poll (fds, fdCount, pollTimeout); } while (pollResult == -1 && errno == EINTR); throw_sys_if (pollResult == -1); timeLeft -= pollTimeout; - if (fds[1].revents & (POLLIN | POLLHUP | POLLERR)) + if (errorFdActive && (fds[errorFdIndex].revents & (POLLIN | POLLHUP | POLLERR))) + { + size_t previousErrOutputSize = errOutput.size(); ReadAvailableData (errorFd, errOutput); + if ((fds[errorFdIndex].revents & (POLLHUP | POLLERR)) && errOutput.size() == previousErrOutputSize) + errorFdActive = false; + } + if (authFdActive && (fds[authFdIndex].revents & (POLLIN | POLLHUP | POLLERR))) + { + size_t previousAuthOutputSize = authOutput.size(); + ReadAvailableData (authFd, authOutput); + if ((fds[authFdIndex].revents & (POLLHUP | POLLERR)) && authOutput.size() == previousAuthOutputSize) + authFdActive = false; + } + if (DoasAuthenticationFailed (authOutput)) + { + TerminateChildProcessAsync (childPid); + ThrowSerializedExceptionIfAny (errOutput); + throw ElevationFailed (SRC_POS, helperName, 1, CombineElevationErrorOutput (errOutput, authOutput)); + } if (fds[0].revents & POLLIN) { @@ -428,7 +688,7 @@ namespace VeraCrypt TerminateChildProcessAsync (childPid); ThrowSerializedExceptionIfAny (errOutput); - throw ElevationFailed (SRC_POS, helperName, 1, ErrorOutputToString (errOutput)); + throw ElevationFailed (SRC_POS, helperName, 1, CombineElevationErrorOutput (errOutput, authOutput)); } int status; @@ -440,10 +700,11 @@ namespace VeraCrypt if (waitRes == childPid) { - ReadAvailableData (errorFd, errOutput); + ReadAvailableDataIfAny (errorFd, errOutput); + ReadAvailableDataIfAny (authFd, authOutput); ThrowSerializedExceptionIfAny (errOutput); int exitCode = WIFEXITED (status) ? WEXITSTATUS (status) : 1; - throw ElevationFailed (SRC_POS, helperName, exitCode, ErrorOutputToString (errOutput)); + throw ElevationFailed (SRC_POS, helperName, exitCode, CombineElevationErrorOutput (errOutput, authOutput)); } throw_sys_if (waitRes == -1); @@ -452,13 +713,14 @@ namespace VeraCrypt { TerminateChildProcessAsync (childPid); ThrowSerializedExceptionIfAny (errOutput); - throw ElevationFailed (SRC_POS, helperName, 1, ErrorOutputToString (errOutput)); + throw ElevationFailed (SRC_POS, helperName, 1, CombineElevationErrorOutput (errOutput, authOutput)); } } - ReadAvailableData (errorFd, errOutput); + ReadAvailableDataIfAny (errorFd, errOutput); + ReadAvailableDataIfAny (authFd, authOutput); ThrowSerializedExceptionIfAny (errOutput); - string errorOutput = ErrorOutputToString (errOutput); + string errorOutput = CombineElevationErrorOutput (errOutput, authOutput); if (errorOutput.empty()) errorOutput = "Timed out while waiting for the elevated VeraCrypt service to start"; @@ -1068,14 +1330,26 @@ namespace VeraCrypt appPath = appImageEnv; } #endif - if (privilegeHelper.IsDoas() && !useCallerDoasTerminal) + bool useDoasAuthTerminal = privilegeHelper.IsDoas() && !useCallerDoasTerminal; +#ifdef TC_OPENBSD + // OpenBSD doas requires stderr to be a terminal while it + // prompts for the password. Keeping the private PTY slave + // on stderr satisfies that while stdin/stdout stay as the + // service pipes. + bool keepDoasStderrOnAuthTerminal = useDoasAuthTerminal; +#else + bool keepDoasStderrOnAuthTerminal = false; +#endif + if (useDoasAuthTerminal) { - AttachDoasAuthTerminal (doasAuthTerminalPath); + AttachDoasAuthTerminal (doasAuthTerminalPath, keepDoasStderrOnAuthTerminal); } - if (doasAuthTerminal != -1) + bool doasAuthTerminalReplacedByStderr = keepDoasStderrOnAuthTerminal && doasAuthTerminal == STDERR_FILENO; + if (doasAuthTerminal != -1 && !doasAuthTerminalReplacedByStderr) close (doasAuthTerminal); - DupToStandardFd (errPipe.GetWriteFD(), STDERR_FILENO); + if (!keepDoasStderrOnAuthTerminal) + DupToStandardFd (errPipe.GetWriteFD(), STDERR_FILENO); DupToStandardFd (inPipe->GetReadFD(), STDIN_FILENO); DupToStandardFd (outPipe->GetWriteFD(), STDOUT_FILENO); @@ -1131,6 +1405,7 @@ namespace VeraCrypt Memory::Copy (&adminPassword.front(), request.AdminPassword.c_str(), request.AdminPassword.size()); adminPassword[request.AdminPassword.size()] = '\n'; } + vector authOutput; #if defined(TC_LINUX ) Thread::Sleep (1000); // wait 1 second for the forked privilege helper to start @@ -1142,7 +1417,10 @@ namespace VeraCrypt else if (doasAuthTerminal != -1 && !doasNoPasswordAttempt) { // doas reads authentication from the controlling terminal, not stdin. - WriteAllBestEffort (doasAuthTerminal, &adminPassword.front(), adminPassword.size()); +#ifdef TC_OPENBSD + WaitForDoasAuthTerminalPrompt (doasAuthTerminal, authOutput, 2000); +#endif + WriteAllBestEffort (doasAuthTerminal, &adminPassword.front(), adminPassword.size(), 2000); } burn (&adminPassword.front(), adminPassword.size()); @@ -1154,8 +1432,19 @@ namespace VeraCrypt { shared_ptr inputStream (new FileStream (serviceInputFd)); shared_ptr outputStream (new FileStream (serviceOutputFd)); + int authOutputFd = -1; - SendElevatedServiceSyncWithTimeout (inputStream, serviceOutputFd, errPipe.GetReadFD(), forkedPid, privilegeHelper.Name, timeout); +#ifdef TC_OPENBSD + if (doasAuthTerminal != -1) + { + int authTerminalFlags = fcntl (doasAuthTerminal, F_GETFL, 0); + throw_sys_if (authTerminalFlags == -1); + throw_sys_if (fcntl (doasAuthTerminal, F_SETFL, authTerminalFlags | O_NONBLOCK) == -1); + authOutputFd = doasAuthTerminal; + } +#endif + + SendElevatedServiceSyncWithTimeout (inputStream, serviceOutputFd, errPipe.GetReadFD(), authOutputFd, authOutput, forkedPid, privilegeHelper.Name, timeout); throw_sys_if (fcntl (serviceOutputFd, F_SETFL, 0) == -1); ReapChildProcessAsync (forkedPid);