diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 204d29af8b..dec30fd835 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_text_overwrite.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_text_overwrite.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 + python3 Tests/test_text_overwrite.py sudo apt remove -y veracrypt - name: Install and test VeraCrypt Console .deb packages diff --git a/Tests/test_text_overwrite.py b/Tests/test_text_overwrite.py new file mode 100644 index 0000000000..67d825c531 --- /dev/null +++ b/Tests/test_text_overwrite.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Regression test for issue #1962, against the installed veracrypt. + +Usage (after installing a package; root is not needed): + python3 Tests/test_text_overwrite.py [--veracrypt PATH] + +Checks that --create-keyfile refuses an existing keyfile without asking, +also in interactive mode, unless --force is given, and that +--backup-headers asks before replacing an existing file. +""" + +import argparse +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +PASSWORD_A = "regression-test-password-a" +# A low PIM keeps the tests fast; it needs passwords of 20 characters or more. +COMMON = ["--pim=1", "--keyfiles="] + + +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 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)) + r = vc.run(["--create-keyfile", str(keyfile), + "--random-source=" + str(work / "random")], stdin=b"y\n") + c.check("#1962: interactive --create-keyfile refuses without asking", + r.returncode != 0 and "--force" in text(r) + and "(y=Yes" not 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") + args = parser.parse_args() + + work = Path(tempfile.mkdtemp(prefix="vc-regression-")) + vc = VeraCrypt(args.veracrypt, work) + c = Checks() + (work / "random").write_bytes(os.urandom(4096)) + try: + test_overwrite(vc, c, work) + finally: + 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/Main/TextUserInterface.cpp b/src/Main/TextUserInterface.cpp index 7f00579556..4d22abf2f8 100644 --- a/src/Main/TextUserInterface.cpp +++ b/src/Main/TextUserInterface.cpp @@ -460,6 +460,19 @@ namespace VeraCrypt if (filePath.IsEmpty()) throw UserAbort (SRC_POS); + // Same prompt as the file dialog of the graphical interface. Header + // backups are interactive anyway; --force overwrites without asking. + if (!CmdLine->ArgForce && FilesystemPath (wstring (filePath)).IsFile()) + { + wxString confirmMsg = wxString::Format (_("File '%s' already exists, do you really want to overwrite it?"), wstring (filePath).c_str()); + + if (Preferences.NonInteractive) + throw_err (confirmMsg); + + if (!AskYesNo (confirmMsg, false, true)) + throw UserAbort (SRC_POS); + } + File backupFile; backupFile.Open (filePath, File::CreateWrite); @@ -621,24 +634,29 @@ namespace VeraCrypt { 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); } + // Replacing a keyfile makes every volume that uses it unmountable. Refuse + // without asking, so that existing scripts are not stopped by a prompt. + if (!CmdLine->ArgForce && FilesystemPath (wstring (path)).IsFile()) + throw_err (wxString::Format (_("The keyfile '%s' already exists. Use --force to replace it."), wstring (path).c_str())); + + RandomNumberGenerator::Start(); + /* force the display of the random enriching interface */ + RandomNumberGenerator::SetEnrichedByUserStatus (false); + UserEnrichRandomPool(); + + Core->CreateKeyfile (path); + ShowInfo ("KEYFILE_CREATED"); }