Repository navigation
Conversation
idrassi
left a comment
There was a problem hiding this comment.
Thanks for investigating this and putting together a reproducible case. I have noted inline the issues I see.
Before revising the implementation, I would like us to settle what should happen when the quick attempts don't find a match. Preserving full autodetection means an unknown Argon2 volume may still need an expensive attempt. Simply adding a full retry at the end would bring back the original worst case. We need a deliberate choice about when to ask the user for input and when to continue searching.
6718018 to
4ef4ad1
Compare
|
Thanks for the review. The test now dismounts only its own volumes and keeps its files if that fails (4ef4ad1); the implementation is unchanged until we settle the behaviour. A proposal:
This bounds the cost of a cache miss to PBKDF2 derivations, plus Argon2 only for passwords that have been used with Argon2. Excluding Argon2 from an autodetection attempt needs a new mount option for the core service; I would add it like Would this be acceptable, or would you rather keep full autodetection for some case in step 4 (for example the most recent entry)? |
4ef4ad1 to
2db9096
Compare
|
Updated in 24a3019, which settles the question in the code rather than through a protocol change:
The extra PBKDF2 attempts are made one at a time, so a cache miss costs more than before this PR's first version: in the test (PIM 100, three favorites) about 10 s instead of 2 s, against 173 s on master with Argon2. https://github.com/flatstik/VeraCrypt/actions/runs/40772767313 |
When a volume is mounted with an empty password, every cached password is tried with the selected KDF, or with autodetection, which tries all KDFs including Argon2. Argon2 uses up to 1 GiB, and above PIM 31 its passes grow with the PIM, so with a large PIM every cached password that does not belong to the volume took minutes, as with favorites in veracrypt#1815. Cache the KDF of the mounted volume together with the password, and with autodetection try each cached password only with that KDF. A password used with several KDFs gets an entry for each. If the KDF is unknown, autodetection is used as before. The Windows driver cache (src/Common/Cache.c) has the same problem (veracrypt#1969) and is not changed here.
Trying a cached password only with the KDF that it last opened a volume with stopped it from opening a volume that uses another KDF until the password was typed again, which matters most for keyfile contents. With an explicit KDF, an Argon2 derivation failure also ended the search instead of being skipped as in autodetection. With autodetection, each cached password is now tried with the KDF it opened a volume with, then with the other PBKDF2 KDFs, which are cheap. Argon2 is tried only for passwords that opened an Argon2 volume, or a volume whose KDF is unknown, through autodetection so that its handling of derivation failures is kept. Passwords associated with PBKDF2 come first, so an Argon2 association no longer delays them. If nothing matches, the user is asked for the password instead of trying Argon2 with every cached password: an Argon2 volume whose password is cached only from PBKDF2 volumes needs it once, and is then known as Argon2.
Tests/test_password_cache_kdf.py mounts three favorites with password caching and a PIM of 100, and checks that a cached password that does not belong to a volume is not tried with Argon2, and that a matching cached password still mounts a volume without a prompt.
2db9096 to
d132aa3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Cached-password attempts can prioritize an older outer-volume password over a newer hidden-volume password, risking a writable mount without hidden-volume protection and consequent hidden-data corruption.
6 open issues
Preserve cached-password priority across KDF fallbacks · New Parse quoted mount paths without splitting on whitespace · New Deduplicate cached passwords by selected KDF · New Retry other KDFs for system-partition credential failures · New Record the resolved KDF after cached mounts · New Keep cache capacity per distinct password · New
What changed in this PR
Reduces slow Unix cached mounts associated with #1969 by recording each password’s KDF and avoiding unrelated, expensive Argon2 attempts.
Changes:
- Stores password/KDF pairs and prioritizes known KDFs during cached mounts.
- Retains explicitly selected KDFs and cross-KDF PBKDF2 fallback.
- Adds a Linux regression test for cached cross-KDF mounts and high-PIM performance.
| File | Description |
|---|---|
Tests/test_password_cache_kdf.py |
Checks cached mounts, password prompts, and timing across three volumes. |
src/Volume/VolumePasswordCache.h |
Defines cache entries containing a password and its KDF. |
src/Volume/VolumePasswordCache.cpp |
Stores and copies password/KDF associations. |
src/Core/Unix/CoreServiceProxy.h |
Schedules cached KDF attempts and records detected KDFs. |
.github/workflows/build-linux.yml |
Adds regression-test triggers and execution to Linux package checks. |
Review effort: Max
The attempts were ordered by cost across all cached passwords, so an older password could be tried before a newer one. If the newer password opens a hidden volume with Argon2 and the older one its outer volume with SHA-512, the outer volume was mounted, writable and without protection of the hidden volume. Finish each cached password before the next one, from the most recently used. Within a password the order is unchanged: the PBKDF2 KDFs it opened a volume with, the other PBKDF2 KDFs, then autodetection if it opened an Argon2 volume or a volume with an unknown KDF. Otherwise Argon2 is not tried and the user is asked for the password, as before. An Argon2 password that does not belong to the volume now costs its derivations before older passwords are tried, as it does without this PR. Keep one cache entry per password with the KDFs it opened volumes with, so that a second KDF does not evict another password and a selected KDF is tried once per password. Remember the KDF after a mount from the cache too, without moving the password in the cache. Try the remaining attempts after PasswordOrKeyboardLayoutIncorrect and PasswordOrMountOptionsIncorrect, and throw the last failure if all of them fail. Decode quoted paths of --list in the test, and add a check that a container is mounted as the hidden volume in the case above.


Linux and macOS part of #1969 (see #1815 and #1831 for the same cost path).
When a volume is mounted with an empty password, every cached password is tried with the selected KDF, or with autodetection, which also tries Argon2. Above PIM 31, Argon2 uses 1 GiB and its passes grow with the PIM, so each cached password that does not belong to the volume took minutes.
The cache now keeps with each password the KDFs it opened volumes with. Passwords are tried from the most recently used, each one before the next, so an older password cannot open an outer volume ahead of the newer password of its hidden volume. With autodetection, a password is tried with its remembered PBKDF2 KDFs, then the other PBKDF2 KDFs, then full autodetection only if it opened an Argon2 volume or one with an unknown KDF; otherwise the user is asked for the password. An explicitly selected KDF is still used for all cached passwords, once each.
A different, newer Argon2 password still costs its derivations before an older password is tried (122 s at PIM 100 in my measurement), which by my reading of the code master pays too; I did not run master. An allocation failure does not stop the search.
The Windows driver cache (
src/Common/Cache.c) has the same problem and is not changed here; I cannot test Windows or macOS.Tested with three volumes (SHA-512, A and C share a password, B has another), mounted as favorites in one run with password caching, typing only the passwords of A and B:
Tests/test_password_cache_kdf.py(new, PIM 100, run in the Linux workflow): with this change the run takes 1.8 s (ubuntu-24.04) and 2.1 s (ubuntu-26.04), against 0.4 to 0.5 s for one mount; on unchanged master it takes 173 s and the test fails. Runs on my fork: https://github.com/flatstik/VeraCrypt/actions/runs/40508100174 (this change), https://github.com/flatstik/VeraCrypt/actions/runs/40508103148 (master with only the test).