diff --git a/Tests/encryption_thread_pool_exception_test.cpp b/Tests/encryption_thread_pool_exception_test.cpp index 368ac957..77f08e68 100644 --- a/Tests/encryption_thread_pool_exception_test.cpp +++ b/Tests/encryption_thread_pool_exception_test.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -10,10 +11,23 @@ #include #include #include "Volume/EncryptionThreadPool.h" +#include "Volume/Pkcs5Kdf.h" using namespace VeraCrypt; static std::atomic Clones (0), LiveClones (0); +static thread_local bool FailNextAllocation; + +void *operator new (size_t size) +{ + if (FailNextAllocation) { FailNextAllocation = false; throw std::bad_alloc(); } + void *memory = std::malloc (size ? size : 1); + if (!memory) throw std::bad_alloc(); + return memory; +} +void operator delete (void *memory) noexcept { std::free (memory); } +void *operator new[] (size_t size) { return ::operator new (size); } +void operator delete[] (void *memory) noexcept { ::operator delete (memory); } static void Check (bool condition, const char *message) { @@ -35,6 +49,17 @@ struct TrackedException : Exception bool Stored; }; +struct UncopyableException : Exception +{ + Exception *CloneNew () override { throw std::bad_alloc(); } +}; + +struct InvalidMessageException : std::exception +{ + // Invalid in the C locale: exercise the real exception formatter's failure. + const char *what () const noexcept override { return "\xff"; } +}; + class PoolView : public EncryptionThreadPool { public: @@ -44,7 +69,7 @@ public: class TestMode : public EncryptionMode { public: - enum Failure { None, Native, Standard, Unknown }; + enum Failure { None, Native, Standard, Unknown, CloneFailure, FormattingFailure, AllocationFailure }; TestMode (size_t fragments, Failure failure) : Expected (fragments), Arrived (0), Kind (failure) { } @@ -62,6 +87,7 @@ public: private: void Process (uint8 *data, uint64 count, size_t sectorSize) const { + FailNextAllocation = false; { std::unique_lock lock (Gate); ++Arrived; @@ -74,6 +100,9 @@ private: case Native: throw TrackedException (); case Standard: throw std::runtime_error ("fragment failure"); case Unknown: throw 7; + case CloneFailure: throw UncopyableException(); + case FormattingFailure: throw InvalidMessageException(); + case AllocationFailure: FailNextAllocation = true; throw 7; case None: std::memset (data, 0xa5, count * sectorSize); break; } } @@ -85,8 +114,68 @@ private: mutable std::condition_variable Ready; }; +class TestKdf : public Pkcs5Kdf +{ +public: + explicit TestKdf (TestMode &mode) : Mode (mode) { } + int DeriveKey (const BufferPtr &key, const VolumePassword &, const ConstBufferPtr &, int) const override + { Mode.EncryptSectorsCurrentThread (key.Get(), 0, 1, key.Size()); return 0; } + int DeriveKey (const BufferPtr &key, const VolumePassword &password, const ConstBufferPtr &salt, int iterations, long volatile *) const override + { return DeriveKey (key, password, salt, iterations); } + shared_ptr GetHash () const override { return shared_ptr(); } + int GetIterationCount (int) const override { return 1; } + std::wstring GetName () const override { return L"exception-test"; } + Pkcs5Kdf *Clone () const override { return new TestKdf (Mode); } +private: + TestMode &Mode; +}; + +// The runner extracts the actual drain and exception checks from VolumeHeader.cpp. +#include "kdf_exception_completion.inc" + +static int KdfExceptions (size_t width) +{ + for (int kind = TestMode::None; kind <= TestMode::AllocationFailure; ++kind) + { + TestMode mode (width, static_cast (kind)); + shared_ptr kdf (new TestKdf (mode)); + VolumePassword password; + uint8 salt[64] = {}; + SyncEvent completed, noOutstandingWorkItemEvent; + SharedVal outstanding (0); + long volatile abortKeyDerivation = 0; + bool workItemsDrained = false; + std::vector> items; + for (size_t i = 0; i < width; ++i) + { + shared_ptr item (new EncryptionThreadPool::KeyDerivationWorkItem (kdf, 64)); + items.push_back (item); + EncryptionThreadPool::BeginKeyDerivation (*item, password, 0, ConstBufferPtr (salt, sizeof salt), completed, noOutstandingWorkItemEvent, outstanding, &abortKeyDerivation); + } + while (!std::all_of (items.begin(), items.end(), [] (const shared_ptr &item) { return item->Completed.Get(); })) + completed.Wait(); + for (const auto &item : items) + { + int observed = TestMode::None; + try { HandleKdfFailure (item, noOutstandingWorkItemEvent, width, workItemsDrained, abortKeyDerivation); } + catch (const TrackedException &) { observed = TestMode::Native; } + catch (const ExternalException &) { observed = TestMode::Standard; } + catch (const UnknownException &) { observed = TestMode::Unknown; } + Check (observed == std::min (kind, static_cast (TestMode::Unknown)), "KDF failure was lost or changed"); + Check (item->ExceptionCaptureFailed == (kind >= TestMode::CloneFailure), "KDF capture failure was not recorded"); + if (kind == TestMode::None) + for (size_t i = 0; i < item->DerivedKey.Size(); ++i) Check (item->DerivedKey[i] == 0xa5, "KDF recovery failed"); + } + DrainKeyDerivationWorkItems (noOutstandingWorkItemEvent, width, workItemsDrained); + Check (outstanding.Get() == 0, "KDF drain returned early"); + Check (kind == TestMode::None || abortKeyDerivation == 1, "KDF failure did not abort the header read"); + } + return TestMode::AllocationFailure + 1; +} + int main () { + std::setlocale (LC_CTYPE, "C"); EncryptionThreadPool::Start (); if (!EncryptionThreadPool::IsRunning ()) { @@ -101,7 +190,7 @@ int main () for (int direction = 0; direction < 2; ++direction) { auto type = direction ? EncryptionThreadPool::WorkType::DecryptDataUnits : EncryptionThreadPool::WorkType::EncryptDataUnits; - for (int kind = TestMode::Native; kind <= TestMode::Unknown; ++kind) + for (int kind = TestMode::Native; kind <= TestMode::AllocationFailure; ++kind) { TestMode mode (width, static_cast (kind)); int observed = TestMode::None; @@ -110,7 +199,7 @@ int main () catch (const TrackedException &) { observed = TestMode::Native; } catch (const ExternalException &) { observed = TestMode::Standard; } catch (const UnknownException &) { observed = TestMode::Unknown; } - Check (observed == kind, "requester received the wrong exception"); + Check (observed == std::min (kind, static_cast (TestMode::Unknown)), "requester received the wrong exception"); Check (LiveClones == 0, "stored exception leaked"); if (kind == TestMode::Native) Check (Clones == 1, "more than one fragment exception retained"); ++checks; @@ -121,7 +210,9 @@ int main () for (uint8 byte : data) Check (byte == 0xa5, "pool failed after exception propagation"); ++checks; } + checks += KdfExceptions (width); } EncryptionThreadPool::Stop (); - std::cout << "PASS: " << checks << " fragment exception and recovery checks" << std::endl; + Check (LiveClones == 0, "stored KDF exception leaked"); + std::cout << "PASS: " << checks << " fragment/KDF exception, capture-failure and recovery checks" << std::endl; } diff --git a/Tests/test_encryption_thread_pool_exceptions.py b/Tests/test_encryption_thread_pool_exceptions.py index 31df1d6a..2884c5a2 100644 --- a/Tests/test_encryption_thread_pool_exceptions.py +++ b/Tests/test_encryption_thread_pool_exceptions.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Check concurrent fragment exceptions in the production Unix encryption pool. +"""Check fragment/KDF exceptions and exception-capture failures in the Unix pool. Uses real pool, encryption-mode dispatch and platform synchronization code with an event-controlled test cipher. Requires Linux, Python 3 and a C++11 compiler. @@ -16,6 +16,16 @@ import tempfile ROOT = Path(__file__).resolve().parents[1] +def source_block(source, start): + first = source.index("{", start) + depth = 1 + for end in range(first + 1, len(source)): + depth += (source[end] == "{") - (source[end] == "}") + if depth == 0: + return source[start:end + 1] + raise RuntimeError("Unbalanced production source block") + + def build_test(build, sanitize, test_source, extra_sources=(), extra_flags=()): build.mkdir(parents=True, exist_ok=True) sources = ["Volume/EncryptionThreadPool.cpp", "Volume/EncryptionMode.cpp"] @@ -51,7 +61,19 @@ def main(): if not sys.platform.startswith("linux"): parser.error("This build runner requires Linux") build = (args.build_dir or Path(tempfile.mkdtemp(prefix="vc-pool-exceptions-"))).resolve() - exe = build_test(build, args.sanitize, "encryption_thread_pool_exception_test.cpp") + build.mkdir(parents=True, exist_ok=True) + header = (ROOT / "src/Volume/VolumeHeader.cpp").read_text() + drain = source_block(header, header.index("static void DrainKeyDerivationWorkItems (")) + failure = source_block(header, header.index("if (keyDerivationWorkItem->ItemException.get()")) + (build / "kdf_exception_completion.inc").write_text(drain + "\n" + """ +static void HandleKdfFailure ( + shared_ptr keyDerivationWorkItem, + SyncEvent &noOutstandingWorkItemEvent, size_t enqueuedWorkItemCount, + bool &workItemsDrained, long volatile &abortKeyDerivation) +{ +""" + failure + "\n}\n") + exe = build_test(build, args.sanitize, "encryption_thread_pool_exception_test.cpp", + ("Volume/Pkcs5Kdf.cpp", "Volume/VolumePassword.cpp"), ["-I" + str(build)]) result = subprocess.run([str(exe)], timeout=60) if result.returncode not in (0, 77): raise SystemExit(result.returncode) diff --git a/src/Volume/EncryptionThreadPool.cpp b/src/Volume/EncryptionThreadPool.cpp index 337cef59..611edf77 100644 --- a/src/Volume/EncryptionThreadPool.cpp +++ b/src/Volume/EncryptionThreadPool.cpp @@ -27,6 +27,21 @@ namespace VeraCrypt { + // Called from a catch handler. Failure to copy/format an exception must not + // bypass completion or turn a failed operation into a successful one. + static bool CaptureWorkItemException (unique_ptr &itemException) noexcept + { + try + { + try { throw; } + catch (Exception &e) { itemException.reset (e.CloneNew()); } + catch (exception &e) { itemException.reset (new ExternalException (SRC_POS, StringConverter::ToExceptionString (e))); } + catch (...) { itemException.reset (new UnknownException (SRC_POS)); } + return itemException.get() != nullptr; + } + catch (...) { return false; } + } + bool EncryptionThreadPool::AcquirePoolReference () { uint32 references = PoolReferences.load(); @@ -45,7 +60,7 @@ namespace VeraCrypt } EncryptionThreadPool::KeyDerivationWorkItem::KeyDerivationWorkItem (shared_ptr kdf, size_t derivedKeySize) - : Completed (false), DerivedKey (derivedKeySize), Kdf (kdf), Processed (false), Result (0) + : Completed (false), DerivedKey (derivedKeySize), ExceptionCaptureFailed (false), Kdf (kdf), Processed (false), Result (0) { } @@ -60,6 +75,7 @@ namespace VeraCrypt { keyDerivationWorkItem.Completed.Set (false); keyDerivationWorkItem.ItemException.reset(); + keyDerivationWorkItem.ExceptionCaptureFailed = false; keyDerivationWorkItem.Processed = false; keyDerivationWorkItem.Result = abortFlag && *abortFlag ? ERR_USER_ABORT : keyDerivationWorkItem.Kdf->DeriveKey (keyDerivationWorkItem.DerivedKey, password, pim, salt, abortFlag); @@ -86,6 +102,7 @@ namespace VeraCrypt keyDerivationWorkItem.Completed.Set (false); keyDerivationWorkItem.ItemException.reset(); + keyDerivationWorkItem.ExceptionCaptureFailed = false; keyDerivationWorkItem.Processed = false; keyDerivationWorkItem.Result = 0; @@ -177,6 +194,7 @@ namespace VeraCrypt firstFragmentWorkItem->OutstandingFragmentCount.Set (fragmentCount); firstFragmentWorkItem->ItemException.reset(); + firstFragmentWorkItem->ExceptionCaptureFailed = false; while (fragmentCount-- > 0) { @@ -215,12 +233,15 @@ namespace VeraCrypt unique_ptr itemException; if (firstFragmentWorkItem->ItemException.get()) itemException = move_ptr(firstFragmentWorkItem->ItemException); + bool exceptionCaptureFailed = firstFragmentWorkItem->ExceptionCaptureFailed.load(); firstFragmentWorkItem->State.Set (WorkItem::State::Free); WorkItemCompletedEvent.Signal(); if (itemException.get()) itemException->Throw(); + if (exceptionCaptureFailed) + throw UnknownException (SRC_POS); } void EncryptionThreadPool::Start () @@ -395,37 +416,23 @@ namespace VeraCrypt throw ParameterIncorrect (SRC_POS); } } - catch (Exception &e) - { - if (workItem->Type == WorkType::DeriveKey) - workItem->KeyDerivation.WorkItem->ItemException.reset (e.CloneNew()); - else - { - ScopeLock exceptionLock (FragmentExceptionMutex); - if (!workItem->FirstFragment->ItemException.get()) - workItem->FirstFragment->ItemException.reset (e.CloneNew()); - } - } - catch (exception &e) - { - if (workItem->Type == WorkType::DeriveKey) - workItem->KeyDerivation.WorkItem->ItemException.reset (new ExternalException (SRC_POS, StringConverter::ToExceptionString (e))); - else - { - ScopeLock exceptionLock (FragmentExceptionMutex); - if (!workItem->FirstFragment->ItemException.get()) - workItem->FirstFragment->ItemException.reset (new ExternalException (SRC_POS, StringConverter::ToExceptionString (e))); - } - } catch (...) { if (workItem->Type == WorkType::DeriveKey) - workItem->KeyDerivation.WorkItem->ItemException.reset (new UnknownException (SRC_POS)); + { + KeyDerivationWorkItem &item = *workItem->KeyDerivation.WorkItem; + item.ExceptionCaptureFailed = !CaptureWorkItemException (item.ItemException); + } else { - ScopeLock exceptionLock (FragmentExceptionMutex); - if (!workItem->FirstFragment->ItemException.get()) - workItem->FirstFragment->ItemException.reset (new UnknownException (SRC_POS)); + WorkItem &firstFragment = *workItem->FirstFragment; + try + { + ScopeLock exceptionLock (FragmentExceptionMutex); + if (!firstFragment.ItemException.get() && !firstFragment.ExceptionCaptureFailed) + firstFragment.ExceptionCaptureFailed = !CaptureWorkItemException (firstFragment.ItemException); + } + catch (...) { firstFragment.ExceptionCaptureFailed = true; } } } diff --git a/src/Volume/EncryptionThreadPool.h b/src/Volume/EncryptionThreadPool.h index 87780387..26018bc6 100644 --- a/src/Volume/EncryptionThreadPool.h +++ b/src/Volume/EncryptionThreadPool.h @@ -51,6 +51,7 @@ namespace VeraCrypt struct WorkItem *FirstFragment; unique_ptr ItemException; + std::atomic ExceptionCaptureFailed; SyncEvent ItemCompletedEvent; SharedVal OutstandingFragmentCount; SharedVal State; @@ -90,6 +91,7 @@ namespace VeraCrypt SharedVal Completed; SecureBuffer DerivedKey; unique_ptr ItemException; + bool ExceptionCaptureFailed; shared_ptr Kdf; bool Processed; int Result; diff --git a/src/Volume/VolumeHeader.cpp b/src/Volume/VolumeHeader.cpp index 7f79e274..f6a4a321 100644 --- a/src/Volume/VolumeHeader.cpp +++ b/src/Volume/VolumeHeader.cpp @@ -144,12 +144,14 @@ namespace VeraCrypt ++processedWorkItemCount; processed = true; - if (keyDerivationWorkItem->ItemException.get()) + if (keyDerivationWorkItem->ItemException.get() || keyDerivationWorkItem->ExceptionCaptureFailed) { // KDF exceptions are fatal setup/runtime errors; candidate failures are reported via Result. abortKeyDerivation = 1; DrainKeyDerivationWorkItems (noOutstandingWorkItemEvent, enqueuedWorkItemCount, workItemsDrained); - keyDerivationWorkItem->ItemException->Throw(); + if (keyDerivationWorkItem->ItemException.get()) + keyDerivationWorkItem->ItemException->Throw(); + throw UnknownException (SRC_POS); } if (keyDerivationWorkItem->Result != 0)